| Message ID | 1361907862-13009-1-git-send-email-vyasevic@redhat.com |
|---|---|
| State | RFC, archived |
| Delegated to: | David Miller |
| Headers | show |
From: Vlad Yasevich <vyasevic@redhat.com> Date: Tue, 26 Feb 2013 14:44:22 -0500 > David Miller wrote: >> No new ioctls please, extend the netlink interfaces as necessary >> instead. >> >> Thanks. > > So, something like below patch (compiled only) would be better in your opinion? > The one concern I have is that there is no way to probe for the availabilty > of this functionality, since netlink will just ignore unknown types and appear > to succeed. With an ioctl it is clear if the support is present. > > -- >8 -- > > > [RFC PATCH] rtnetlink: Add support for adding/removing additional hw addresses. > > Add an ability to add and remove HW addresses to the device > unicast and multicast address lists. Right now, we only have > an ioctl() to manage the multicast addresses and there is no > way the manage the unicast list. > > Signed-off-by: Vlad Yasevich <vyasevic@redhat.com> This is a step in the right direction, and you're right that there is a difficulty in detecting whether support exists or not. I am so surprised that we've have ->set_rx_mode() support for multiple unicast MAC addresses in so many drivers all this time, yet no way outside of FDB to make use of it at all. Anyways, as per the feature detection issue, we could create a new RTM_* operation that returns a u32 feature flag mask. It's just one idea. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 02/26/2013 03:03 PM, David Miller wrote: > From: Vlad Yasevich <vyasevic@redhat.com> > Date: Tue, 26 Feb 2013 14:44:22 -0500 > >> David Miller wrote: >>> No new ioctls please, extend the netlink interfaces as necessary >>> instead. >>> >>> Thanks. >> >> So, something like below patch (compiled only) would be better in your opinion? >> The one concern I have is that there is no way to probe for the availabilty >> of this functionality, since netlink will just ignore unknown types and appear >> to succeed. With an ioctl it is clear if the support is present. >> >> -- >8 -- >> >> >> [RFC PATCH] rtnetlink: Add support for adding/removing additional hw addresses. >> >> Add an ability to add and remove HW addresses to the device >> unicast and multicast address lists. Right now, we only have >> an ioctl() to manage the multicast addresses and there is no >> way the manage the unicast list. >> >> Signed-off-by: Vlad Yasevich <vyasevic@redhat.com> > > This is a step in the right direction, and you're right that there is > a difficulty in detecting whether support exists or not. > > I am so surprised that we've have ->set_rx_mode() support for multiple > unicast MAC addresses in so many drivers all this time, yet no way > outside of FDB to make use of it at all. And even that is not always available. In most drivers it requires module parameters or other explicit configuration steps. Meanwhile set_rx_mode() doesn't seem to depend on any of those and just does the right thing. For what I was trying to do ioctl() was a really easy way out for both kernel and user space implementation, so I gave is shot. -vlad > > Anyways, as per the feature detection issue, we could create a new > RTM_* operation that returns a u32 feature flag mask. It's just one > idea. > -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
From: Vlad Yasevich <vyasevic@redhat.com> Date: Tue, 26 Feb 2013 15:24:33 -0500 > For what I was trying to do ioctl() was a really easy way out for both > kernel and user space implementation, so I gave is shot. Look at it this way, you were going to have to add these config values to the rtnetlink LINK dumps anyways. :-) -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[...] >>> >>> >>> [RFC PATCH] rtnetlink: Add support for adding/removing additional hw >>> addresses. >>> >>> Add an ability to add and remove HW addresses to the device >>> unicast and multicast address lists. Right now, we only have >>> an ioctl() to manage the multicast addresses and there is no >>> way the manage the unicast list. >>> >>> Signed-off-by: Vlad Yasevich <vyasevic@redhat.com> >> >> This is a step in the right direction, and you're right that there is >> a difficulty in detecting whether support exists or not. >> >> I am so surprised that we've have ->set_rx_mode() support for multiple >> unicast MAC addresses in so many drivers all this time, yet no way >> outside of FDB to make use of it at all. > > And even that is not always available. In most drivers it requires > module parameters or other explicit configuration steps. Meanwhile > set_rx_mode() doesn't seem to depend on any of those and just does the > right thing. > > For what I was trying to do ioctl() was a really easy way out for both > kernel and user space implementation, so I gave is shot. > > -vlad > Don't we already support this with int (*ndo_fdb_add)(struct ndmsg *ndm, struct nlattr *tb[], struct net_device *dev, const unsigned char *addr, u16 flags); int (*ndo_fdb_del)(struct ndmsg *ndm, struct nlattr *tb[], struct net_device *dev, const unsigned char *addr); int (*ndo_fdb_dump)(struct sk_buff *skb, struct netlink_callback *cb, struct net_device *dev, int idx); -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
From: John Fastabend <john.r.fastabend@intel.com> Date: Tue, 26 Feb 2013 13:58:39 -0800 > [...] > >>>> >>>> >>>> [RFC PATCH] rtnetlink: Add support for adding/removing additional hw >>>> addresses. >>>> >>>> Add an ability to add and remove HW addresses to the device >>>> unicast and multicast address lists. Right now, we only have >>>> an ioctl() to manage the multicast addresses and there is no >>>> way the manage the unicast list. >>>> >>>> Signed-off-by: Vlad Yasevich <vyasevic@redhat.com> >>> >>> This is a step in the right direction, and you're right that there is >>> a difficulty in detecting whether support exists or not. >>> >>> I am so surprised that we've have ->set_rx_mode() support for multiple >>> unicast MAC addresses in so many drivers all this time, yet no way >>> outside of FDB to make use of it at all. >> >> And even that is not always available. In most drivers it requires >> module parameters or other explicit configuration steps. Meanwhile >> set_rx_mode() doesn't seem to depend on any of those and just does the >> right thing. >> >> For what I was trying to do ioctl() was a really easy way out for both >> kernel and user space implementation, so I gave is shot. >> >> -vlad >> > > Don't we already support this with The whole point is that these multiple-unicast-address configuration facilities are inaccessible without FDB, and there is no reason whatsoever for that. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/include/uapi/linux/if_link.h b/include/uapi/linux/if_link.h index c4edfe1..7933a1c 100644 --- a/include/uapi/linux/if_link.h +++ b/include/uapi/linux/if_link.h @@ -143,6 +143,9 @@ enum { IFLA_NUM_TX_QUEUES, IFLA_NUM_RX_QUEUES, IFLA_CARRIER, + IFLA_HW_ADDR_ADD, + IFLA_HW_ADDR_DEL, + IFLA_HW_ADDR_LIST, __IFLA_MAX }; diff --git a/include/uapi/linux/rtnetlink.h b/include/uapi/linux/rtnetlink.h index 7a2144e..4d6f1c9 100644 --- a/include/uapi/linux/rtnetlink.h +++ b/include/uapi/linux/rtnetlink.h @@ -631,6 +631,7 @@ struct tcamsg { /* New extended info filters for IFLA_EXT_MASK */ #define RTEXT_FILTER_VF (1 << 0) #define RTEXT_FILTER_BRVLAN (1 << 1) +#define RTEXT_FILTER_MACLIST (1 << 2) /* End of information exported to user level */ diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c index d8aa20f..4df2ff1 100644 --- a/net/core/rtnetlink.c +++ b/net/core/rtnetlink.c @@ -763,6 +763,20 @@ static size_t rtnl_port_size(const struct net_device *dev) return port_self_size; } +static size_t rtnl_maclist_size(const struct net_device *dev, + u32 ext_filter_mask) +{ + size_t size = 0; + + if (ext_filter_mask & RTEXT_FILTER_MACLIST) { + size = nla_total_size(sizeof(struct nlattr)) + + nla_total_size(ETH_ALEN) * + (netdev_uc_count(dev) + netdev_mc_count(dev)); + } + + return size; +} + static noinline size_t if_nlmsg_size(const struct net_device *dev, u32 ext_filter_mask) { @@ -790,6 +804,7 @@ static noinline size_t if_nlmsg_size(const struct net_device *dev, & RTEXT_FILTER_VF ? 4 : 0) /* IFLA_NUM_VF */ + rtnl_vfinfo_size(dev, ext_filter_mask) /* IFLA_VFINFO_LIST */ + rtnl_port_size(dev) /* IFLA_VF_PORTS + IFLA_PORT_SELF */ + + rtnl_maclist_size(dev, ext_filter_mask) /* IFLA_MAC_LIST */ + rtnl_link_get_size(dev) /* IFLA_LINKINFO */ + rtnl_link_get_af_size(dev); /* IFLA_AF_SPEC */ } @@ -870,6 +885,39 @@ static int rtnl_port_fill(struct sk_buff *skb, struct net_device *dev) return 0; } +static int rtnl_hwaddr_fill(struct sk_buff *skb, struct net_device *dev, + u32 ext_filter_mask) +{ + struct netdev_hw_addr *ha; + struct nlattr *hw_list; + + if (!(ext_filter_mask & RTEXT_FILTER_MACLIST)) + return 0; + + if ((netdev_uc_count(dev) + netdev_mc_count(dev)) == 0) + return 0; + + hw_list = nla_nest_start(skb, IFLA_HW_ADDR_LIST); + if (!hw_list) + return -EMSGSIZE; + + /* Unicasts first */ + netdev_for_each_uc_addr(ha, dev) { + if (nla_put(skb, IFLA_ADDRESS, ETH_ALEN, ha->addr)) + goto nla_put_fail; + } + + /* Now multicasts */ + netdev_for_each_mc_addr(ha, dev) { + if (nla_put(skb, IFLA_ADDRESS, ETH_ALEN, ha->addr)) + goto nla_put_fail; + } + +nla_put_fail: + nla_nest_cancel(skb, hw_list); + return -EMSGSIZE; +} + static int rtnl_fill_ifinfo(struct sk_buff *skb, struct net_device *dev, int type, u32 pid, u32 seq, u32 change, unsigned int flags, u32 ext_filter_mask) @@ -1044,6 +1092,9 @@ static int rtnl_fill_ifinfo(struct sk_buff *skb, struct net_device *dev, } } + if (rtnl_hwaddr_fill(skb, dev, ext_filter_mask)) + goto nla_put_failure; + nla_nest_end(skb, af_spec); return nlmsg_end(skb, nlh); @@ -1128,6 +1179,8 @@ const struct nla_policy ifla_policy[IFLA_MAX+1] = { [IFLA_PROMISCUITY] = { .type = NLA_U32 }, [IFLA_NUM_TX_QUEUES] = { .type = NLA_U32 }, [IFLA_NUM_RX_QUEUES] = { .type = NLA_U32 }, + [IFLA_HW_ADDR_ADD] = { .type = NLA_BINARY, .len = ETH_ALEN }, + [IFLA_HW_ADDR_DEL] = { .type = NLA_BINARY, .len = ETH_ALEN }, }; EXPORT_SYMBOL(ifla_policy); @@ -1527,7 +1580,36 @@ static int do_setlink(struct net_device *dev, struct ifinfomsg *ifm, modified = 1; } } - err = 0; + + if (tb[IFLA_HW_ADDR_ADD]) { + const unsigned char *addr; + + addr = nla_data(tb[IFLA_HW_ADDR_ADD]); + + if (is_unicast_ether_addr(addr)) + err = dev_uc_add_excl(dev, addr); + else if (is_multicast_ether_addr(addr)) + err = dev_mc_add_excl(dev, addr); + + if (err) + goto errout; + modified = 1; + } + + if (tb[IFLA_HW_ADDR_DEL]) { + const unsigned char *addr; + + addr = nla_data(tb[IFLA_HW_ADDR_DEL]); + + if (is_unicast_ether_addr(addr)) + err = dev_uc_del(dev, addr); + else if (is_multicast_ether_addr(addr)) + err = dev_mc_del(dev, addr); + + if (err) + goto errout; + modified = 1; + } errout: if (err < 0 && modified)