| Message ID | 1351897012.2703.17.camel@bwh-desktop.uk.solarflarecom.com |
|---|---|
| State | Accepted, archived |
| Delegated to: | David Miller |
| Headers | show |
On Fri, 2012-11-02 at 22:56 +0000, Ben Hutchings wrote: [...] > I'm still not sure it's correct to break iteration if > ndo_bridge_getlink returns -EOPNOTSUPP (possible if the driver supports > a mixture of bridging and non-bridging devices). [...] Never mind that - the driver can just return 0 for those devices, as ixgbe is doing. But the bug this patch addresses seems to be real. Ben.
On 11/2/2012 3:56 PM, Ben Hutchings wrote: > Commit e5a55a898720096f43bc24938f8875c0a1b34cd7 ('net: create generic > bridge ops') broke the handling of a non-zero starting index in > rtnl_bridge_getlink() (based on the old br_dump_ifinfo()). > > When the starting index is non-zero, we need to increment the current > index for each entry that we are skipping. Also, we need to check the > index before both cases, since we may previously have stopped > iteration between getting information about a device from its master > and from itself. > > Signed-off-by: Ben Hutchings <bhutchings@solarflare.com> > --- I needed to be testing with more interfaces. Its clearly broke with >41 veth devices. This patch fixes it thanks a lot Ben! Tested-by: John Fastabend <john.r.fastabend@intel.com> -- 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: Fri, 02 Nov 2012 18:42:03 -0700 > On 11/2/2012 3:56 PM, Ben Hutchings wrote: >> Commit e5a55a898720096f43bc24938f8875c0a1b34cd7 ('net: create generic >> bridge ops') broke the handling of a non-zero starting index in >> rtnl_bridge_getlink() (based on the old br_dump_ifinfo()). >> >> When the starting index is non-zero, we need to increment the current >> index for each entry that we are skipping. Also, we need to check the >> index before both cases, since we may previously have stopped >> iteration between getting information about a device from its master >> and from itself. >> >> Signed-off-by: Ben Hutchings <bhutchings@solarflare.com> >> --- > > I needed to be testing with more interfaces. Its clearly broke > with >41 veth devices. This patch fixes it thanks a lot Ben! > > Tested-by: John Fastabend <john.r.fastabend@intel.com> Applied, thanks everyone. -- 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 51dc58f..a0e35076 100644 --- a/net/core/rtnetlink.c +++ b/net/core/rtnetlink.c @@ -2315,28 +2315,19 @@ static int rtnl_bridge_getlink(struct sk_buff *skb, struct netlink_callback *cb) const struct net_device_ops *ops = dev->netdev_ops; struct net_device *master = dev->master; - if (idx < cb->args[0]) - continue; - if (master && master->netdev_ops->ndo_bridge_getlink) { - const struct net_device_ops *bops = master->netdev_ops; - int err = bops->ndo_bridge_getlink(skb, portid, - seq, dev); - - if (err < 0) + if (idx >= cb->args[0] && + master->netdev_ops->ndo_bridge_getlink( + skb, portid, seq, dev) < 0) break; - else - idx++; + idx++; } if (ops->ndo_bridge_getlink) { - int err = ops->ndo_bridge_getlink(skb, portid, - seq, dev); - - if (err < 0) + if (idx >= cb->args[0] && + ops->ndo_bridge_getlink(skb, portid, seq, dev) < 0) break; - else - idx++; + idx++; } } rcu_read_unlock();
Commit e5a55a898720096f43bc24938f8875c0a1b34cd7 ('net: create generic bridge ops') broke the handling of a non-zero starting index in rtnl_bridge_getlink() (based on the old br_dump_ifinfo()). When the starting index is non-zero, we need to increment the current index for each entry that we are skipping. Also, we need to check the index before both cases, since we may previously have stopped iteration between getting information about a device from its master and from itself. Signed-off-by: Ben Hutchings <bhutchings@solarflare.com> --- This is compile-tested only, as I don't know what userland code to test with. I'm still not sure it's correct to break iteration if ndo_bridge_getlink returns -EOPNOTSUPP (possible if the driver supports a mixture of bridging and non-bridging devices). Ben. net/core/rtnetlink.c | 23 +++++++---------------- 1 files changed, 7 insertions(+), 16 deletions(-)