* [PATCH 3.17 100/122] net/ping: handle protocol mismatching scenario
From: Greg Kroah-Hartman @ 2014-12-05 22:44 UTC (permalink / raw)
To: linux-kernel
Cc: Greg Kroah-Hartman, stable, David S. Miller, Alexey Kuznetsov,
James Morris, Hideaki YOSHIFUJI, Patrick McHardy, netdev,
Jane Zhou, Yiwei Zhao
In-Reply-To: <20141205223305.514276242@linuxfoundation.org>
3.17-stable review patch. If anyone has any objections, please let me know.
------------------
From: Jane Zhou <a17711@motorola.com>
commit 91a0b603469069cdcce4d572b7525ffc9fd352a6 upstream.
ping_lookup() may return a wrong sock if sk_buff's and sock's protocols
dont' match. For example, sk_buff's protocol is ETH_P_IPV6, but sock's
sk_family is AF_INET, in that case, if sk->sk_bound_dev_if is zero, a wrong
sock will be returned.
the fix is to "continue" the searching, if no matching, return NULL.
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Alexey Kuznetsov <kuznet@ms2.inr.ac.ru>
Cc: James Morris <jmorris@namei.org>
Cc: Hideaki YOSHIFUJI <yoshfuji@linux-ipv6.org>
Cc: Patrick McHardy <kaber@trash.net>
Cc: netdev@vger.kernel.org
Signed-off-by: Jane Zhou <a17711@motorola.com>
Signed-off-by: Yiwei Zhao <gbjc64@motorola.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
net/ipv4/ping.c | 2 ++
1 file changed, 2 insertions(+)
--- a/net/ipv4/ping.c
+++ b/net/ipv4/ping.c
@@ -217,6 +217,8 @@ static struct sock *ping_lookup(struct n
&ipv6_hdr(skb)->daddr))
continue;
#endif
+ } else {
+ continue;
}
if (sk->sk_bound_dev_if && sk->sk_bound_dev_if != dif)
^ permalink raw reply
* [PATCH 3.14 64/73] net/ping: handle protocol mismatching scenario
From: Greg Kroah-Hartman @ 2014-12-05 22:45 UTC (permalink / raw)
To: linux-kernel
Cc: Greg Kroah-Hartman, stable, David S. Miller, Alexey Kuznetsov,
James Morris, Hideaki YOSHIFUJI, Patrick McHardy, netdev,
Jane Zhou, Yiwei Zhao
In-Reply-To: <20141205224433.921659956@linuxfoundation.org>
3.14-stable review patch. If anyone has any objections, please let me know.
------------------
From: Jane Zhou <a17711@motorola.com>
commit 91a0b603469069cdcce4d572b7525ffc9fd352a6 upstream.
ping_lookup() may return a wrong sock if sk_buff's and sock's protocols
dont' match. For example, sk_buff's protocol is ETH_P_IPV6, but sock's
sk_family is AF_INET, in that case, if sk->sk_bound_dev_if is zero, a wrong
sock will be returned.
the fix is to "continue" the searching, if no matching, return NULL.
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Alexey Kuznetsov <kuznet@ms2.inr.ac.ru>
Cc: James Morris <jmorris@namei.org>
Cc: Hideaki YOSHIFUJI <yoshfuji@linux-ipv6.org>
Cc: Patrick McHardy <kaber@trash.net>
Cc: netdev@vger.kernel.org
Signed-off-by: Jane Zhou <a17711@motorola.com>
Signed-off-by: Yiwei Zhao <gbjc64@motorola.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
net/ipv4/ping.c | 2 ++
1 file changed, 2 insertions(+)
--- a/net/ipv4/ping.c
+++ b/net/ipv4/ping.c
@@ -217,6 +217,8 @@ static struct sock *ping_lookup(struct n
&ipv6_hdr(skb)->daddr))
continue;
#endif
+ } else {
+ continue;
}
if (sk->sk_bound_dev_if && sk->sk_bound_dev_if != dif)
^ permalink raw reply
* RE: [PATCH 2/3] bridge: offload bridge port attributes to switch asic if feature flag set
From: Arad, Ronen @ 2014-12-05 23:21 UTC (permalink / raw)
To: Roopa Prabhu, Scott Feldman, Netdev
Cc: Jirí Pírko, Jamal Hadi Salim, Benjamin LaHaise,
Thomas Graf, john fastabend, stephen@networkplumber.org,
John Linville, nhorman@tuxdriver.com, Nicolas Dichtel,
vyasevic@redhat.com, Florian Fainelli, buytenh@wantstofly.org,
Aviad Raveh, David S. Miller, shm@cumulusnetworks.com,
Andy Gospodarek
In-Reply-To: <54815883.80909@cumulusnetworks.com>
> -----Original Message-----
> From: netdev-owner@vger.kernel.org [mailto:netdev-
> owner@vger.kernel.org] On Behalf Of Roopa Prabhu
> Sent: Thursday, December 04, 2014 11:02 PM
> To: Scott Feldman
> Cc: Jiří Pírko; Jamal Hadi Salim; Benjamin LaHaise; Thomas Graf; john
> fastabend; stephen@networkplumber.org; John Linville;
> nhorman@tuxdriver.com; Nicolas Dichtel; vyasevic@redhat.com; Florian
> Fainelli; buytenh@wantstofly.org; Aviad Raveh; Netdev; David S. Miller;
> shm@cumulusnetworks.com; Andy Gospodarek
> Subject: Re: [PATCH 2/3] bridge: offload bridge port attributes to switch asic
> if feature flag set
>
> On 12/4/14, 10:41 PM, Scott Feldman wrote:
> > On Thu, Dec 4, 2014 at 6:26 PM, <roopa@cumulusnetworks.com> wrote:
> >> From: Roopa Prabhu <roopa@cumulusnetworks.com>
> >>
> >> This allows offloading to switch asic without having the user to set
> >> any flag. And this is done in the bridge driver to rollback kernel
> >> settings on hw offload failure if required in the future.
> >>
> >> With this, it also makes sure a notification goes out only after the
> >> attributes are set both in the kernel and hw.
> > I like this approach as it streamlines the steps for the user in
> > setting port flags. There is one case for FLOODING where you'll have
> > to turn off flooding for both, and then turn on flooding in hw. You
> > don't want flooding turned on on kernel and hw.
> ok, maybe using the higher bits as in
> https://patchwork.ozlabs.org/patch/413211/
>
> might help with that. Let me think some more.
> >
> >> ---
> >> net/bridge/br_netlink.c | 27 ++++++++++++++++++++++++++-
> >> 1 file changed, 26 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/net/bridge/br_netlink.c b/net/bridge/br_netlink.c index
> >> 9f5eb55..ce173f0 100644
> >> --- a/net/bridge/br_netlink.c
> >> +++ b/net/bridge/br_netlink.c
> >> @@ -407,9 +407,21 @@ int br_setlink(struct net_device *dev, struct
> nlmsghdr *nlh)
> >> afspec, RTM_SETLINK);
> >> }
> >>
> >> + if ((dev->features & NETIF_F_HW_SWITCH_OFFLOAD) &&
> >> + dev->netdev_ops->ndo_bridge_setlink) {
> >> + int ret = dev->netdev_ops->ndo_bridge_setlink(dev,
> >> + nlh);
> > I think you want to up-level this to net/core/rtnetlink.c because
> > you're only enabling the feature for one instance of a driver that
> > implements ndo_bridge_setlink: the bridge driver. If another driver
> > was MASTER and implemented ndo_bridge_setlink, you'd want same check
> > to push setting down to SELF port driver.
>
> yeah, i thought about that. But i moved it here so that rollback would be
> easier.
There is a need for propagating setlink/dellink requests down multiple levels.
The use-case I have in mind is a bridge at the top, team/bond in the middle, and port devices at the bottom.
A setlink for VLAN filtering attributes would come with MASTER flag set, and either port or bond/team netdev.
How would this be handled?
The propagation rules between bridge and enslaved port device could be different from those between bond/team and enslaved devices.
The current bridge driver does not propagate VLAN filtering from bridge to its ports as each port could have different configuration. In a case of a bond/team all members need to have the same configuration such that the a bond/team would be indistinguishable from a simple port.
Therefore rtnetlink.c might not have the knowledge for propagation across multiple levels.
It seems that each device which implements ndo_bridge_setlink/ndo_bridge_dellink and could have master role, need to take care of propagation to its slaves.
> >
> >> + if (ret && ret != -EOPNOTSUPP) {
> >> + /* XXX Fix this in the future to rollback
> >> + * kernel settings and return error
> >> + */
> > The future is now. Let's fix this now for the rollback case (again up
> > in rtnetlink.c). So then a general question comes to mind: for these
> > dual target sets, is it best to try HW first and then SW, or the other
> > way around? Either way, on failure on second you need to rollback
> > first. And, on failure, you need to know rollback value for first, so
> > you have to do a getlink on first before attempting set.
> yep, exactly, I went through the same thought process yesterday when i was
> trying to implement rollback.
> >
> >> + br_warn(p->br, "error offloading bridge attributes "
> >> + "on port %u(%s)\n", (unsigned int) p->port_no,
> >> + p->dev->name);
> >> + }
> >> + }
> >> +
> >> if (err == 0)
> >> br_ifinfo_notify(RTM_NEWLINK, p);
> >> -
> >> out:
> >> return err;
> >> }
> >> @@ -433,6 +445,19 @@ int br_dellink(struct net_device *dev, struct
> nlmsghdr *nlh)
> >> err = br_afspec((struct net_bridge *)netdev_priv(dev), p,
> >> afspec, RTM_DELLINK);
> >>
> >> + if (dev->features & NETIF_F_HW_SWITCH_OFFLOAD
> >> + && dev->netdev_ops->ndo_bridge_setlink) {
> >> + int ret = dev->netdev_ops->ndo_bridge_dellink(dev, nlh);
> >> + if (ret && ret != -EOPNOTSUPP) {
> >> + /* XXX Fix this in the future to rollback
> >> + * kernel settings and return error
> >> + */
> >> + br_warn(p->br, "error offloading bridge attributes "
> >> + "on port %u(%s)\n", (unsigned int) p->port_no,
> >> + p->dev->name);
> >> + }
> >> + }
> >> +
> > Same comments as setlink above.
> >
> >> return err;
> >> }
> >> static int br_validate(struct nlattr *tb[], struct nlattr *data[])
> >> --
> >> 1.7.10.4
> >>
>
> --
> 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
^ permalink raw reply
* iproute2/nstat: Bug in displaying icmp stats
From: Vijay Subramanian @ 2014-12-05 23:35 UTC (permalink / raw)
To: netdev
Hi,
I noticed nstat is displaying icmp stats incorrectly.
$ cat /proc/net/snmp | grep Icmp | head -2 | awk '{print $1 " " $2 " "
$3 " " $4}'
Icmp: InMsgs InErrors InCsumErrors
Icmp: 215 0 0
$ nstat -az | grep IcmpIn | head -3
IcmpInMsgs 0 0.0
IcmpInErrors 215 0.0
IcmpInCsumErrors 0 0.0
For example, as seen in /proc/net/snmp, IcmpInMsgs should be 215 but
that value is assigned to IcmpInErrors.
The issue seems to be the way the values are populated.
$vim +209 misc/nstat.c
-----x----
/* Trick to skip "dummy" trailing ICMP MIB in 2.4 */
if (strcmp(idbuf, "IcmpOutAddrMaskReps") == 0)
idbuf[5] = 0;
else
n = n->next;
-----x------
It seems "IcmpOutAddrMaskReps" is processed twice and values assigned
are off by one.
Any idea what the code is doing for 2.4 kernel and how to fix this?
vijay
^ permalink raw reply
* Re: KVM vs Xen-PV netperf numbers
From: Nick H @ 2014-12-05 23:36 UTC (permalink / raw)
To: Madhu Challa; +Cc: netdev
In-Reply-To: <CAN_zqfP-=8su7hdnd3HCjU++bWn_Qmom=5Zyf9-=iy_DJkAd2Q@mail.gmail.com>
Please do not top post..
Comments inline:
On Fri, Dec 5, 2014 at 12:20 PM, Madhu Challa <challa@noironetworks.com> wrote:
> Could you please attach your kvm command line. If you are running ubuntu you
> might also want to verify you have
>
> # To load the vhost_net module, which in some cases can speed up
> # network performance, set VHOST_NET_ENABLED to 1.
> VHOST_NET_ENABLED=1
>
> in /etc/default/qemu-kvm
sudo qemu-system-x86_64 -hda vdisk.img -smp 4 -netdev
type=tap,vhost=on,script=/usr/bin/qemu-ifup,id=net0 -device
virtio-net-pci,netdev=net0,mac=00:AD:44:44:CB:02 -m 8192
N
>
> Thanks.
>
> On Fri, Dec 5, 2014 at 12:02 PM, Nick H <nickkvm@gmail.com> wrote:
>>
>> Hello
>>
>> Not sure I have the right audience, I have two VM's on similar hosts.
>> One VM is KVM + virtio + vhost based while other is Xen PV with xen
>> netfront driver. Running a simple netperf test for 1400 byte packets
>> on both VM's, I see wider difference between throughput as follows.
>>
>> Xen-pv throughput comes to :
>>
>> ./netperf -H 10.xx.xx.49 -l 20 -t UDP_STREAM -- -m 1400
>> MIGRATED UDP STREAM TEST from 0.0.0.0 (0.0.0.0) port 0 AF_INET to
>> 10.xx.xx.49 (10.xx.xx.49) port 0 AF_INET : demo
>> Socket Message Elapsed Messages
>> Size Size Time Okay Errors Throughput
>> bytes bytes secs # # 10^6bits/sec
>>
>> 212992 1400 20.00 1910549 0 1069.89
>> 262144 20.00 1704789 954.66
>>
>> whereas KVM virtio number comes to :
>>
>> ./netperf -t UDP_STREAM -l 10 -H 10.xx.xx.49 -- -m 1400
>> MIGRATED UDP STREAM TEST from 0.0.0.0 (0.0.0.0) port 0 AF_INET to
>> 10.xx.xx.49 (10.xx.xx.49) port 0 AF_INET : demo
>> Socket Message Elapsed Messages
>> Size Size Time Okay Errors Throughput
>> bytes bytes secs # # 10^6bits/sec
>>
>> 212992 1400 10.00 155060 0 173.65
>> 262144 10.00 155060 173.65
>>
>> I built a custom kernel where I simply free up the (UDP only) skb in
>> virtio: xmit_skb() routine and I count how many skb's I have received.
>> Surprisingly it was not too high either:
>>
>> ./netperf -t UDP_STREAM -l 10 -H 10.xx.xx.49 -- -m 1400
>> MIGRATED UDP STREAM TEST from 0.0.0.0 (0.0.0.0) port 0 AF_INET to
>> 10.xx.xx.49 (10.xx.xx.49) port 0 AF_INET : demo
>> Socket Message Elapsed Messages
>> Size Size Time Okay Errors Throughput
>> bytes bytes secs # # 10^6bits/sec
>>
>> 212992 1400 10.00 224792 0 251.74
>> 262144 10.00 0 0.00
>>
>>
>> 1910549 packets pumped in xen pv driver versus 212992 packets pumped
>> in case of virtio driver. Assuming the data path inside the kernel is
>> same for both drivers , and I have eliminated virtio's
>> virtqueue_kick() call by freeing the packet ahead in my experiment,
>> can all this overhead attributed to system call overhead in case of
>> KVM+virtio combination ? Anything I am missing ?
>>
>> The KVM setup is based off:
>> Linux ubn-nested 3.17.0+ #16 SMP Thu Dec 4 12:00:09 PST 2014 x86_64
>> x86_64 x86_64 GNU/Linux
>>
>> Regards
>> N
>> --
>> 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
>
>
^ permalink raw reply
* [PATCH] dummy: add support for ethtool get_drvinfo
From: Flavio Leitner @ 2014-12-06 0:13 UTC (permalink / raw)
To: netdev; +Cc: Flavio Leitner
The command 'ethtool -i' is useful to find details
about the interface like the device driver being used.
This was missing for dummy driver.
Signed-off-by: Flavio Leitner <fbl@redhat.com>
---
drivers/net/dummy.c | 19 +++++++++++++++++--
1 file changed, 17 insertions(+), 2 deletions(-)
diff --git a/drivers/net/dummy.c b/drivers/net/dummy.c
index ff435fb..413ca4f 100644
--- a/drivers/net/dummy.c
+++ b/drivers/net/dummy.c
@@ -38,6 +38,9 @@
#include <net/rtnetlink.h>
#include <linux/u64_stats_sync.h>
+#define DRV_NAME "dummy"
+#define DRV_VERSION "1.0"
+
static int numdummies = 1;
/* fake multicast ability */
@@ -120,12 +123,24 @@ static const struct net_device_ops dummy_netdev_ops = {
.ndo_change_carrier = dummy_change_carrier,
};
+static void dummy_get_drvinfo(struct net_device *dev,
+ struct ethtool_drvinfo *info)
+{
+ strlcpy(info->driver, DRV_NAME, sizeof(info->driver));
+ strlcpy(info->version, DRV_VERSION, sizeof(info->version));
+}
+
+static const struct ethtool_ops dummy_ethtool_ops = {
+ .get_drvinfo = dummy_get_drvinfo,
+};
+
static void dummy_setup(struct net_device *dev)
{
ether_setup(dev);
/* Initialize the device structure. */
dev->netdev_ops = &dummy_netdev_ops;
+ dev->ethtool_ops = &dummy_ethtool_ops;
dev->destructor = free_netdev;
/* Fill in device structure with ethernet-generic values. */
@@ -150,7 +165,7 @@ static int dummy_validate(struct nlattr *tb[], struct nlattr *data[])
}
static struct rtnl_link_ops dummy_link_ops __read_mostly = {
- .kind = "dummy",
+ .kind = DRV_NAME,
.setup = dummy_setup,
.validate = dummy_validate,
};
@@ -209,4 +224,4 @@ static void __exit dummy_cleanup_module(void)
module_init(dummy_init_module);
module_exit(dummy_cleanup_module);
MODULE_LICENSE("GPL");
-MODULE_ALIAS_RTNL_LINK("dummy");
+MODULE_ALIAS_RTNL_LINK(DRV_NAME);
--
1.9.3
^ permalink raw reply related
* [PATCH v2 iproute] bridge link: add option 'self'
From: roopa @ 2014-12-06 0:59 UTC (permalink / raw)
To: jiri, sfeldma, jhs, bcrl, tgraf, john.fastabend, stephen,
linville, vyasevic
Cc: netdev, davem, shm, gospo, Roopa Prabhu
From: Roopa Prabhu <roopa@cumulusnetworks.com>
Currently self is set internally only if hwmode is set.
This makes it necessary for the hw to have a mode.
There is no hwmode really required to go to hardware. So, introduce
self for anybody who wants to target hardware.
v1 -> v2
- fix a few bugs. Initialize flags to zero: this was required to
keep the current behaviour unchanged.
Signed-off-by: Roopa Prabhu <roopa@cumulusnetworks.com>
---
bridge/link.c | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/bridge/link.c b/bridge/link.c
index 90d9e7f..b8b8675 100644
--- a/bridge/link.c
+++ b/bridge/link.c
@@ -261,7 +261,7 @@ static int brlink_modify(int argc, char **argv)
__s16 priority = -1;
__s8 state = -1;
__s16 mode = -1;
- __u16 flags = BRIDGE_FLAGS_MASTER;
+ __u16 flags = 0;
struct rtattr *nest;
memset(&req, 0, sizeof(req));
@@ -321,6 +321,8 @@ static int brlink_modify(int argc, char **argv)
"\"veb\".\n");
exit(-1);
}
+ } else if (strcmp(*argv, "self") == 0) {
+ flags = BRIDGE_FLAGS_SELF;
} else {
usage();
}
@@ -375,10 +377,11 @@ static int brlink_modify(int argc, char **argv)
* devices so far. Thus we only need to include the flags attribute
* if we are setting the hw mode.
*/
- if (mode >= 0) {
+ if (mode >= 0 || flags > 0) {
nest = addattr_nest(&req.n, sizeof(req), IFLA_AF_SPEC);
- addattr16(&req.n, sizeof(req), IFLA_BRIDGE_FLAGS, flags);
+ if (flags > 0)
+ addattr16(&req.n, sizeof(req), IFLA_BRIDGE_FLAGS, flags);
if (mode >= 0)
addattr16(&req.n, sizeof(req), IFLA_BRIDGE_MODE, mode);
--
1.7.10.4
^ permalink raw reply related
* [PATCH iproute2] ss: Use nl_proto_a2n for filtering by netlink proto
From: Vadim Kochan @ 2014-12-06 0:52 UTC (permalink / raw)
To: netdev; +Cc: Vadim Kochan
Now it is posible to filter by existing Netlink protos:
ss -A netlink src uevent
ss -A netlink src nft
ss -A netlink src genl
Signed-off-by: Vadim Kochan <vadim4j@gmail.com>
---
misc/ss.c | 12 ++----------
1 file changed, 2 insertions(+), 10 deletions(-)
diff --git a/misc/ss.c b/misc/ss.c
index a99294d..b9dbfd6 100644
--- a/misc/ss.c
+++ b/misc/ss.c
@@ -1231,16 +1231,8 @@ void *parse_hostcond(char *addr)
}
if (addr[0] && strcmp(addr, "*")) {
a.addr.bitlen = 32;
- if (get_u32(a.addr.data, addr, 0)) {
- if (strcmp(addr, "rtnl") == 0)
- a.addr.data[0] = 0;
- else if (strcmp(addr, "fw") == 0)
- a.addr.data[0] = 3;
- else if (strcmp(addr, "tcpdiag") == 0)
- a.addr.data[0] = 4;
- else
- return NULL;
- }
+ if (nl_proto_a2n(&a.addr.data[0], addr) == -1)
+ return NULL;
}
goto out;
}
--
2.1.3
^ permalink raw reply related
* RE: [PATCH 2/3] bridge: offload bridge port attributes to switch asic if feature flag set
From: Arad, Ronen @ 2014-12-06 1:04 UTC (permalink / raw)
To: Arad, Ronen, Roopa Prabhu, Scott Feldman, Netdev
Cc: Jirí Pírko, Jamal Hadi Salim, Benjamin LaHaise,
Thomas Graf, john fastabend, stephen@networkplumber.org,
John Linville, nhorman@tuxdriver.com, Nicolas Dichtel,
vyasevic@redhat.com, Florian Fainelli, buytenh@wantstofly.org,
Aviad Raveh, David S. Miller, shm@cumulusnetworks.com,
Andy Gospodarek
In-Reply-To: <E4CD12F19ABA0C4D8729E087A761DC3505D842BA@ORSMSX101.amr.corp.intel.com>
I have another case of propagation which is not covered by the proposed patch.
A recent patch introduced default_pvid attribute for a bridge (so far supported only via sysfs and not via netlink).
When a port joins a bridge, it inherits a PVID from the default_pvid of the bridge.
The bridge driver propagates that to the newly created net_bridge_port. This is done in br_vlan.c:
int nbp_vlan_init(struct net_bridge_port *p)
{
int rc = 0;
if (p->br->default_pvid) {
rc = nbp_vlan_add(p, p->br->default_pvid,
BRIDGE_VLAN_INFO_PVID |
BRIDGE_VLAN_INFO_UNTAGGED);
}
return rc;
}
When L2 switching is offloaded to the HW, this PVID setting need to be propagated. However, it does not come via ndo_bridge_setlink. The proposed propagation at br_setlink or an up level one at rtnetlink are not capable of handling this case.
One possible way for handling that is to replace the call to nbp_vlan_add with a call to a new function let's say
int br_propagate_vlan_add(struct net_bridge_port *port, u16 vid, u16 flags)
This function will compose a netlink message with VLAN filtering information (i.e. AF_SPEC with VLAN_INFO) and call br_setlink - leveraging the offload support proposed by Roopa.
If this is an acceptable course of action, I could work on such patch.
> -----Original Message-----
> From: netdev-owner@vger.kernel.org [mailto:netdev-
> owner@vger.kernel.org] On Behalf Of Arad, Ronen
> Sent: Friday, December 05, 2014 3:21 PM
> To: Roopa Prabhu; Scott Feldman; Netdev
> Cc: Jirí Pírko; Jamal Hadi Salim; Benjamin LaHaise; Thomas Graf; john
> fastabend; stephen@networkplumber.org; John Linville;
> nhorman@tuxdriver.com; Nicolas Dichtel; vyasevic@redhat.com; Florian
> Fainelli; buytenh@wantstofly.org; Aviad Raveh; David S. Miller;
> shm@cumulusnetworks.com; Andy Gospodarek
> Subject: RE: [PATCH 2/3] bridge: offload bridge port attributes to switch asic
> if feature flag set
>
>
>
> > -----Original Message-----
> > From: netdev-owner@vger.kernel.org [mailto:netdev-
> > owner@vger.kernel.org] On Behalf Of Roopa Prabhu
> > Sent: Thursday, December 04, 2014 11:02 PM
> > To: Scott Feldman
> > Cc: Jiří Pírko; Jamal Hadi Salim; Benjamin LaHaise; Thomas Graf; john
> > fastabend; stephen@networkplumber.org; John Linville;
> > nhorman@tuxdriver.com; Nicolas Dichtel; vyasevic@redhat.com; Florian
> > Fainelli; buytenh@wantstofly.org; Aviad Raveh; Netdev; David S.
> > Miller; shm@cumulusnetworks.com; Andy Gospodarek
> > Subject: Re: [PATCH 2/3] bridge: offload bridge port attributes to
> > switch asic if feature flag set
> >
> > On 12/4/14, 10:41 PM, Scott Feldman wrote:
> > > On Thu, Dec 4, 2014 at 6:26 PM, <roopa@cumulusnetworks.com> wrote:
> > >> From: Roopa Prabhu <roopa@cumulusnetworks.com>
> > >>
> > >> This allows offloading to switch asic without having the user to
> > >> set any flag. And this is done in the bridge driver to rollback
> > >> kernel settings on hw offload failure if required in the future.
> > >>
> > >> With this, it also makes sure a notification goes out only after
> > >> the attributes are set both in the kernel and hw.
> > > I like this approach as it streamlines the steps for the user in
> > > setting port flags. There is one case for FLOODING where you'll
> > > have to turn off flooding for both, and then turn on flooding in hw.
> > > You don't want flooding turned on on kernel and hw.
> > ok, maybe using the higher bits as in
> > https://patchwork.ozlabs.org/patch/413211/
> >
> > might help with that. Let me think some more.
> > >
> > >> ---
> > >> net/bridge/br_netlink.c | 27 ++++++++++++++++++++++++++-
> > >> 1 file changed, 26 insertions(+), 1 deletion(-)
> > >>
> > >> diff --git a/net/bridge/br_netlink.c b/net/bridge/br_netlink.c
> > >> index
> > >> 9f5eb55..ce173f0 100644
> > >> --- a/net/bridge/br_netlink.c
> > >> +++ b/net/bridge/br_netlink.c
> > >> @@ -407,9 +407,21 @@ int br_setlink(struct net_device *dev, struct
> > nlmsghdr *nlh)
> > >> afspec, RTM_SETLINK);
> > >> }
> > >>
> > >> + if ((dev->features & NETIF_F_HW_SWITCH_OFFLOAD) &&
> > >> + dev->netdev_ops->ndo_bridge_setlink) {
> > >> + int ret = dev->netdev_ops->ndo_bridge_setlink(dev,
> > >> + nlh);
> > > I think you want to up-level this to net/core/rtnetlink.c because
> > > you're only enabling the feature for one instance of a driver that
> > > implements ndo_bridge_setlink: the bridge driver. If another driver
> > > was MASTER and implemented ndo_bridge_setlink, you'd want same
> check
> > > to push setting down to SELF port driver.
> >
> > yeah, i thought about that. But i moved it here so that rollback would
> > be easier.
>
> There is a need for propagating setlink/dellink requests down multiple levels.
> The use-case I have in mind is a bridge at the top, team/bond in the middle,
> and port devices at the bottom.
> A setlink for VLAN filtering attributes would come with MASTER flag set, and
> either port or bond/team netdev.
> How would this be handled?
>
> The propagation rules between bridge and enslaved port device could be
> different from those between bond/team and enslaved devices.
> The current bridge driver does not propagate VLAN filtering from bridge to its
> ports as each port could have different configuration. In a case of a
> bond/team all members need to have the same configuration such that the a
> bond/team would be indistinguishable from a simple port.
>
> Therefore rtnetlink.c might not have the knowledge for propagation across
> multiple levels.
> It seems that each device which implements
> ndo_bridge_setlink/ndo_bridge_dellink and could have master role, need to
> take care of propagation to its slaves.
>
> > >
> > >> + if (ret && ret != -EOPNOTSUPP) {
> > >> + /* XXX Fix this in the future to rollback
> > >> + * kernel settings and return error
> > >> + */
> > > The future is now. Let's fix this now for the rollback case (again
> > > up in rtnetlink.c). So then a general question comes to mind: for
> > > these dual target sets, is it best to try HW first and then SW, or
> > > the other way around? Either way, on failure on second you need to
> > > rollback first. And, on failure, you need to know rollback value
> > > for first, so you have to do a getlink on first before attempting set.
> > yep, exactly, I went through the same thought process yesterday when i
> > was trying to implement rollback.
> > >
> > >> + br_warn(p->br, "error offloading bridge attributes "
> > >> + "on port %u(%s)\n", (unsigned int) p->port_no,
> > >> + p->dev->name);
> > >> + }
> > >> + }
> > >> +
> > >> if (err == 0)
> > >> br_ifinfo_notify(RTM_NEWLINK, p);
> > >> -
> > >> out:
> > >> return err;
> > >> }
> > >> @@ -433,6 +445,19 @@ int br_dellink(struct net_device *dev, struct
> > nlmsghdr *nlh)
> > >> err = br_afspec((struct net_bridge *)netdev_priv(dev), p,
> > >> afspec, RTM_DELLINK);
> > >>
> > >> + if (dev->features & NETIF_F_HW_SWITCH_OFFLOAD
> > >> + && dev->netdev_ops->ndo_bridge_setlink) {
> > >> + int ret = dev->netdev_ops->ndo_bridge_dellink(dev, nlh);
> > >> + if (ret && ret != -EOPNOTSUPP) {
> > >> + /* XXX Fix this in the future to rollback
> > >> + * kernel settings and return error
> > >> + */
> > >> + br_warn(p->br, "error offloading bridge attributes "
> > >> + "on port %u(%s)\n", (unsigned int) p->port_no,
> > >> + p->dev->name);
> > >> + }
> > >> + }
> > >> +
> > > Same comments as setlink above.
> > >
> > >> return err;
> > >> }
> > >> static int br_validate(struct nlattr *tb[], struct nlattr
> > >> *data[])
> > >> --
> > >> 1.7.10.4
> > >>
> >
> > --
> > 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
> \x04 {.n + +% lzwm b 맲 r zw u ^n r z \x1a h & \x1e G h \x03( 階 ݢj" \x1a ^[m z ޖ
> f h ~ m
^ permalink raw reply
* Re: iproute2/nstat: Bug in displaying icmp stats
From: Eric Dumazet @ 2014-12-06 1:10 UTC (permalink / raw)
To: Vijay Subramanian; +Cc: netdev
In-Reply-To: <CAGK4HS_ty2=f0PNU-w8QPP9BoP67E+4MaDxLTLV4G22dx_7A-Q@mail.gmail.com>
On Fri, 2014-12-05 at 15:35 -0800, Vijay Subramanian wrote:
> Hi,
>
> I noticed nstat is displaying icmp stats incorrectly.
>
> $ cat /proc/net/snmp | grep Icmp | head -2 | awk '{print $1 " " $2 " "
> $3 " " $4}'
> Icmp: InMsgs InErrors InCsumErrors
> Icmp: 215 0 0
>
> $ nstat -az | grep IcmpIn | head -3
> IcmpInMsgs 0 0.0
> IcmpInErrors 215 0.0
> IcmpInCsumErrors 0 0.0
>
> For example, as seen in /proc/net/snmp, IcmpInMsgs should be 215 but
> that value is assigned to IcmpInErrors.
>
> The issue seems to be the way the values are populated.
>
> $vim +209 misc/nstat.c
> -----x----
>
> /* Trick to skip "dummy" trailing ICMP MIB in 2.4 */
>
> if (strcmp(idbuf, "IcmpOutAddrMaskReps") == 0)
>
> idbuf[5] = 0;
>
> else
>
> n = n->next;
>
> -----x------
>
> It seems "IcmpOutAddrMaskReps" is processed twice and values assigned
> are off by one.
>
> Any idea what the code is doing for 2.4 kernel and how to fix this?
>
> vijay
According to
git://git.kernel.org/pub/scm/linux/kernel/git/tglx/history.git
This was fixed in 2002 :
commit b838c6d5c189d03d3db56c2774de451cf041a39f
Author: Erik Schoenfelder <schoenfr@gaaertner.de>
Date: Sun Sep 29 03:56:52 2002 -0700
net/ipv4/proc.c: Dont print dummy member of icmp_mib.
diff --git a/net/ipv4/proc.c b/net/ipv4/proc.c
index 649620a..3f2dbcb 100644
--- a/net/ipv4/proc.c
+++ b/net/ipv4/proc.c
@@ -128,7 +128,7 @@ int snmp_get_info(char *buffer, char **start, off_t offset, int length)
len += sprintf (buffer + len,
"\nIcmp: InMsgs InErrors InDestUnreachs InTimeExcds InParmProbs InSrcQuenchs InRedirects InEchos InEchoReps InTimestamps InTimestampReps InAddrMasks InAddrMaskReps OutMsgs OutErrors OutDestUnreachs OutTimeExcds OutParmProbs OutSrcQuenchs OutRedirects OutEchos OutEchoReps OutTimestamps OutTimestampReps OutAddrMasks OutAddrMaskReps\n"
"Icmp:");
- for (i=0; i<offsetof(struct icmp_mib, __pad)/sizeof(unsigned long); i++)
+ for (i=0; i<offsetof(struct icmp_mib, dummy)/sizeof(unsigned long); i++)
len += sprintf(buffer+len, " %lu", fold_field((unsigned long*)icmp_statistics, sizeof(struct icmp_mib), i));
len += sprintf (buffer + len,
^ permalink raw reply related
* Re: iproute2/nstat: Bug in displaying icmp stats
From: Eric Dumazet @ 2014-12-06 1:13 UTC (permalink / raw)
To: Vijay Subramanian; +Cc: netdev
In-Reply-To: <1417828247.15618.19.camel@edumazet-glaptop2.roam.corp.google.com>
On Fri, 2014-12-05 at 17:10 -0800, Eric Dumazet wrote:
> On Fri, 2014-12-05 at 15:35 -0800, Vijay Subramanian wrote:
> > Hi,
> >
> > I noticed nstat is displaying icmp stats incorrectly.
> >
> > $ cat /proc/net/snmp | grep Icmp | head -2 | awk '{print $1 " " $2 " "
> > $3 " " $4}'
> > Icmp: InMsgs InErrors InCsumErrors
> > Icmp: 215 0 0
> >
> > $ nstat -az | grep IcmpIn | head -3
> > IcmpInMsgs 0 0.0
> > IcmpInErrors 215 0.0
> > IcmpInCsumErrors 0 0.0
> >
> > For example, as seen in /proc/net/snmp, IcmpInMsgs should be 215 but
> > that value is assigned to IcmpInErrors.
> >
> > The issue seems to be the way the values are populated.
> >
> > $vim +209 misc/nstat.c
> > -----x----
> >
> > /* Trick to skip "dummy" trailing ICMP MIB in 2.4 */
> >
> > if (strcmp(idbuf, "IcmpOutAddrMaskReps") == 0)
> >
> > idbuf[5] = 0;
> >
> > else
> >
> > n = n->next;
> >
> > -----x------
> >
> > It seems "IcmpOutAddrMaskReps" is processed twice and values assigned
> > are off by one.
> >
> > Any idea what the code is doing for 2.4 kernel and how to fix this?
> >
> > vijay
>
> According to
> git://git.kernel.org/pub/scm/linux/kernel/git/tglx/history.git
>
> This was fixed in 2002 :
>
> commit b838c6d5c189d03d3db56c2774de451cf041a39f
> Author: Erik Schoenfelder <schoenfr@gaaertner.de>
> Date: Sun Sep 29 03:56:52 2002 -0700
>
> net/ipv4/proc.c: Dont print dummy member of icmp_mib.
>
> diff --git a/net/ipv4/proc.c b/net/ipv4/proc.c
> index 649620a..3f2dbcb 100644
> --- a/net/ipv4/proc.c
> +++ b/net/ipv4/proc.c
> @@ -128,7 +128,7 @@ int snmp_get_info(char *buffer, char **start, off_t offset, int length)
> len += sprintf (buffer + len,
> "\nIcmp: InMsgs InErrors InDestUnreachs InTimeExcds InParmProbs InSrcQuenchs InRedirects InEchos InEchoReps InTimestamps InTimestampReps InAddrMasks InAddrMaskReps OutMsgs OutErrors OutDestUnreachs OutTimeExcds OutParmProbs OutSrcQuenchs OutRedirects OutEchos OutEchoReps OutTimestamps OutTimestampReps OutAddrMasks OutAddrMaskReps\n"
> "Icmp:");
> - for (i=0; i<offsetof(struct icmp_mib, __pad)/sizeof(unsigned long); i++)
> + for (i=0; i<offsetof(struct icmp_mib, dummy)/sizeof(unsigned long); i++)
> len += sprintf(buffer+len, " %lu", fold_field((unsigned long*)icmp_statistics, sizeof(struct icmp_mib), i));
>
> len += sprintf (buffer + len,
>
I guess we could count number of spaces/fields in both lines,
and disable the iproute2 trick if counts match.
^ permalink raw reply
* [PATCH net-next v2 0/2] remove bridge BRIDGE_MODE_SWDEV
From: roopa @ 2014-12-06 1:16 UTC (permalink / raw)
To: jiri, sfeldma, jhs, bcrl, tgraf, john.fastabend, stephen,
linville, vyasevic
Cc: netdev, davem, shm, gospo, Roopa Prabhu
From: Roopa Prabhu <roopa@cumulusnetworks.com>
Roopa Prabhu (2):
bridge: remove mode 'swdev'
rocker: remove swdev mode
drivers/net/ethernet/rocker/rocker.c | 18 +-----------------
include/uapi/linux/if_bridge.h | 1 -
2 files changed, 1 insertion(+), 18 deletions(-)
--
1.7.10.4
^ permalink raw reply
* [PATCH net-next v2 1/2] bridge: remove mode 'swdev'
From: roopa @ 2014-12-06 1:16 UTC (permalink / raw)
To: jiri, sfeldma, jhs, bcrl, tgraf, john.fastabend, stephen,
linville, vyasevic
Cc: netdev, davem, shm, gospo, Roopa Prabhu
From: Roopa Prabhu <roopa@cumulusnetworks.com>
swdev mode was introduced to indicate switchdev offloads
for bridging from user space. But user can
use BRIDGE_FLAGS_SELF to directly call into the
hw switch port driver today. swdev mode is not required anymore.
Signed-off-by: Roopa Prabhu <roopa@cumulusnetworks.com>
---
include/uapi/linux/if_bridge.h | 1 -
1 file changed, 1 deletion(-)
diff --git a/include/uapi/linux/if_bridge.h b/include/uapi/linux/if_bridge.h
index 296a556..da17e45 100644
--- a/include/uapi/linux/if_bridge.h
+++ b/include/uapi/linux/if_bridge.h
@@ -105,7 +105,6 @@ struct __fdb_entry {
#define BRIDGE_MODE_VEB 0 /* Default loopback mode */
#define BRIDGE_MODE_VEPA 1 /* 802.1Qbg defined VEPA mode */
-#define BRIDGE_MODE_SWDEV 2 /* Full switch device offload */
/* Bridge management nested attributes
* [IFLA_AF_SPEC] = {
--
1.7.10.4
^ permalink raw reply related
* [PATCH net-next v2 2/2] rocker: remove swdev mode
From: roopa @ 2014-12-06 1:16 UTC (permalink / raw)
To: jiri, sfeldma, jhs, bcrl, tgraf, john.fastabend, stephen,
linville, vyasevic
Cc: netdev, davem, shm, gospo, Roopa Prabhu
From: Roopa Prabhu <roopa@cumulusnetworks.com>
This resets rocker mode to zero (vepa) during gets.
This is because the default getlink handler that rocker
uses today always takes a mode.
Will fix that in a subsequent patch.
Signed-off-by: Roopa Prabhu <roopa@cumulusnetworks.com>
---
drivers/net/ethernet/rocker/rocker.c | 18 +-----------------
1 file changed, 1 insertion(+), 17 deletions(-)
diff --git a/drivers/net/ethernet/rocker/rocker.c b/drivers/net/ethernet/rocker/rocker.c
index fded127..391077c 100644
--- a/drivers/net/ethernet/rocker/rocker.c
+++ b/drivers/net/ethernet/rocker/rocker.c
@@ -3700,27 +3700,11 @@ static int rocker_port_bridge_setlink(struct net_device *dev,
{
struct rocker_port *rocker_port = netdev_priv(dev);
struct nlattr *protinfo;
- struct nlattr *afspec;
struct nlattr *attr;
- u16 mode;
int err;
protinfo = nlmsg_find_attr(nlh, sizeof(struct ifinfomsg),
IFLA_PROTINFO);
- afspec = nlmsg_find_attr(nlh, sizeof(struct ifinfomsg), IFLA_AF_SPEC);
-
- if (afspec) {
- attr = nla_find_nested(afspec, IFLA_BRIDGE_MODE);
- if (attr) {
- if (nla_len(attr) < sizeof(mode))
- return -EINVAL;
-
- mode = nla_get_u16(attr);
- if (mode != BRIDGE_MODE_SWDEV)
- return -EINVAL;
- }
- }
-
if (protinfo) {
attr = nla_find_nested(protinfo, IFLA_BRPORT_LEARNING);
if (attr) {
@@ -3755,7 +3739,7 @@ static int rocker_port_bridge_getlink(struct sk_buff *skb, u32 pid, u32 seq,
u32 filter_mask)
{
struct rocker_port *rocker_port = netdev_priv(dev);
- u16 mode = BRIDGE_MODE_SWDEV;
+ u16 mode = 0;
u32 mask = BR_LEARNING | BR_LEARNING_SYNC;
return ndo_dflt_bridge_getlink(skb, pid, seq, dev, mode,
--
1.7.10.4
^ permalink raw reply related
* Re: iproute2/nstat: Bug in displaying icmp stats
From: Eric Dumazet @ 2014-12-06 2:10 UTC (permalink / raw)
To: Vijay Subramanian; +Cc: netdev
In-Reply-To: <1417828430.15618.20.camel@edumazet-glaptop2.roam.corp.google.com>
On Fri, 2014-12-05 at 17:13 -0800, Eric Dumazet wrote:
> I guess we could count number of spaces/fields in both lines,
> and disable the iproute2 trick if counts match.
Something like that maybe ?
misc/nstat.c | 18 ++++++++++++++++--
1 file changed, 16 insertions(+), 2 deletions(-)
diff --git a/misc/nstat.c b/misc/nstat.c
index e54b3ae..c2cb056 100644
--- a/misc/nstat.c
+++ b/misc/nstat.c
@@ -156,6 +156,15 @@ static void load_good_table(FILE *fp)
}
}
+static int count_spaces(const char *line)
+{
+ int count = 0;
+ char c;
+
+ while ((c = *line++) != 0)
+ count += c == ' ' || c == '\n';
+ return count;
+}
static void load_ugly_table(FILE *fp)
{
@@ -167,10 +176,12 @@ static void load_ugly_table(FILE *fp)
char idbuf[sizeof(buf)];
int off;
char *p;
+ int count1, count2, skip = 0;
p = strchr(buf, ':');
if (!p)
abort();
+ count1 = count_spaces(buf);
*p = 0;
idbuf[0] = 0;
strncat(idbuf, buf, sizeof(idbuf) - 1);
@@ -199,6 +210,9 @@ static void load_ugly_table(FILE *fp)
n = db;
if (fgets(buf, sizeof(buf), fp) == NULL)
abort();
+ count2 = count_spaces(buf);
+ if (count2 > count1)
+ skip = count2 - count1;
do {
p = strrchr(buf, ' ');
if (!p)
@@ -207,8 +221,8 @@ static void load_ugly_table(FILE *fp)
if (sscanf(p+1, "%llu", &n->val) != 1)
abort();
/* Trick to skip "dummy" trailing ICMP MIB in 2.4 */
- if (strcmp(idbuf, "IcmpOutAddrMaskReps") == 0)
- idbuf[5] = 0;
+ if (skip)
+ skip--;
else
n = n->next;
} while (p > buf + off + 2);
^ permalink raw reply related
* [PATCH iproute2 0/2] lib names: Refactoring and cleanups
From: Vadim Kochan @ 2014-12-06 2:05 UTC (permalink / raw)
To: netdev; +Cc: Vadim Kochan
Some cleanups and refactoring in lib/rt_names.c:
#1 Replaced using of /etc/iproute2 path by CONFDIR define
when initializing tables of group names.
#2 Added helper to have one func for parsing id and names from
db files.
Vadim Kochan (2):
lib names: Use CONFDIR for specify 'group' file path
lib names: Add helper func for parse id and name from file
lib/rt_names.c | 70 +++++++++++++++++++++++++++++++++-------------------------
1 file changed, 40 insertions(+), 30 deletions(-)
--
2.1.3
^ permalink raw reply
* [PATCH iproute2 1/2] lib names: Use CONFDIR for specify 'group' file path
From: Vadim Kochan @ 2014-12-06 2:05 UTC (permalink / raw)
To: netdev; +Cc: Vadim Kochan
In-Reply-To: <1417831512-19452-1-git-send-email-vadim4j@gmail.com>
Signed-off-by: Vadim Kochan <vadim4j@gmail.com>
---
lib/rt_names.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lib/rt_names.c b/lib/rt_names.c
index 369e0f4..e6a1e01 100644
--- a/lib/rt_names.c
+++ b/lib/rt_names.c
@@ -469,7 +469,7 @@ static int rtnl_group_init;
static void rtnl_group_initialize(void)
{
rtnl_group_init = 1;
- rtnl_hash_initialize("/etc/iproute2/group",
+ rtnl_hash_initialize(CONFDIR "/group",
rtnl_group_hash, 256);
}
--
2.1.3
^ permalink raw reply related
* [PATCH iproute2 2/2] lib names: Add helper func for parse id and name from file
From: Vadim Kochan @ 2014-12-06 2:05 UTC (permalink / raw)
To: netdev; +Cc: Vadim Kochan
In-Reply-To: <1417831512-19452-1-git-send-email-vadim4j@gmail.com>
Signed-off-by: Vadim Kochan <vadim4j@gmail.com>
---
lib/rt_names.c | 68 +++++++++++++++++++++++++++++++++-------------------------
1 file changed, 39 insertions(+), 29 deletions(-)
diff --git a/lib/rt_names.c b/lib/rt_names.c
index e6a1e01..2f14723 100644
--- a/lib/rt_names.c
+++ b/lib/rt_names.c
@@ -27,43 +27,62 @@
#define CONFDIR "/etc/iproute2"
#endif
+#define NAME_MAX_LEN 512
+
struct rtnl_hash_entry {
struct rtnl_hash_entry *next;
const char * name;
unsigned int id;
};
+static int fread_id_name(FILE *fp, int *id, char *namebuf)
+{
+ char buf[NAME_MAX_LEN];
+ while (fgets(buf, sizeof(buf), fp)) {
+ char *p = buf;
+
+ while (*p == ' ' || *p == '\t')
+ p++;
+
+ if (*p == '#' || *p == '\n' || *p == 0)
+ continue;
+
+ if (sscanf(p, "0x%x %s\n", id, namebuf) != 2 &&
+ sscanf(p, "0x%x %s #", id, namebuf) != 2 &&
+ sscanf(p, "%d %s\n", id, namebuf) != 2 &&
+ sscanf(p, "%d %s #", id, namebuf) != 2) {
+ strcpy(namebuf, p);
+ return -1;
+ }
+ return 1;
+ }
+ return 0;
+}
+
static void
rtnl_hash_initialize(const char *file, struct rtnl_hash_entry **hash, int size)
{
struct rtnl_hash_entry *entry;
- char buf[512];
FILE *fp;
+ int id;
+ char namebuf[NAME_MAX_LEN] = {0};
+ int ret;
fp = fopen(file, "r");
if (!fp)
return;
- while (fgets(buf, sizeof(buf), fp)) {
- char *p = buf;
- int id;
- char namebuf[512];
- while (*p == ' ' || *p == '\t')
- p++;
- if (*p == '#' || *p == '\n' || *p == 0)
- continue;
- if (sscanf(p, "0x%x %s\n", &id, namebuf) != 2 &&
- sscanf(p, "0x%x %s #", &id, namebuf) != 2 &&
- sscanf(p, "%d %s\n", &id, namebuf) != 2 &&
- sscanf(p, "%d %s #", &id, namebuf) != 2) {
+ while ((ret = fread_id_name(fp, &id, &namebuf[0]))) {
+ if (ret == -1) {
fprintf(stderr, "Database %s is corrupted at %s\n",
- file, p);
+ file, namebuf);
fclose(fp);
return;
}
if (id<0)
continue;
+
entry = malloc(sizeof(*entry));
entry->id = id;
entry->name = strdup(namebuf);
@@ -75,31 +94,22 @@ rtnl_hash_initialize(const char *file, struct rtnl_hash_entry **hash, int size)
static void rtnl_tab_initialize(const char *file, char **tab, int size)
{
- char buf[512];
FILE *fp;
+ int id;
+ char namebuf[NAME_MAX_LEN] = {0};
+ int ret;
fp = fopen(file, "r");
if (!fp)
return;
- while (fgets(buf, sizeof(buf), fp)) {
- char *p = buf;
- int id;
- char namebuf[512];
- while (*p == ' ' || *p == '\t')
- p++;
- if (*p == '#' || *p == '\n' || *p == 0)
- continue;
- if (sscanf(p, "0x%x %s\n", &id, namebuf) != 2 &&
- sscanf(p, "0x%x %s #", &id, namebuf) != 2 &&
- sscanf(p, "%d %s\n", &id, namebuf) != 2 &&
- sscanf(p, "%d %s #", &id, namebuf) != 2) {
+ while ((ret = fread_id_name(fp, &id, &namebuf[0]))) {
+ if (ret == -1) {
fprintf(stderr, "Database %s is corrupted at %s\n",
- file, p);
+ file, namebuf);
fclose(fp);
return;
}
-
if (id<0 || id>size)
continue;
--
2.1.3
^ permalink raw reply related
* [PATCH v2 net-next] tcp: refine TSO autosizing
From: Eric Dumazet @ 2014-12-06 2:22 UTC (permalink / raw)
To: David Miller; +Cc: netdev, Neal Cardwell, Yuchung Cheng, Nandita Dukkipati
In-Reply-To: <1417788937.4322.21.camel@edumazet-glaptop2.roam.corp.google.com>
From: Eric Dumazet <edumazet@google.com>
Commit 95bd09eb2750 ("tcp: TSO packets automatic sizing") tried to
control TSO size, but did this at the wrong place (sendmsg() time)
At sendmsg() time, we might have a pessimistic view of flow rate,
and we end up building very small skbs (with 2 MSS per skb).
This is bad because :
- It sends small TSO packets even in Slow Start where rate quickly
increases.
- It tends to make socket write queue very big, increasing tcp_ack()
processing time, but also increasing memory needs, not necessarily
accounted for, as fast clones overhead is currently ignored.
- Lower GRO efficiency and more ACK packets.
Servers with a lot of small lived connections suffer from this.
Lets instead fill skbs as much as possible (64KB of payload), but split
them at xmit time, when we have a precise idea of the flow rate.
skb split is actually quite efficient.
Patch looks bigger than necessary, because TCP Small Queue decision now
has to take place after the eventual split.
As Neal suggested, get rid of tp->xmit_size_goal_segs, and
introduce a new tcp_tso_autosize() helper, so that
tcp_tso_should_defer() can be synchronized on same goal.
Tested:
40 ms rtt link
nstat >/dev/null
netperf -H remote -Cc -l -2000000 -- -s 1000000
nstat | egrep "IpInReceives|IpOutRequests|TcpOutSegs|IpExtOutOctets"
Before patch :
Recv Send Send Utilization Service Demand
Socket Socket Message Elapsed Send Recv Send Recv
Size Size Size Time Throughput local remote local remote
bytes bytes bytes secs. 10^6bits/s % S % S us/KB us/KB
87380 2000000 2000000 0.36 44.22 0.00 0.06 0.000 5.007
IpInReceives 600 0.0
IpOutRequests 599 0.0
TcpOutSegs 1397 0.0
IpExtOutOctets 2033249 0.0
After patch :
Recv Send Send Utilization Service Demand
Socket Socket Message Elapsed Send Recv Send Recv
Size Size Size Time Throughput local remote local remote
bytes bytes bytes secs. 10^6bits/s % S % S us/KB us/KB
87380 2000000 2000000 0.36 44.09 0.00 0.00 0.000 0.000
IpInReceives 257 0.0
IpOutRequests 226 0.0
TcpOutSegs 1399 0.0
IpExtOutOctets 2013777 0.0
Signed-off-by: Eric Dumazet <edumazet@google.com>
Signed-off-by: Neal Cardwell <ncardwell@google.com>
---
v2: added tcp_tso_autosize() helper and removed tp->xmit_size_goal_segs
include/linux/tcp.h | 2 -
net/ipv4/tcp.c | 44 ++++++-----------------------
net/ipv4/tcp_output.c | 59 +++++++++++++++++++++++++++-------------
3 files changed, 51 insertions(+), 54 deletions(-)
diff --git a/include/linux/tcp.h b/include/linux/tcp.h
index f566b8567892ef0bb213de0540b37cfc6ac03ca0..a2944717f1b7d1d12b4e2397c20457931ad71bac 100644
--- a/include/linux/tcp.h
+++ b/include/linux/tcp.h
@@ -130,7 +130,7 @@ struct tcp_sock {
/* inet_connection_sock has to be the first member of tcp_sock */
struct inet_connection_sock inet_conn;
u16 tcp_header_len; /* Bytes of tcp header to send */
- u16 xmit_size_goal_segs; /* Goal for segmenting output packets */
+ /* 2 bytes hole */
/*
* Header prediction flags
diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
index dc13a3657e8e1b81ba0cb1fcd5386a9d0b106168..6e692f7ac62880415093ca7a43c782c7fd6a049b 100644
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c
@@ -835,45 +835,19 @@ static unsigned int tcp_xmit_size_goal(struct sock *sk, u32 mss_now,
int large_allowed)
{
struct tcp_sock *tp = tcp_sk(sk);
- u32 xmit_size_goal, old_size_goal;
+ u32 hlen, xmit_size_goal;
- xmit_size_goal = mss_now;
+ if (!large_allowed || !sk_can_gso(sk))
+ return mss_now;
- if (large_allowed && sk_can_gso(sk)) {
- u32 gso_size, hlen;
+ /* Maybe we should/could use sk->sk_prot->max_header here ? */
+ hlen = inet_csk(sk)->icsk_af_ops->net_header_len +
+ inet_csk(sk)->icsk_ext_hdr_len +
+ tp->tcp_header_len;
- /* Maybe we should/could use sk->sk_prot->max_header here ? */
- hlen = inet_csk(sk)->icsk_af_ops->net_header_len +
- inet_csk(sk)->icsk_ext_hdr_len +
- tp->tcp_header_len;
+ xmit_size_goal = sk->sk_gso_max_size - 1 - hlen;
- /* Goal is to send at least one packet per ms,
- * not one big TSO packet every 100 ms.
- * This preserves ACK clocking and is consistent
- * with tcp_tso_should_defer() heuristic.
- */
- gso_size = sk->sk_pacing_rate / (2 * MSEC_PER_SEC);
- gso_size = max_t(u32, gso_size,
- sysctl_tcp_min_tso_segs * mss_now);
-
- xmit_size_goal = min_t(u32, gso_size,
- sk->sk_gso_max_size - 1 - hlen);
-
- xmit_size_goal = tcp_bound_to_half_wnd(tp, xmit_size_goal);
-
- /* We try hard to avoid divides here */
- old_size_goal = tp->xmit_size_goal_segs * mss_now;
-
- if (likely(old_size_goal <= xmit_size_goal &&
- old_size_goal + mss_now > xmit_size_goal)) {
- xmit_size_goal = old_size_goal;
- } else {
- tp->xmit_size_goal_segs =
- min_t(u16, xmit_size_goal / mss_now,
- sk->sk_gso_max_segs);
- xmit_size_goal = tp->xmit_size_goal_segs * mss_now;
- }
- }
+ xmit_size_goal = tcp_bound_to_half_wnd(tp, xmit_size_goal);
return max(xmit_size_goal, mss_now);
}
diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
index f5bd4bd3f7e669b3fd48a843d55e7313a30a3409..f37ecf53ee8a96827fc08bd203b0ca8857f8fc34 100644
--- a/net/ipv4/tcp_output.c
+++ b/net/ipv4/tcp_output.c
@@ -1524,6 +1524,27 @@ static bool tcp_nagle_check(bool partial, const struct tcp_sock *tp,
((nonagle & TCP_NAGLE_CORK) ||
(!nonagle && tp->packets_out && tcp_minshall_check(tp)));
}
+
+/* Return how many segs we'd like on a TSO packet,
+ * to send one TSO packet per ms.
+ */
+static u32 tcp_tso_autosize(const struct sock *sk, unsigned int mss_now)
+{
+ u32 bytes, segs;
+
+ bytes = min(sk->sk_pacing_rate >> 10,
+ sk->sk_gso_max_size - 1 - MAX_TCP_HEADER);
+
+ /* Goal is to send at least one packet per ms,
+ * not one big TSO packet every 100 ms.
+ * This preserves ACK clocking and is consistent
+ * with tcp_tso_should_defer() heuristic.
+ */
+ segs = max_t(u32, bytes / mss_now, sysctl_tcp_min_tso_segs);
+
+ return min_t(u32, segs, sk->sk_gso_max_segs);
+}
+
/* Returns the portion of skb which can be sent right away */
static unsigned int tcp_mss_split_point(const struct sock *sk,
const struct sk_buff *skb,
@@ -1731,7 +1752,7 @@ static int tso_fragment(struct sock *sk, struct sk_buff *skb, unsigned int len,
* This algorithm is from John Heffner.
*/
static bool tcp_tso_should_defer(struct sock *sk, struct sk_buff *skb,
- bool *is_cwnd_limited)
+ bool *is_cwnd_limited, u32 max_segs)
{
struct tcp_sock *tp = tcp_sk(sk);
const struct inet_connection_sock *icsk = inet_csk(sk);
@@ -1761,8 +1782,7 @@ static bool tcp_tso_should_defer(struct sock *sk, struct sk_buff *skb,
limit = min(send_win, cong_win);
/* If a full-sized TSO skb can be sent, do it. */
- if (limit >= min_t(unsigned int, sk->sk_gso_max_size,
- tp->xmit_size_goal_segs * tp->mss_cache))
+ if (limit >= max_segs * tp->mss_cache)
goto send_now;
/* Middle in queue won't get any more data, full sendable already? */
@@ -1959,6 +1979,7 @@ static bool tcp_write_xmit(struct sock *sk, unsigned int mss_now, int nonagle,
int cwnd_quota;
int result;
bool is_cwnd_limited = false;
+ u32 max_segs;
sent_pkts = 0;
@@ -1972,6 +1993,7 @@ static bool tcp_write_xmit(struct sock *sk, unsigned int mss_now, int nonagle,
}
}
+ max_segs = tcp_tso_autosize(sk, mss_now);
while ((skb = tcp_send_head(sk))) {
unsigned int limit;
@@ -2004,10 +2026,23 @@ static bool tcp_write_xmit(struct sock *sk, unsigned int mss_now, int nonagle,
break;
} else {
if (!push_one &&
- tcp_tso_should_defer(sk, skb, &is_cwnd_limited))
+ tcp_tso_should_defer(sk, skb, &is_cwnd_limited,
+ max_segs))
break;
}
+ limit = mss_now;
+ if (tso_segs > 1 && !tcp_urg_mode(tp))
+ limit = tcp_mss_split_point(sk, skb, mss_now,
+ min_t(unsigned int,
+ cwnd_quota,
+ max_segs),
+ nonagle);
+
+ if (skb->len > limit &&
+ unlikely(tso_fragment(sk, skb, limit, mss_now, gfp)))
+ break;
+
/* TCP Small Queues :
* Control number of packets in qdisc/devices to two packets / or ~1 ms.
* This allows for :
@@ -2018,8 +2053,8 @@ static bool tcp_write_xmit(struct sock *sk, unsigned int mss_now, int nonagle,
* of queued bytes to ensure line rate.
* One example is wifi aggregation (802.11 AMPDU)
*/
- limit = max_t(unsigned int, sysctl_tcp_limit_output_bytes,
- sk->sk_pacing_rate >> 10);
+ limit = max(2 * skb->truesize, sk->sk_pacing_rate >> 10);
+ limit = min_t(u32, limit, sysctl_tcp_limit_output_bytes);
if (atomic_read(&sk->sk_wmem_alloc) > limit) {
set_bit(TSQ_THROTTLED, &tp->tsq_flags);
@@ -2032,18 +2067,6 @@ static bool tcp_write_xmit(struct sock *sk, unsigned int mss_now, int nonagle,
break;
}
- limit = mss_now;
- if (tso_segs > 1 && !tcp_urg_mode(tp))
- limit = tcp_mss_split_point(sk, skb, mss_now,
- min_t(unsigned int,
- cwnd_quota,
- sk->sk_gso_max_segs),
- nonagle);
-
- if (skb->len > limit &&
- unlikely(tso_fragment(sk, skb, limit, mss_now, gfp)))
- break;
-
if (unlikely(tcp_transmit_skb(sk, skb, 1, gfp)))
break;
^ permalink raw reply related
* Re: [PATCH 2/3] bridge: offload bridge port attributes to switch asic if feature flag set
From: John Fastabend @ 2014-12-06 2:46 UTC (permalink / raw)
To: Arad, Ronen
Cc: Roopa Prabhu, Scott Feldman, Netdev, Jirí Pírko,
Jamal Hadi Salim, Benjamin LaHaise, Thomas Graf,
stephen@networkplumber.org, John Linville, nhorman@tuxdriver.com,
Nicolas Dichtel, vyasevic@redhat.com, Florian Fainelli,
buytenh@wantstofly.org, Aviad Raveh, David S. Miller,
shm@cumulusnetworks.com, Andy Gospodarek
In-Reply-To: <E4CD12F19ABA0C4D8729E087A761DC3505D8436D@ORSMSX101.amr.corp.intel.com>
On 12/05/2014 05:04 PM, Arad, Ronen wrote:
> I have another case of propagation which is not covered by the proposed patch.
> A recent patch introduced default_pvid attribute for a bridge (so far supported only via sysfs and not via netlink).
> When a port joins a bridge, it inherits a PVID from the default_pvid of the bridge.
> The bridge driver propagates that to the newly created net_bridge_port. This is done in br_vlan.c:
>
> int nbp_vlan_init(struct net_bridge_port *p)
> {
> int rc = 0;
>
> if (p->br->default_pvid) {
> rc = nbp_vlan_add(p, p->br->default_pvid,
> BRIDGE_VLAN_INFO_PVID |
> BRIDGE_VLAN_INFO_UNTAGGED);
> }
>
> return rc;
> }
>
> When L2 switching is offloaded to the HW, this PVID setting need to be propagated. However, it does not come via ndo_bridge_setlink. The proposed propagation at br_setlink or an up level one at rtnetlink are not capable of handling this case.
> One possible way for handling that is to replace the call to nbp_vlan_add with a call to a new function let's say
> int br_propagate_vlan_add(struct net_bridge_port *port, u16 vid, u16 flags)
> This function will compose a netlink message with VLAN filtering information (i.e. AF_SPEC with VLAN_INFO) and call br_setlink - leveraging the offload support proposed by Roopa.
>
No, we shouldn't be crafting netlink messages in the kernel just
re-inject them into an interface. Really the setlink/dellink interface
should be cleaned up so that it no longer consumes raw netlink messages.
Then either (a) add another parameter to the setlink ops or (b) create
a new op for it.
I think cleaning up the setlink/dellink hooks is on the TBD list
already.
> If this is an acceptable course of action, I could work on such patch.
>
>
[...]
Thanks,
John
--
John Fastabend Intel Corporation
^ permalink raw reply
* Re: kernel panic receiving flooded VXLAN traffic with OVS
From: Jesse Gross @ 2014-12-06 2:51 UTC (permalink / raw)
To: Jay Vosburgh; +Cc: netdev, discuss@openvswitch.org, Pravin Shelar
In-Reply-To: <8549.1417657547@famine>
On Wed, Dec 3, 2014 at 5:45 PM, Jay Vosburgh <jay.vosburgh@canonical.com> wrote:
>
> Jay Vosburgh <jay.vosburgh@canonical.com> wrote:
>
>> I am able to reproduce a kernel panic on an system using
>>openvswitch when receiving VXLAN traffic under a very specific set of
>>circumstances. This occurs with a recent net-next as well as an Ubuntu
>>3.13 kernel. I'm not sure if the error lies in OVS, GRO, or elsewhere.
>>
>> In summary, when the system receives multiple VXLAN encapsulated
>>TCP segments for a different system (not intended for local reception)
>>that are from the middle of an active connection (received due to a switch
>>flood), and are tagged to a VLAN not configured on the local host, then
>>the system panics in skb_segment when OVS calls __skb_gso_segment on the
>>GRO skb prior to performing an upcall to user space.
>>
>> The panic occurs in skbuff.c:skb_segment(), at the BUG_ON around
>>line 3036:
>>
>>struct sk_buff *skb_segment(struct sk_buff *head_skb,
>> netdev_features_t features)
>>{
>>[...]
>> skb_shinfo(nskb)->tx_flags = skb_shinfo(head_skb)->tx_flags &
>> SKBTX_SHARED_FRAG;
>>
>> while (pos < offset + len) {
>> if (i >= nfrags) {
>> BUG_ON(skb_headlen(list_skb));
>>
>> i = 0;
>>
>>
>> The BUG_ON triggers because the skbs that have been GRO
>>accumulated are partially or entirely linear, depending upon the receiving
>>network device (sky2 is partial, enic is entire). The receive buffers end
>>up being linear evidently because the mtu is set to 9000, and
>>__netdev_alloc_skb calls __alloc_skb (and thus kmalloc) instead of
>>__netdev_alloc_frag followed by build_skb.
>>
>> The foreign-VLAN VXLAN TCP segments are not processed as normal
>>VXLAN traffic, as there is no listener on the VLAN in question, so once
>>GRO processes them, they are sent directly to ovs_vport_receive. The
>>panic stack appears as follows:
>
> I've worked out some more details on this with regards to the
> cause.
>
> There seems to be a mismatch between GRO and the packet receive
> processing. GRO only looks at the receiving port number in order to
> trigger VXLAN GRO accumulation (which will in turn perform TCP
> accumulation on the encapsulated segment). For the panicking case, the
> packet receive processing doesn't deliver the GRO skb to VXLAN because
> there is no VXLAN listener on the foreign VLAN.
>
> The GRO skb is not processed through iptunnel_pull_header by
> vxlan_udp_encap_recv, so the GRO skb is left with the skb header
> pointing to the UDP header, not the inner TCP header. Note that second
> and later skbs within the GRO skb have their headers pointing to the
> inner TCP header.
>
> Then, when ovs_dp_upcall later ends up in inet_gso_segment, it
> passes the GRO skb to udp4_ufo_fragment, not tcp_gso_segment.
>
> GRO and the skb_segment call from ovs_dp_upcall appear to work
> fine on TCP-in-VXLAN segments that do pass through the VXLAN receive
> processing.
>
> I'm not sure how best to resolve this; adding a check to the GRO
> processing that an skb destined for the VXLAN port would actually be
> received by VXLAN sounds like a possible solution, but that doesn't seem
> to be simple to implement (because the skb->dev at the time GRO runs may
> not match what it becomes later if the VXLAN runs on a VLAN).
I don't think there is anything inherently wrong with aggregating TCP
segments in VXLAN that are not destined for the local host. This is
conceptually the same as doing aggregation for TCP packets where we
only perform L2 bridging - in theory we shouldn't look at the upper
layers but it is fine as long as we faithfully reconstruct it on the
way out.
A VXLAN packet that has been properly GRO-ed should result in a call
to tcp_tso_segment() even without the header being pulled off, since
that's what would happen for locally generated VXLAN packets on
egress. That's what I thought I was fixing with my previous patch to
the VXLAN GRO code although perhaps there is another issue.
^ permalink raw reply
* RE: [PATCH 2/3] bridge: offload bridge port attributes to switch asic if feature flag set
From: Arad, Ronen @ 2014-12-06 3:06 UTC (permalink / raw)
To: John Fastabend, Netdev
Cc: Roopa Prabhu, Scott Feldman, Jirí Pírko,
Jamal Hadi Salim, Benjamin LaHaise, Thomas Graf,
stephen@networkplumber.org, John Linville, nhorman@tuxdriver.com,
Nicolas Dichtel, vyasevic@redhat.com, Florian Fainelli,
buytenh@wantstofly.org, Aviad Raveh, David S. Miller,
shm@cumulusnetworks.com, Andy Gospodarek
In-Reply-To: <54826DFF.6090906@gmail.com>
> -----Original Message-----
> From: John Fastabend [mailto:john.fastabend@gmail.com]
> Sent: Friday, December 05, 2014 6:46 PM
> To: Arad, Ronen
> Cc: Roopa Prabhu; Scott Feldman; Netdev; Jirí Pírko; Jamal Hadi Salim;
> Benjamin LaHaise; Thomas Graf; stephen@networkplumber.org; John
> Linville; nhorman@tuxdriver.com; Nicolas Dichtel; vyasevic@redhat.com;
> Florian Fainelli; buytenh@wantstofly.org; Aviad Raveh; David S. Miller;
> shm@cumulusnetworks.com; Andy Gospodarek
> Subject: Re: [PATCH 2/3] bridge: offload bridge port attributes to switch asic
> if feature flag set
>
> On 12/05/2014 05:04 PM, Arad, Ronen wrote:
> > I have another case of propagation which is not covered by the proposed
> patch.
> > A recent patch introduced default_pvid attribute for a bridge (so far
> supported only via sysfs and not via netlink).
> > When a port joins a bridge, it inherits a PVID from the default_pvid of the
> bridge.
> > The bridge driver propagates that to the newly created net_bridge_port.
> This is done in br_vlan.c:
> >
> > int nbp_vlan_init(struct net_bridge_port *p) {
> > int rc = 0;
> >
> > if (p->br->default_pvid) {
> > rc = nbp_vlan_add(p, p->br->default_pvid,
> > BRIDGE_VLAN_INFO_PVID |
> > BRIDGE_VLAN_INFO_UNTAGGED);
> > }
> >
> > return rc;
> > }
> >
> > When L2 switching is offloaded to the HW, this PVID setting need to be
> propagated. However, it does not come via ndo_bridge_setlink. The
> proposed propagation at br_setlink or an up level one at rtnetlink are not
> capable of handling this case.
> > One possible way for handling that is to replace the call to
> > nbp_vlan_add with a call to a new function let's say int
> > br_propagate_vlan_add(struct net_bridge_port *port, u16 vid, u16 flags)
> This function will compose a netlink message with VLAN filtering information
> (i.e. AF_SPEC with VLAN_INFO) and call br_setlink - leveraging the offload
> support proposed by Roopa.
> >
>
> No, we shouldn't be crafting netlink messages in the kernel just re-inject
> them into an interface. Really the setlink/dellink interface should be cleaned
> up so that it no longer consumes raw netlink messages.
>
> Then either (a) add another parameter to the setlink ops or (b) create a new
> op for it.
>
> I think cleaning up the setlink/dellink hooks is on the TBD list already.
>
This would be a lot cleaner even though there could be loss of flexibility. Fixed argument interface will not be extensible.
Will the non-Netlink based driver setlink/dellink hooks be TLV based or take a pointer to a single struct with some indication of what is actually populated there?
>
> > If this is an acceptable course of action, I could work on such patch.
> >
> >
>
> [...]
>
> Thanks,
> John
>
> --
> John Fastabend Intel Corporation
^ permalink raw reply
* Re: [PATCH 2/3] bridge: offload bridge port attributes to switch asic if feature flag set
From: John Fastabend @ 2014-12-06 3:21 UTC (permalink / raw)
To: Arad, Ronen
Cc: Netdev, Roopa Prabhu, Scott Feldman, Jirí Pírko,
Jamal Hadi Salim, Benjamin LaHaise, Thomas Graf,
stephen@networkplumber.org, John Linville, nhorman@tuxdriver.com,
Nicolas Dichtel, vyasevic@redhat.com, Florian Fainelli,
buytenh@wantstofly.org, Aviad Raveh, David S. Miller,
shm@cumulusnetworks.com, Andy Gospodarek
In-Reply-To: <E4CD12F19ABA0C4D8729E087A761DC3505D84494@ORSMSX101.amr.corp.intel.com>
On 12/05/2014 07:06 PM, Arad, Ronen wrote:
>
>
>> -----Original Message-----
>> From: John Fastabend [mailto:john.fastabend@gmail.com]
>> Sent: Friday, December 05, 2014 6:46 PM
>> To: Arad, Ronen
>> Cc: Roopa Prabhu; Scott Feldman; Netdev; Jirí Pírko; Jamal Hadi Salim;
>> Benjamin LaHaise; Thomas Graf; stephen@networkplumber.org; John
>> Linville; nhorman@tuxdriver.com; Nicolas Dichtel; vyasevic@redhat.com;
>> Florian Fainelli; buytenh@wantstofly.org; Aviad Raveh; David S. Miller;
>> shm@cumulusnetworks.com; Andy Gospodarek
>> Subject: Re: [PATCH 2/3] bridge: offload bridge port attributes to switch asic
>> if feature flag set
>>
>> On 12/05/2014 05:04 PM, Arad, Ronen wrote:
>>> I have another case of propagation which is not covered by the proposed
>> patch.
>>> A recent patch introduced default_pvid attribute for a bridge (so far
>> supported only via sysfs and not via netlink).
>>> When a port joins a bridge, it inherits a PVID from the default_pvid of the
>> bridge.
>>> The bridge driver propagates that to the newly created net_bridge_port.
>> This is done in br_vlan.c:
>>>
>>> int nbp_vlan_init(struct net_bridge_port *p) {
>>> int rc = 0;
>>>
>>> if (p->br->default_pvid) {
>>> rc = nbp_vlan_add(p, p->br->default_pvid,
>>> BRIDGE_VLAN_INFO_PVID |
>>> BRIDGE_VLAN_INFO_UNTAGGED);
>>> }
>>>
>>> return rc;
>>> }
>>>
>>> When L2 switching is offloaded to the HW, this PVID setting need to be
>> propagated. However, it does not come via ndo_bridge_setlink. The
>> proposed propagation at br_setlink or an up level one at rtnetlink are not
>> capable of handling this case.
>>> One possible way for handling that is to replace the call to
>>> nbp_vlan_add with a call to a new function let's say int
>>> br_propagate_vlan_add(struct net_bridge_port *port, u16 vid, u16 flags)
>> This function will compose a netlink message with VLAN filtering information
>> (i.e. AF_SPEC with VLAN_INFO) and call br_setlink - leveraging the offload
>> support proposed by Roopa.
>>>
>>
>> No, we shouldn't be crafting netlink messages in the kernel just re-inject
>> them into an interface. Really the setlink/dellink interface should be cleaned
>> up so that it no longer consumes raw netlink messages.
>>
>> Then either (a) add another parameter to the setlink ops or (b) create a new
>> op for it.
>>
>> I think cleaning up the setlink/dellink hooks is on the TBD list already.
>>
> This would be a lot cleaner even though there could be loss of
> flexibility. Fixed argument interface will not be extensible.
> Will the non-Netlink based driver setlink/dellink hooks be TLV based
> or take a pointer to a single struct with some indication of what is
> actually populated there?
There shouldn't be any loss of flexibility, we can add new attributes
and new ops as we need them.
I had assumed it would be basic structures and additional ndo ops as
needed but I've not coded anything up.
>>> If this is an acceptable course of action, I could work on such patch.
>>>
>>>
>>
>> [...]
>>
>> Thanks,
>> John
>>
>> --
>> John Fastabend Intel Corporation
--
John Fastabend Intel Corporation
^ permalink raw reply
* Re: iproute2/nstat: Bug in displaying icmp stats
From: Vijay Subramanian @ 2014-12-06 3:45 UTC (permalink / raw)
To: Eric Dumazet, netdev
In-Reply-To: <1417831808.15618.23.camel@edumazet-glaptop2.roam.corp.google.com>
> Something like that maybe ?
>
> misc/nstat.c | 18 ++++++++++++++++--
> 1 file changed, 16 insertions(+), 2 deletions(-)
> diff --git a/misc/nstat.c b/misc/nstat.c
Thanks Eric!
This works on current kernel and looks good to me for 2.4 too..
Tested-by: Vijay Subramanian <subramanian.vijay@gmail.com>
Vijay
^ permalink raw reply
* Re: [net-next 04/14] ixgbe: remove CIAA/D register reads from bad VF check
From: David Miller @ 2014-12-06 4:49 UTC (permalink / raw)
To: jeffrey.t.kirsher
Cc: emil.s.tantilov, netdev, nhorman, sassmann, jogreene,
alex.williamson
In-Reply-To: <1417801973-28793-5-git-send-email-jeffrey.t.kirsher@intel.com>
From: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Date: Fri, 5 Dec 2014 09:52:43 -0800
> From: Emil Tantilov <emil.s.tantilov@intel.com>
>
> Accessing the CIAA/D register can block access to the PCI config space.
>
> This patch removes the read/write operations to the CIAA/D registers
> and makes use of standard kernel functions for accessing the PCI config
> space.
>
> In addition it moves ixgbevf_check_for_bad_vf() into the watchdog subtask
> which reduces the frequency of the checks.
>
> CC: Alex Williamson <alex.williamson@redhat.com>
> Reported-by: Alex Williamson <alex.williamson@redhat.com>
> Signed-off-by: Emil Tantilov <emil.s.tantilov@intel.com>
> Tested-by: Aaron Brown <aaron.f.brown@intel.com>
> Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Alex Willaimson stated that he'd like to see this for -stable, but I'm warning
right now that a change not appropriate for 'net' is not approperiate for
'-stable' either.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox