| Message ID | 20120409220030.3288.31389.stgit@jf-dev1-dcblab |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | David Miller |
| Headers | show |
On Mon, 2012-04-09 at 15:00 -0700, John Fastabend wrote: > This adds a generic dump routine drivers can call. It > should be sufficient to handle any bridging model that > uses the unicast address list. This should be most SR-IOV > enabled NICs. [...] > +static int nlmsg_populate_fdb(struct sk_buff *skb, > + struct netlink_callback *cb, > + struct net_device *dev, > + int *idx, > + struct netdev_hw_addr_list *list) > +{ > + struct netdev_hw_addr *ha; > + struct ndmsg *ndm; > + struct nlmsghdr *nlh; > + u32 pid, seq; > + > + pid = NETLINK_CB(cb->skb).pid; > + seq = cb->nlh->nlmsg_seq; > + > + list_for_each_entry(ha, &list->list, list) { > + if (*idx < cb->args[0]) > + goto skip; > + > + nlh = nlmsg_put(skb, pid, seq, > + RTM_NEWNEIGH, sizeof(*ndm), NLM_F_MULTI); > + if (!nlh) > + break; This break is effectively return 0, but shouldn't we return an error? In practice this does no harm because any subsequent invocation of nlmsg_populate_fdb() for the same skb is also going to fail here with no change to *idx. But it would be more obviously correct to return an error. [...] > + } > + return 0; > +nla_put_failure: > + nlmsg_cancel(skb, nlh); > + return -ENOMEM; > +} [...]
On 4/10/2012 8:45 PM, Ben Hutchings wrote: > On Mon, 2012-04-09 at 15:00 -0700, John Fastabend wrote: >> This adds a generic dump routine drivers can call. It >> should be sufficient to handle any bridging model that >> uses the unicast address list. This should be most SR-IOV >> enabled NICs. > [...] >> +static int nlmsg_populate_fdb(struct sk_buff *skb, >> + struct netlink_callback *cb, >> + struct net_device *dev, >> + int *idx, >> + struct netdev_hw_addr_list *list) >> +{ >> + struct netdev_hw_addr *ha; >> + struct ndmsg *ndm; >> + struct nlmsghdr *nlh; >> + u32 pid, seq; >> + >> + pid = NETLINK_CB(cb->skb).pid; >> + seq = cb->nlh->nlmsg_seq; >> + >> + list_for_each_entry(ha, &list->list, list) { >> + if (*idx < cb->args[0]) >> + goto skip; >> + >> + nlh = nlmsg_put(skb, pid, seq, >> + RTM_NEWNEIGH, sizeof(*ndm), NLM_F_MULTI); >> + if (!nlh) >> + break; > > This break is effectively return 0, but shouldn't we return an error? > In practice this does no harm because any subsequent invocation of > nlmsg_populate_fdb() for the same skb is also going to fail here with no > change to *idx. But it would be more obviously correct to return an > error. > sure returning -EMSGSIZE seems to be inline with rtnl_fill_ifinfo and easier to read. > [...] >> + } >> + return 0; >> +nla_put_failure: >> + nlmsg_cancel(skb, nlh); >> + return -ENOMEM; >> +} > [...] > also might be better to return -EMSGSIZE here as well. It seems to be more inline with convention. .John -- 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/net/core/rtnetlink.c b/net/core/rtnetlink.c index d6ce728..2bb4f59 100644 --- a/net/core/rtnetlink.c +++ b/net/core/rtnetlink.c @@ -2088,6 +2088,77 @@ static int rtnl_fdb_del(struct sk_buff *skb, struct nlmsghdr *nlh, void *arg) return err; } +static int nlmsg_populate_fdb(struct sk_buff *skb, + struct netlink_callback *cb, + struct net_device *dev, + int *idx, + struct netdev_hw_addr_list *list) +{ + struct netdev_hw_addr *ha; + struct ndmsg *ndm; + struct nlmsghdr *nlh; + u32 pid, seq; + + pid = NETLINK_CB(cb->skb).pid; + seq = cb->nlh->nlmsg_seq; + + list_for_each_entry(ha, &list->list, list) { + if (*idx < cb->args[0]) + goto skip; + + nlh = nlmsg_put(skb, pid, seq, + RTM_NEWNEIGH, sizeof(*ndm), NLM_F_MULTI); + if (!nlh) + break; + + ndm = nlmsg_data(nlh); + ndm->ndm_family = AF_BRIDGE; + ndm->ndm_pad1 = 0; + ndm->ndm_pad2 = 0; + ndm->ndm_flags = NTF_SELF; + ndm->ndm_type = 0; + ndm->ndm_ifindex = dev->ifindex; + ndm->ndm_state = NUD_PERMANENT; + + if (nla_put(skb, NDA_LLADDR, ETH_ALEN, ha->addr)) + goto nla_put_failure; + + nlmsg_end(skb, nlh); +skip: + *idx += 1; + } + return 0; +nla_put_failure: + nlmsg_cancel(skb, nlh); + return -ENOMEM; +} + +/** + * ndo_dflt_fdb_dump: default netdevice operation to dump an FDB table. + * @nlh: netlink message header + * @dev: netdevice + * + * Default netdevice operation to dump the existing unicast address list. + * Returns zero on success. + */ +int ndo_dflt_fdb_dump(struct sk_buff *skb, + struct netlink_callback *cb, + struct net_device *dev, + int idx) +{ + int err; + + netif_addr_lock_bh(dev); + err = nlmsg_populate_fdb(skb, cb, dev, &idx, &dev->uc); + if (err) + goto out; + nlmsg_populate_fdb(skb, cb, dev, &idx, &dev->mc); +out: + netif_addr_unlock_bh(dev); + return idx; +} +EXPORT_SYMBOL(ndo_dflt_fdb_dump); + static int rtnl_fdb_dump(struct sk_buff *skb, struct netlink_callback *cb) { int idx = 0;
This adds a generic dump routine drivers can call. It should be sufficient to handle any bridging model that uses the unicast address list. This should be most SR-IOV enabled NICs. Signed-off-by: John Fastabend <john.r.fastabend@intel.com> --- net/core/rtnetlink.c | 71 ++++++++++++++++++++++++++++++++++++++++++++++++++ 1 files changed, 71 insertions(+), 0 deletions(-) -- 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