Messages in this thread | | | Date | Mon, 13 Jan 2020 16:02:02 +0100 | From | Jiri Pirko <> | Subject | Re: [PATCH net-next v5 06/15] net: macsec: add nla support for changing the offloading selection |
| |
Fri, Jan 10, 2020 at 05:20:01PM CET, antoine.tenart@bootlin.com wrote:
[...]
>+static int macsec_upd_offload(struct sk_buff *skb, struct genl_info *info) >+{ >+ struct nlattr *tb_offload[MACSEC_OFFLOAD_ATTR_MAX + 1]; >+ enum macsec_offload offload, prev_offload; >+ int (*func)(struct macsec_context *ctx); >+ struct nlattr **attrs = info->attrs; >+ struct net_device *dev, *loop_dev; >+ const struct macsec_ops *ops; >+ struct macsec_context ctx; >+ struct macsec_dev *macsec; >+ struct net *loop_net; >+ int ret; >+ >+ if (!attrs[MACSEC_ATTR_IFINDEX]) >+ return -EINVAL; >+ >+ if (!attrs[MACSEC_ATTR_OFFLOAD]) >+ return -EINVAL; >+ >+ if (nla_parse_nested_deprecated(tb_offload, MACSEC_OFFLOAD_ATTR_MAX, >+ attrs[MACSEC_ATTR_OFFLOAD], >+ macsec_genl_offload_policy, NULL)) >+ return -EINVAL; >+ >+ dev = get_dev_from_nl(genl_info_net(info), attrs); >+ if (IS_ERR(dev)) >+ return PTR_ERR(dev); >+ macsec = macsec_priv(dev); >+ >+ offload = nla_get_u8(tb_offload[MACSEC_OFFLOAD_ATTR_TYPE]); >+ if (macsec->offload == offload) >+ return 0; >+ >+ /* Check if the offloading mode is supported by the underlying layers */ >+ if (offload != MACSEC_OFFLOAD_OFF && >+ !macsec_check_offload(offload, macsec)) >+ return -EOPNOTSUPP; >+ >+ if (offload == MACSEC_OFFLOAD_OFF) >+ goto skip_limitation; >+ >+ /* Check the physical interface isn't offloading another interface >+ * first. >+ */ >+ for_each_net(loop_net) { >+ for_each_netdev(loop_net, loop_dev) { >+ struct macsec_dev *priv; >+ >+ if (!netif_is_macsec(loop_dev)) >+ continue; >+ >+ priv = macsec_priv(loop_dev); >+ >+ if (priv->real_dev == macsec->real_dev && >+ priv->offload != MACSEC_OFFLOAD_OFF) >+ return -EBUSY; >+ } >+ } >+ >+skip_limitation: >+ /* Check if the net device is busy. */ >+ if (netif_running(dev)) >+ return -EBUSY; >+ >+ rtnl_lock(); >+ >+ prev_offload = macsec->offload; >+ macsec->offload = offload; >+ >+ /* Check if the device already has rules configured: we do not support >+ * rules migration. >+ */ >+ if (macsec_is_configured(macsec)) { >+ ret = -EBUSY; >+ goto rollback; >+ }
I wonder, did you consider having MACSEC_OFFLOAD_ATTR_TYPE attribute passed during the macsec device creation (to macsec_newlink), so the device is either created "offloded" or not? Looks like an extra step. Or do you see a scenario one would change "offload" setting on fly? If not, I don't see any benefit in having this as a separate command.
[...]
>+ { >+ .cmd = MACSEC_CMD_UPD_OFFLOAD, >+ .validate = GENL_DONT_VALIDATE_STRICT | GENL_DONT_VALIDATE_DUMP, >+ .doit = macsec_upd_offload, >+ .flags = GENL_ADMIN_PERM, >+ },
[...]
| |