Re: [patch net-next v3 02/17] net: make vid as a parameter for ndo_fdb_add/ndo_fdb_del

Andy Gospodarek
2014-11-25T15:13:46Z

On Tue, Nov 25, 2014 at 11:28:33AM +0100, Jiri Pirko wrote:
> Do the work of parsing NDA_VLAN directly in rtnetlink code, pass simple
> u16 vid to drivers from there.
>
> Signed-off-by: Jiri Pirko <jiri@resnulli.us>

Structurally this looks fine, just a misspelling noted below.

Acked-by: Andy Gospodarek <gospo@cumulusnetworks.com>

> ---
> new in v3
> ---
> drivers/net/ethernet/intel/i40e/i40e_main.c | 2 +-
> drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 4 +-
> drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c | 9 +++--
> drivers/net/macvlan.c | 4 +-
> drivers/net/vxlan.c | 4 +-
> include/linux/netdevice.h | 8 ++--
> include/linux/rtnetlink.h | 6 ++-
> net/bridge/br_fdb.c | 39 ++----------------
> net/bridge/br_private.h | 4 +-
> net/core/rtnetlink.c | 50 ++++++++++++++++++++----
> 10 files changed, 70 insertions(+), 60 deletions(-)
>
[...]
> diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
> index a688268..f2a4b38 100644
> --- a/net/core/rtnetlink.c
> +++ b/net/core/rtnetlink.c
> @@ -36,6 +36,7 @@
> #include <linux/mutex.h>
> #include <linux/if_addr.h>
> #include <linux/if_bridge.h>
> +#include <linux/if_vlan.h>
> #include <linux/pci.h>
> #include <linux/etherdevice.h>
>
> @@ -2312,7 +2313,7 @@ errout:
> int ndo_dflt_fdb_add(struct ndmsg *ndm,
> struct nlattr *tb[],
> struct net_device *dev,
> - const unsigned char *addr,
> + const unsigned char *addr, u16 vid,
> u16 flags)
> {
> int err = -EINVAL;
> @@ -2338,6 +2339,28 @@ int ndo_dflt_fdb_add(struct ndmsg *ndm,
> }
> EXPORT_SYMBOL(ndo_dflt_fdb_add);
>
> +static int fbd_vid_parse(struct nlattr *vlan_attr, u16 *p_vid)
I presume this is a misspelling?

[...]
> @@ -2370,6 +2394,10 @@ static int rtnl_fdb_add(struct sk_buff *skb, struct nlmsghdr *nlh)
>
> addr = nla_data(tb[NDA_LLADDR]);
>
> + err = fbd_vid_parse(tb[NDA_VLAN], &vid);
Same here....

> + if (err)
> + return err;
> +
> err = -EOPNOTSUPP;
>
> /* Support fdb on master device the net/bridge default case */

[...]
> @@ -2465,6 +2496,10 @@ static int rtnl_fdb_del(struct sk_buff *skb, struct nlmsghdr *nlh)
>
> addr = nla_data(tb[NDA_LLADDR]);
>
> + err = fbd_vid_parse(tb[NDA_VLAN], &vid);
...and here.

> + if (err)
> + return err;
> +
> err = -EOPNOTSUPP;
>
> /* Support fdb on master device the net/bridge default case */
> @@ -2474,7 +2509,7 @@ static int rtnl_fdb_del(struct sk_buff *skb, struct nlmsghdr *nlh)
> const struct net_device_ops *ops = br_dev->netdev_ops;
>
> if (ops->ndo_fdb_del)
> - err = ops->ndo_fdb_del(ndm, tb, dev, addr);
> + err = ops->ndo_fdb_del(ndm, tb, dev, addr, vid);
>
> if (err)
> goto out;
> @@ -2485,9 +2520,10 @@ static int rtnl_fdb_del(struct sk_buff *skb, struct nlmsghdr *nlh)
> /* Embedded bridge, macvlan, and any other device support */
> if (ndm->ndm_flags & NTF_SELF) {
> if (dev->netdev_ops->ndo_fdb_del)
> - err = dev->netdev_ops->ndo_fdb_del(ndm, tb, dev, addr);
> + err = dev->netdev_ops->ndo_fdb_del(ndm, tb, dev, addr,
> + vid);
> else
> - err = ndo_dflt_fdb_del(ndm, tb, dev, addr);
> + err = ndo_dflt_fdb_del(ndm, tb, dev, addr, vid);
>
> if (!err) {
> rtnl_fdb_notify(dev, addr, RTM_DELNEIGH);
> --
> 1.9.3
>