* Re: [patch net-next v3 02/17] net: make vid as a parameter for ndo_fdb_add/ndo_fdb_del
From: Jamal Hadi Salim @ 2014-11-25 16:50 UTC (permalink / raw)
To: John Fastabend, Jiri Pirko, netdev
Cc: davem, nhorman, andy, tgraf, dborkman, ogerlitz, jesse, pshelar,
azhou, ben, stephen, jeffrey.t.kirsher, vyasevic, xiyou.wangcong,
edumazet, sfeldma, f.fainelli, roopa, linville, jasowang,
ebiederm, nicolas.dichtel, ryazanov.s.a, buytenh, aviadr, nbd,
alexei.starovoitov, Neil.Jerram, ronye, simon.horman,
alexander.h.duyck, john.ronciak, mleitner, shrijeet, gospo, bcrl
In-Reply-To: <5474AE9B.6000500@intel.com>
On 11/25/14 11:30, John Fastabend wrote:
> On 11/25/2014 08:18 AM, Jamal Hadi Salim wrote:
>> On 11/25/14 11:01, John Fastabend wrote:
>>> On 11/25/2014 07:38 AM, Jamal Hadi Salim wrote:
>>>> On 11/25/14 05:28, Jiri Pirko wrote:
>>>>> Do the work of parsing NDA_VLAN directly in rtnetlink code, pass simple
>>>>> u16 vid to drivers from there.
>>>>>
>>>>
> Actually (after having some coffee) this becomes much more useful
> if you return which items failed. Then you can slam the hardware
> with your 100 entries, probably a lot more then that, and come back
> later and clean it up.
>
Yes, that is the general use case.
Unfortunately at the moment we only return codes on a netlink set
direction - but would be a beauty if we could return what succeeded
and didnt in some form of vector.
Note: all is not lost because you can always do a get afterwards and
find what is missing if you got a return code of "partial success".
Just a little less efficient..
> We return a bitmask of which operations were successful. So if SW fails
> we have both bits cleared and we abort. When SW is successful we set the
> SW bit and try to program the HW. If its sucessful we set the HW bit if
> its not we abort with an err. Converting this to (1) is not much work
> just skip the abort.
>
Ok, guess i am gonna have to go stare at the code some more.
I thought we returned one of the error codes?
A bitmask would work for a single entry - because you have two
options add to h/ware and/or s/ware. So response is easy to encode.
But if i have 1000 and they are sparsely populated (think an indexed
table and i have indices 1, 23, 45, etc), then a bitmask would be
hard to use.
cheers,
jamal
^ permalink raw reply
* Re: [patch net-next v3 08/17] bridge: call netdev_sw_port_stp_update when bridge port STP status changes
From: Jiri Pirko @ 2014-11-25 16:53 UTC (permalink / raw)
To: Andy Gospodarek
Cc: netdev, davem, nhorman, andy, tgraf, dborkman, ogerlitz, jesse,
pshelar, azhou, ben, stephen, jeffrey.t.kirsher, vyasevic,
xiyou.wangcong, john.r.fastabend, edumazet, jhs, sfeldma,
f.fainelli, roopa, linville, jasowang, ebiederm, nicolas.dichtel,
ryazanov.s.a, buytenh, aviadr, nbd, alexei.starovoitov,
Neil.Jerram, ronye, simon.horman, alexander.h.duyck, john.ronciak,
mleitner, shrijeet, bcrl
In-Reply-To: <20141125155832.GG27416@gospo.rtplab.test>
Tue, Nov 25, 2014 at 04:58:32PM CET, gospo@cumulusnetworks.com wrote:
>On Tue, Nov 25, 2014 at 11:28:39AM +0100, Jiri Pirko wrote:
>> From: Scott Feldman <sfeldma@gmail.com>
>>
>> To notify switch driver of change in STP state of bridge port, add new
>> .ndo op and provide switchdev wrapper func to call ndo op. Use it in bridge
>> code then.
>>
>> Signed-off-by: Scott Feldman <sfeldma@gmail.com>
>> Signed-off-by: Jiri Pirko <jiri@resnulli.us>
>> ---
>> v2->v3:
>> -changed "sw" string to "switch" to avoid confusion
>> v1->v2:
>> -no change
>> ---
>> include/linux/netdevice.h | 5 +++++
>> include/net/switchdev.h | 7 +++++++
>> net/bridge/br_stp.c | 2 ++
>> net/switchdev/switchdev.c | 19 +++++++++++++++++++
>> 4 files changed, 33 insertions(+)
>>
>> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
>> index ce096dc..66cb64e 100644
>> --- a/include/linux/netdevice.h
>> +++ b/include/linux/netdevice.h
>> @@ -1024,6 +1024,9 @@ typedef u16 (*select_queue_fallback_t)(struct net_device *dev,
>> * Called to get an ID of the switch chip this port is part of.
>> * If driver implements this, it indicates that it represents a port
>> * of a switch chip.
>> + * int (*ndo_switch_port_stp_update)(struct net_device *dev, u8 state);
>> + * Called to notify switch device port of bridge port STP
>> + * state change.
>> */
>> struct net_device_ops {
>> int (*ndo_init)(struct net_device *dev);
>> @@ -1180,6 +1183,8 @@ struct net_device_ops {
>> #ifdef CONFIG_NET_SWITCHDEV
>> int (*ndo_switch_parent_id_get)(struct net_device *dev,
>> struct netdev_phys_item_id *psid);
>> + int (*ndo_switch_port_stp_update)(struct net_device *dev,
>> + u8 state);
>> #endif
>> };
>>
>> diff --git a/include/net/switchdev.h b/include/net/switchdev.h
>> index 7a52360..8a6d164 100644
>> --- a/include/net/switchdev.h
>> +++ b/include/net/switchdev.h
>> @@ -16,6 +16,7 @@
>>
>> int netdev_switch_parent_id_get(struct net_device *dev,
>> struct netdev_phys_item_id *psid);
>> +int netdev_switch_port_stp_update(struct net_device *dev, u8 state);
>>
>> #else
>>
>> @@ -25,6 +26,12 @@ static inline int netdev_switch_parent_id_get(struct net_device *dev,
>> return -EOPNOTSUPP;
>> }
>>
>> +static inline int netdev_switch_port_stp_update(struct net_device *dev,
>> + u8 state)
>> +{
>> + return -EOPNOTSUPP;
>> +}
>> +
>> #endif
>>
>> #endif /* _LINUX_SWITCHDEV_H_ */
>> diff --git a/net/bridge/br_stp.c b/net/bridge/br_stp.c
>> index 2b047bc..35e016c 100644
>> --- a/net/bridge/br_stp.c
>> +++ b/net/bridge/br_stp.c
>> @@ -12,6 +12,7 @@
>> */
>> #include <linux/kernel.h>
>> #include <linux/rculist.h>
>> +#include <net/switchdev.h>
>>
>> #include "br_private.h"
>> #include "br_private_stp.h"
>> @@ -39,6 +40,7 @@ void br_log_state(const struct net_bridge_port *p)
>> void br_set_state(struct net_bridge_port *p, unsigned int state)
>> {
>> p->state = state;
>> + netdev_switch_port_stp_update(p->dev, state);
>
>The only thing that concerns me about this patch is the fact that there
>is nothing paying attention to the return code.
>
>This means if *something* in the driver fails to set the STP state we
>have no way to feed this information back to the user to let them know
>that their hardware isn't exactly functioning as we expect.
>
>I do not expect that this first set would provide full feedback to
>br_make_forwarding, br_make_blocking, etc, to allow spanning tree to
>properly deal with the failure (that change is fine to add later), but a
>short one-liner indicating that the call to the hardware failed would e
>good. What about something simple like this:
>
>void br_set_state(struct net_bridge_port *p, unsigned int state)
>{
> int ret;
> p->state = state;
> ret = netdev_switch_port_stp_update(p->dev, state);
> if (ret && ret != -EOPNOTSUPP)
> br_warn(br, "error setting offload STP state for interface %s\n",
> p->dev->name);
>}
That makes sense. Will add this.
>
>>
>> /* called under bridge lock */
>> diff --git a/net/switchdev/switchdev.c b/net/switchdev/switchdev.c
>> index 66973de..d162b21 100644
>> --- a/net/switchdev/switchdev.c
>> +++ b/net/switchdev/switchdev.c
>> @@ -31,3 +31,22 @@ int netdev_switch_parent_id_get(struct net_device *dev,
>> return ops->ndo_switch_parent_id_get(dev, psid);
>> }
>> EXPORT_SYMBOL(netdev_switch_parent_id_get);
>> +
>> +/**
>> + * netdev_switch_port_stp_update - Notify switch device port of STP
>> + * state change
>> + * @dev: port device
>> + * @state: port STP state
>> + *
>> + * Notify switch device port of bridge port STP state change.
>> + */
>> +int netdev_switch_port_stp_update(struct net_device *dev, u8 state)
>> +{
>> + const struct net_device_ops *ops = dev->netdev_ops;
>> +
>> + if (!ops->ndo_switch_port_stp_update)
>> + return -EOPNOTSUPP;
>> + WARN_ON(!ops->ndo_switch_parent_id_get);
>> + return ops->ndo_switch_port_stp_update(dev, state);
>> +}
>> +EXPORT_SYMBOL(netdev_switch_port_stp_update);
>> --
>> 1.9.3
>>
^ permalink raw reply
* Re: GRE with GRO very slow when forwarding starting with 3.14.24
From: Wolfgang Walter @ 2014-11-25 16:55 UTC (permalink / raw)
To: Tom Herbert; +Cc: Eric Dumazet, Linux Netdev List, Alexander Duyck
In-Reply-To: <CA+mtBx9SOr5RH7S63S28YbgxR8EwjgfS_dUkS2A4D1W3NzazHQ@mail.gmail.com>
Am Montag, 24. November 2014, 10:16:26 schrieb Tom Herbert:
> On Mon, Nov 24, 2014 at 8:48 AM, Eric Dumazet <eric.dumazet@gmail.com>
wrote:
> > On Mon, 2014-11-24 at 17:13 +0100, Wolfgang Walter wrote:
> > > Hello,
> > >
> > > starting with 3.14.24 GRE with GRO on is very slow. To be more specific:
> > >
> > > yyyy <--> GRO_endpoint <-_> .... <--> |eth0<->GRO-endpoint | eth1 |<->
> > > xxxx
> > >
> > > routing (IPv4) between xxxx and yyyy is very slow when GRO is enabled on
> > > eth0 and/or eth1 starting with stable kernel 3.14.24
> > >
> > > Regards,
> >
> > tcpdump might help, but I presume GSO is no longer working properly on
> > egress.
>
> Inner mac header is probably not being set in GRO->GSO GRE path.
> Please try this also:
Thanks, this patch fixes the problem.
>
> diff --git a/net/ipv4/gre_offload.c b/net/ipv4/gre_offload.c
> index bb5947b..51973dd 100644
> --- a/net/ipv4/gre_offload.c
> +++ b/net/ipv4/gre_offload.c
> @@ -247,6 +247,9 @@ static int gre_gro_complete(struct sk_buff *skb, int
> nhoff) err = ptype->callbacks.gro_complete(skb, nhoff + grehlen);
>
> rcu_read_unlock();
> +
> + skb_set_inner_mac_header(skb, nhoff + grehlen);
> +
> return err;
> }
>
> > Can you try to revert :
> >
> > commit abe640984aa492652232b65d3579361cf6d461f5
> > Author: Tom Herbert <therbert@google.com>
> > Date: Thu Oct 30 08:40:56 2014 -0700
> >
> > gre: Use inner mac length when computing tunnel length
> >
> > [ Upstream commit 14051f0452a2c26a3f4791e6ad6a435e8f1945ff ]
> >
> > Currently, skb_inner_network_header is used but this does not account
> > for Ethernet header for ETH_P_TEB. Use skb_inner_mac_header which
> > handles TEB and also should work with IP encapsulation in which case
> > inner mac and inner network headers are the same.
> >
> > Tested: Ran TCP_STREAM over GRE, worked as expected.
> >
> > Signed-off-by: Tom Herbert <therbert@google.com>
> > Acked-by: Alexander Duyck <alexander.h.duyck@redhat.com>
> > Signed-off-by: David S. Miller <davem@davemloft.net>
> > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> >
> > diff --git a/net/ipv4/gre_offload.c b/net/ipv4/gre_offload.c
> > index 2d24f293f977..8c8493ea6b1c 100644
> > --- a/net/ipv4/gre_offload.c
> > +++ b/net/ipv4/gre_offload.c
> > @@ -50,7 +50,7 @@ static struct sk_buff *gre_gso_segment(struct sk_buff
> > *skb,>
> > greh = (struct gre_base_hdr *)skb_transport_header(skb);
> >
> > - ghl = skb_inner_network_header(skb) - skb_transport_header(skb);
> > + ghl = skb_inner_mac_header(skb) - skb_transport_header(skb);
> >
> > if (unlikely(ghl < sizeof(*greh)))
> >
> > goto out;
Regards,
--
Wolfgang Walter
Studentenwerk München
Anstalt des öffentlichen Rechts
^ permalink raw reply
* [PATCH net-next] pkt_sched: fq: increase max delay from 125 ms to one second
From: Eric Dumazet @ 2014-11-25 16:57 UTC (permalink / raw)
To: Yang Yingliang, David Miller; +Cc: netdev, Neal Cardwell
In-Reply-To: <1416933094.29427.27.camel@edumazet-glaptop2.roam.corp.google.com>
From: Eric Dumazet <edumazet@google.com>
FQ/pacing has a clamp of delay of 125 ms, to avoid some possible harm.
It turns out this delay is too small to allow pacing low rates :
Some ISP setup very aggressive policers as low as 16kbit.
Now TCP stack has spurious rtx prevention, it seems safe to increase
this fixed parameter, without adding a qdisc attribute.
Signed-off-by: Eric Dumazet <edumazet@google.com>
Cc: Yang Yingliang <yangyingliang@huawei.com>
---
This is something we run since July 2014 with good results, I forgot to
upstream this change. Thanks !
Notes for Googlers :
Google-Bug-Id: 9297267
Google-Bug-Id: 11789651
Change-Id: I081d7d842bfb860a4cd620bf1c71a8eb239150c8
net/sched/sch_fq.c | 9 ++++-----
1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/net/sched/sch_fq.c b/net/sched/sch_fq.c
index cbd7e1fd23b41bb7ba0bb348c3a1a287652cca93..9b05924cc386ecc2cdb9816be27e439637fb37b3 100644
--- a/net/sched/sch_fq.c
+++ b/net/sched/sch_fq.c
@@ -481,12 +481,11 @@ begin:
if (likely(rate))
do_div(len, rate);
/* Since socket rate can change later,
- * clamp the delay to 125 ms.
- * TODO: maybe segment the too big skb, as in commit
- * e43ac79a4bc ("sch_tbf: segment too big GSO packets")
+ * clamp the delay to 1 second.
+ * Really, providers of too big packets should be fixed !
*/
- if (unlikely(len > 125 * NSEC_PER_MSEC)) {
- len = 125 * NSEC_PER_MSEC;
+ if (unlikely(len > NSEC_PER_SEC)) {
+ len = NSEC_PER_SEC;
q->stat_pkts_too_long++;
}
^ permalink raw reply related
* Re: [patch net-next v3 02/17] net: make vid as a parameter for ndo_fdb_add/ndo_fdb_del
From: Jamal Hadi Salim @ 2014-11-25 16:57 UTC (permalink / raw)
To: John Fastabend, Roopa Prabhu
Cc: Jiri Pirko, netdev, davem, nhorman, andy, tgraf, dborkman,
ogerlitz, jesse, pshelar, azhou, ben, stephen, jeffrey.t.kirsher,
vyasevic, xiyou.wangcong, edumazet, sfeldma, f.fainelli, linville,
jasowang, ebiederm, nicolas.dichtel, ryazanov.s.a, buytenh,
aviadr, nbd, alexei.starovoitov, Neil.Jerram, ronye, simon.horman,
alexander.h.duyck, john.ronciak, mleitner, shrijeet, gospo, bcrl
In-Reply-To: <5474AF53.8030709@intel.com>
On 11/25/14 11:33, John Fastabend wrote:
> On 11/25/2014 08:19 AM, Roopa Prabhu wrote:
> I agree its needed but your already out of sync for some period of time
> why the software/hardware tables are being programmed. There is no global
> sw/hw commit operation.
>
> I'm not sure it matters if the time being out of sync is a touch longer
> because we go to user space to fix it. But agreed it can be supported.
>
Recent netfilter has 2 phase commit built in. Maybe we can generalize
that?
Note, there are use cases where it is important to do rollbacks.
If i am doing a distributed router, then to make sure a FIB/NH entries
are properly synced in across the cluster is extremely important. You
cant justify letting a few packets sneak in the wrong path.
in other words, strong consistency is important.
Having said that things can be worked around (and i would not use
2pc for the example use case i gave); however, that doesnt negate
the fact we need it.
cheers,
jamal
^ permalink raw reply
* Re: [patch net-next v3 07/17] rocker: introduce rocker switch driver
From: Jiri Pirko @ 2014-11-25 16:57 UTC (permalink / raw)
To: Jamal Hadi Salim
Cc: netdev, davem, nhorman, andy, tgraf, dborkman, ogerlitz, jesse,
pshelar, azhou, ben, stephen, jeffrey.t.kirsher, vyasevic,
xiyou.wangcong, john.r.fastabend, edumazet, sfeldma, f.fainelli,
roopa, linville, jasowang, ebiederm, nicolas.dichtel,
ryazanov.s.a, buytenh, aviadr, nbd, alexei.starovoitov,
Neil.Jerram, ronye, simon.horman, alexander.h.duyck, john.ronciak,
mleitner, shrijeet, gospo, bcrl
In-Reply-To: <5474A705.5070007@mojatatu.com>
Tue, Nov 25, 2014 at 04:57:57PM CET, jhs@mojatatu.com wrote:
>On 11/25/14 05:28, Jiri Pirko wrote:
>>This patch introduces the first driver to benefit from the switchdev
>>infrastructure and to implement newly introduced switch ndos. This is a
>>driver for emulated switch chip implemented in qemu:
>>https://github.com/sfeldma/qemu-rocker/
>>
>>This patch is a result of joint work with Scott Feldman.
>>
>>Signed-off-by: Scott Feldman <sfeldma@gmail.com>
>>Signed-off-by: Jiri Pirko <jiri@resnulli.us>
>>Reviewed-by: Thomas Graf <tgraf@suug.ch>
>>Reviewed-by: John Fastabend <john.r.fastabend@intel.com>
>
>Users should be a different patch set for easier review.
>I think you should separate rocker to be another patch series.
Hmm. When we introduce a new ops/callbacks, we need to use it in the
same patchset. That is the rule if I'm not mistaken. That is why I send
this together. I do not see any problems in that given the fact that
rocker is new driver so it cannot indroduce a regression.
I would like to keep pushing this as a single patchset.
>
>cheers,
>jamal
>
^ permalink raw reply
* Re: [patch net-next v3 16/17] bridge: add brport flags to dflt bridge_getlink
From: Scott Feldman @ 2014-11-25 17:02 UTC (permalink / raw)
To: Jamal Hadi Salim
Cc: Jiri Pirko, Netdev, David S. Miller, nhorman, Andy Gospodarek,
Thomas Graf, dborkman, ogerlitz, jesse, pshelar, azhou, ben,
stephen, Kirsher, Jeffrey T, vyasevic, Cong Wang,
Fastabend, John R, Eric Dumazet, Florian Fainelli, Roopa Prabhu,
John Linville, jasowang, ebiederm, Nicolas Dichtel, ryazanov.s.a,
buytenh, Aviad Raveh, nbd, Alexei Starovoitov <alexei.
In-Reply-To: <5474A992.5010305@mojatatu.com>
On Tue, Nov 25, 2014 at 8:08 AM, Jamal Hadi Salim <jhs@mojatatu.com> wrote:
> On 11/25/14 05:28, Jiri Pirko wrote:
>>
>> From: Scott Feldman <sfeldma@gmail.com>
>>
>> To allow brport device to return current brport flags set on port. Add
>> returned flags to nested IFLA_PROTINFO netlink msg built in dflt getlink.
>> With this change, netlink msg returned for bridge_getlink contains the
>> port's
>> offloaded flag settings (the port's SELF settings).
>>
>> Signed-off-by: Scott Feldman <sfeldma@gmail.com>
>> Signed-off-by: Jiri Pirko <jiri@resnulli.us>
>
>
> Not sure i followed this. VEPA etc are offload modes, no?
> I like the harvesting from netlink->internal format but
> not sure i followed the rest: VEPA etc are offload modes, no?
VEPA etc are offload modes.
> cheers,
> jamal
^ permalink raw reply
* Re: [patch net-next v3 04/17] net: introduce generic switch devices support
From: Jamal Hadi Salim @ 2014-11-25 17:08 UTC (permalink / raw)
To: Jiri Pirko
Cc: netdev, davem, nhorman, andy, tgraf, dborkman, ogerlitz, jesse,
pshelar, azhou, ben, stephen, jeffrey.t.kirsher, vyasevic,
xiyou.wangcong, john.r.fastabend, edumazet, sfeldma, f.fainelli,
roopa, linville, jasowang, ebiederm, nicolas.dichtel,
ryazanov.s.a, buytenh, aviadr, nbd, alexei.starovoitov,
Neil.Jerram, ronye, simon.horman, alexander.h.duyck, john.ronciak,
mleitner, shrijeet, gospo, bcrl
In-Reply-To: <20141125164954.GJ1971@nanopsycho.orion>
On 11/25/14 11:49, Jiri Pirko wrote:
>
> What does it do? "L3 switching"?
>
Absolutely not - that is too easy;-> Why not just a mellanox
chip for that? (Testing if Aviad is awake). But flows and associated
constructs apply.
>> "offload_id" would be the right term. switch doesnt sound right.
>
> When we talk about this area, we use word "switch". I know it is not
> accurate, but in my opinion it is the closest we can get. "chip" and
> "ASIC" are too generic I believe. I would not use "offload" cause it wan
> be easily mistaken with NIC offloads + it is alsno not accurate.
I think this interface is usable for example to offload to user space
ala DPDK and friends just as it would be for ASICs or standard NIC
offload (which we already have with fdb offload).
I dont know what a good name is - but switch looks incorrect.
cheers,
jamal
^ permalink raw reply
* Re: [Bug 88871] New: SO_BINDTODEVICE set sock's sk_bound_dev_if ,but interface's index can chang where interface down and up
From: David Miller @ 2014-11-25 17:09 UTC (permalink / raw)
To: stephen; +Cc: netdev
In-Reply-To: <20141125074338.7476ad08@urahara>
From: Stephen Hemminger <stephen@networkplumber.org>
Date: Tue, 25 Nov 2014 07:43:38 -0800
> This is not really a bug.
Agreed.
^ permalink raw reply
* Re: [patch net-next v3 07/17] rocker: introduce rocker switch driver
From: David Miller @ 2014-11-25 17:10 UTC (permalink / raw)
To: jhs
Cc: jiri, netdev, nhorman, andy, tgraf, dborkman, ogerlitz, jesse,
pshelar, azhou, ben, stephen, jeffrey.t.kirsher, vyasevic,
xiyou.wangcong, john.r.fastabend, edumazet, sfeldma, f.fainelli,
roopa, linville, jasowang, ebiederm, nicolas.dichtel,
ryazanov.s.a, buytenh, aviadr, nbd, alexei.starovoitov,
Neil.Jerram, ronye, simon.horman, alexander.h.duyck, john.ronciak,
mleitner, shrijeet, gospo, bcrl
In-Reply-To: <5474A705.5070007@mojatatu.com>
From: Jamal Hadi Salim <jhs@mojatatu.com>
Date: Tue, 25 Nov 2014 10:57:57 -0500
> On 11/25/14 05:28, Jiri Pirko wrote:
>> This patch introduces the first driver to benefit from the switchdev
>> infrastructure and to implement newly introduced switch ndos. This is
>> a
>> driver for emulated switch chip implemented in qemu:
>> https://github.com/sfeldma/qemu-rocker/
>>
>> This patch is a result of joint work with Scott Feldman.
>>
>> Signed-off-by: Scott Feldman <sfeldma@gmail.com>
>> Signed-off-by: Jiri Pirko <jiri@resnulli.us>
>> Reviewed-by: Thomas Graf <tgraf@suug.ch>
>> Reviewed-by: John Fastabend <john.r.fastabend@intel.com>
>
> Users should be a different patch set for easier review.
> I think you should separate rocker to be another patch series.
I want to see how the new infrastructure is used in the same patch
series as the changes that add that infrastructure.
^ permalink raw reply
* Re: [patch net-next v3 11/17] rocker: implement L2 bridge offloading
From: David Miller @ 2014-11-25 17:11 UTC (permalink / raw)
To: jhs
Cc: jiri, netdev, nhorman, andy, tgraf, dborkman, ogerlitz, jesse,
pshelar, azhou, ben, stephen, jeffrey.t.kirsher, vyasevic,
xiyou.wangcong, john.r.fastabend, edumazet, sfeldma, f.fainelli,
roopa, linville, jasowang, ebiederm, nicolas.dichtel,
ryazanov.s.a, buytenh, aviadr, nbd, alexei.starovoitov,
Neil.Jerram, ronye, simon.horman, alexander.h.duyck, john.ronciak,
mleitner, shrijeet, gospo, bcrl
In-Reply-To: <5474A879.4030807@mojatatu.com>
From: Jamal Hadi Salim <jhs@mojatatu.com>
Date: Tue, 25 Nov 2014 11:04:09 -0500
> Same comment on rocker.
Disagree :)
^ permalink raw reply
* Re: [PATCH v4 0/7] kernel tinification: optionally compile out splice family of syscalls (splice, vmsplice, tee and sendfile)
From: David Miller @ 2014-11-25 17:13 UTC (permalink / raw)
To: rdunlap-wEGCiKHe2LqWVfeAwA7xHQ
Cc: pieter-qeJ+1H9vRZbz+pZb47iToQ, josh-iaAMLnmF4UmaiuxdJuQwMA,
alexander.h.duyck-ral2JQCrhuEAvxtiuMwx3w,
viro-RmSDqhL/yNMiFSDQTTA3OLVCufUGDwFn, ast-uqk4Ao+rVK5Wk0Htik3J/w,
akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b,
beber-2YnHqweIUXrk1uMJSBkQmQ,
catalina.mocanu-Re5JQEeQqe8AvxtiuMwx3w,
dborkman-H+wXaHxf7aLQT0dZR+AlfA, edumazet-hpIqsD4AKlfQT0dZR+AlfA,
ebiederm-aS9lmoZGLiVWk0Htik3J/w, fabf-AgBVmzD5pcezQB+pC5nmwQ,
fuse-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f,
geert-Td1EMuHUCqxL1ZNQvxDV9g, hughd-hpIqsD4AKlfQT0dZR+AlfA,
iulia.manda21-Re5JQEeQqe8AvxtiuMwx3w, JBeulich-IBi9RG/b67k,
bfields-uC3wQj2KruNg9hUCZPvPmw, jlayton-vpEMnDpepFuMZCB2o+C8xQ,
linux-api-u79uwXL29TY76Z2rM5mHXA,
linux-fsdevel-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-nfs-u79uwXL29TY76Z2rM5mHXA, mcgrof-IBi9RG/b67k,
mattst88-Re5JQEeQqe8AvxtiuMwx3w, mgorman-l3A5Bk7waGM,
mst-H+wXaHxf7aLQT0dZR+AlfA, miklos-sUDqSbJrdHQHWmgEVkV9KA,
netdev-u79uwXL29TY76Z2rM5mHXA, oleg-H+wXaHxf7aLQT0dZR+AlfA,
Paul.Durrant-Sxgqhf6Nn4DQT0dZR+AlfA,
paulmck-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8,
pefoley2-lY0TAiDIAFlBDgjK7y7TUQ, tgraf-G/eBtMaohhA,
therbert-hpIqsD4AKlfQT0dZR+AlfA,
trond.myklebust-7I+n7zu2hftEKMMhf/gKZA,
willemb-hpIqsD4AKlfQT0dZR+AlfA,
xiaoguangrong-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8, zhe
In-Reply-To: <5474ABB6.3030400-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org>
From: Randy Dunlap <rdunlap-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org>
Date: Tue, 25 Nov 2014 08:17:58 -0800
> Is the splice family of syscalls the only one that tiny has identified
> for optional building or can we expect similar treatment for other
> syscalls?
>
> Why will many embedded systems not need these syscalls? You know
> exactly what apps they run and you are positive that those apps do
> not use splice?
I think starting to compile out system calls is a very slippery
slope we should not begin the journey down.
This changes the forward facing interface to userspace.
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [patch net-next v3 07/17] rocker: introduce rocker switch driver
From: Jiri Pirko @ 2014-11-25 17:15 UTC (permalink / raw)
To: David Miller
Cc: jhs, netdev, nhorman, andy, tgraf, dborkman, ogerlitz, jesse,
pshelar, azhou, ben, stephen, jeffrey.t.kirsher, vyasevic,
xiyou.wangcong, john.r.fastabend, edumazet, sfeldma, f.fainelli,
roopa, linville, jasowang, ebiederm, nicolas.dichtel,
ryazanov.s.a, buytenh, aviadr, nbd, alexei.starovoitov,
Neil.Jerram, ronye, simon.horman, alexander.h.duyck, john.ronciak,
mleitner, shrijeet, gospo, bcrl
In-Reply-To: <20141125.121019.465267851168750694.davem@davemloft.net>
Tue, Nov 25, 2014 at 06:10:19PM CET, davem@davemloft.net wrote:
>From: Jamal Hadi Salim <jhs@mojatatu.com>
>Date: Tue, 25 Nov 2014 10:57:57 -0500
>
>> On 11/25/14 05:28, Jiri Pirko wrote:
>>> This patch introduces the first driver to benefit from the switchdev
>>> infrastructure and to implement newly introduced switch ndos. This is
>>> a
>>> driver for emulated switch chip implemented in qemu:
>>> https://github.com/sfeldma/qemu-rocker/
>>>
>>> This patch is a result of joint work with Scott Feldman.
>>>
>>> Signed-off-by: Scott Feldman <sfeldma@gmail.com>
>>> Signed-off-by: Jiri Pirko <jiri@resnulli.us>
>>> Reviewed-by: Thomas Graf <tgraf@suug.ch>
>>> Reviewed-by: John Fastabend <john.r.fastabend@intel.com>
>>
>> Users should be a different patch set for easier review.
>> I think you should separate rocker to be another patch series.
>
>I want to see how the new infrastructure is used in the same patch
>series as the changes that add that infrastructure.
Yep. For easier review, I will move the rocker patches to the tail and
leave the other ones on head.
^ permalink raw reply
* Re: [patch net-next v3 07/17] rocker: introduce rocker switch driver
From: Jamal Hadi Salim @ 2014-11-25 17:15 UTC (permalink / raw)
To: Jiri Pirko, David Miller
Cc: netdev, nhorman, andy, tgraf, dborkman, ogerlitz, jesse, pshelar,
azhou, ben, stephen, jeffrey.t.kirsher, vyasevic, xiyou.wangcong,
john.r.fastabend, edumazet, sfeldma, f.fainelli, roopa, linville,
jasowang, ebiederm, nicolas.dichtel, ryazanov.s.a, buytenh,
aviadr, nbd, alexei.starovoitov, Neil.Jerram, ronye, simon.horman,
alexander.h.duyck, john.ronciak, mleitner, shrijeet, gospo, bcrl
In-Reply-To: <20141125171514.GN1971@nanopsycho.orion>
On 11/25/14 12:15, Jiri Pirko wrote:
> Yep. For easier review, I will move the rocker patches to the tail and
> leave the other ones on head.
Sounds reasonable.
cheers,
jamal
^ permalink raw reply
* Re: Patch to fix scheduling while atomic splat
From: Greg Kroah-Hartman @ 2014-11-25 17:23 UTC (permalink / raw)
To: Larry Finger; +Cc: netdev@vger.kernel.org >> netdev
In-Reply-To: <5474B234.9090701@lwfinger.net>
On Tue, Nov 25, 2014 at 10:45:40AM -0600, Larry Finger wrote:
> Greg,
>
> On Nov. 12, I sent a patch for kernel 3.18 with the subject "[PATCH for
> 3.18] staging: r8188eu: Fix scheduling while atomic error introduced in
> commit fadbe0cd". To date, this fix does not seem to have appeared in
> mainline. Did it get missed?
No, it's in my queue of 500+ other staging patches to be reviewed,
sorry. Hope to get to that queue soon.
greg k-h
^ permalink raw reply
* [PATCH net-next] gre: Set inner mac header in gro complete
From: Tom Herbert @ 2014-11-25 17:30 UTC (permalink / raw)
To: davem, linux, netdev
Set the inner mac header to point to the GRE payload when
doing GRO. This is needed if we proceed to send the packet
through GRE GSO which now uses the inner mac header instead
of inner network header to determine the length of encapsulation
headers.
Reported-by: Wolfgang Walter <linux@stwm.de>
Signed-off-by: Tom Herbert <therbert@google.com>
---
net/ipv4/gre_offload.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/net/ipv4/gre_offload.c b/net/ipv4/gre_offload.c
index bb5947b..51973dd 100644
--- a/net/ipv4/gre_offload.c
+++ b/net/ipv4/gre_offload.c
@@ -247,6 +247,9 @@ static int gre_gro_complete(struct sk_buff *skb, int nhoff)
err = ptype->callbacks.gro_complete(skb, nhoff + grehlen);
rcu_read_unlock();
+
+ skb_set_inner_mac_header(skb, nhoff + grehlen);
+
return err;
}
--
2.1.0.rc2.206.gedb03e5
^ permalink raw reply related
* Re: [patch net-next v3 16/17] bridge: add brport flags to dflt bridge_getlink
From: Andy Gospodarek @ 2014-11-25 17:48 UTC (permalink / raw)
To: Jiri Pirko
Cc: netdev, davem, nhorman, andy, tgraf, dborkman, ogerlitz, jesse,
pshelar, azhou, ben, stephen, jeffrey.t.kirsher, vyasevic,
xiyou.wangcong, john.r.fastabend, edumazet, jhs, sfeldma,
f.fainelli, roopa, linville, jasowang, ebiederm, nicolas.dichtel,
ryazanov.s.a, buytenh, aviadr, nbd, alexei.starovoitov,
Neil.Jerram, ronye, simon.horman, alexander.h.duyck, john.ronciak,
mleitner, shrijeet, bcrl
In-Reply-To: <1416911328-10979-17-git-send-email-jiri@resnulli.us>
On Tue, Nov 25, 2014 at 11:28:47AM +0100, Jiri Pirko wrote:
> From: Scott Feldman <sfeldma@gmail.com>
>
> To allow brport device to return current brport flags set on port. Add
> returned flags to nested IFLA_PROTINFO netlink msg built in dflt getlink.
> With this change, netlink msg returned for bridge_getlink contains the port's
> offloaded flag settings (the port's SELF settings).
>
> Signed-off-by: Scott Feldman <sfeldma@gmail.com>
> Signed-off-by: Jiri Pirko <jiri@resnulli.us>
Seems fine.
Acked-by: Andy Gospodarek <gospo@cumulusnetworks.com>
> ---
> new in v3
> ---
> drivers/net/ethernet/emulex/benet/be_main.c | 3 ++-
> drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 2 +-
> include/linux/rtnetlink.h | 3 ++-
> net/core/rtnetlink.c | 39 ++++++++++++++++++++++++++-
> 4 files changed, 43 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/net/ethernet/emulex/benet/be_main.c b/drivers/net/ethernet/emulex/benet/be_main.c
> index 9070b98..6510ec8 100644
> --- a/drivers/net/ethernet/emulex/benet/be_main.c
> +++ b/drivers/net/ethernet/emulex/benet/be_main.c
> @@ -4362,7 +4362,8 @@ static int be_ndo_bridge_getlink(struct sk_buff *skb, u32 pid, u32 seq,
>
> return ndo_dflt_bridge_getlink(skb, pid, seq, dev,
> hsw_mode == PORT_FWD_TYPE_VEPA ?
> - BRIDGE_MODE_VEPA : BRIDGE_MODE_VEB);
> + BRIDGE_MODE_VEPA : BRIDGE_MODE_VEB,
> + 0, 0);
> }
>
> #ifdef CONFIG_BE2NET_VXLAN
> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> index 1bad9f4..eb2a04b 100644
> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> @@ -7773,7 +7773,7 @@ static int ixgbe_ndo_bridge_getlink(struct sk_buff *skb, u32 pid, u32 seq,
> else
> mode = BRIDGE_MODE_VEPA;
>
> - return ndo_dflt_bridge_getlink(skb, pid, seq, dev, mode);
> + return ndo_dflt_bridge_getlink(skb, pid, seq, dev, mode, 0, 0);
> }
>
> static void *ixgbe_fwd_add(struct net_device *pdev, struct net_device *vdev)
> diff --git a/include/linux/rtnetlink.h b/include/linux/rtnetlink.h
> index 063f0f5..3b04190 100644
> --- a/include/linux/rtnetlink.h
> +++ b/include/linux/rtnetlink.h
> @@ -103,5 +103,6 @@ extern int ndo_dflt_fdb_del(struct ndmsg *ndm,
> u16 vid);
>
> extern int ndo_dflt_bridge_getlink(struct sk_buff *skb, u32 pid, u32 seq,
> - struct net_device *dev, u16 mode);
> + struct net_device *dev, u16 mode,
> + u32 flags, u32 mask);
> #endif /* __LINUX_RTNETLINK_H */
> diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
> index bd5e783..91e5368 100644
> --- a/net/core/rtnetlink.c
> +++ b/net/core/rtnetlink.c
> @@ -2687,12 +2687,22 @@ static int rtnl_fdb_dump(struct sk_buff *skb, struct netlink_callback *cb)
> return skb->len;
> }
>
> +static int brport_nla_put_flag(struct sk_buff *skb, u32 flags, u32 mask,
> + unsigned int attrnum, unsigned int flag)
> +{
> + if (mask & flag)
> + return nla_put_u8(skb, attrnum, !!(flags & flag));
> + return 0;
> +}
> +
> int ndo_dflt_bridge_getlink(struct sk_buff *skb, u32 pid, u32 seq,
> - struct net_device *dev, u16 mode)
> + struct net_device *dev, u16 mode,
> + u32 flags, u32 mask)
> {
> struct nlmsghdr *nlh;
> struct ifinfomsg *ifm;
> struct nlattr *br_afspec;
> + struct nlattr *protinfo;
> u8 operstate = netif_running(dev) ? dev->operstate : IF_OPER_DOWN;
> struct net_device *br_dev = netdev_master_upper_dev_get(dev);
>
> @@ -2731,6 +2741,33 @@ int ndo_dflt_bridge_getlink(struct sk_buff *skb, u32 pid, u32 seq,
> }
> nla_nest_end(skb, br_afspec);
>
> + protinfo = nla_nest_start(skb, IFLA_PROTINFO | NLA_F_NESTED);
> + if (!protinfo)
> + goto nla_put_failure;
> +
> + if (brport_nla_put_flag(skb, flags, mask,
> + IFLA_BRPORT_MODE, BR_HAIRPIN_MODE) ||
> + brport_nla_put_flag(skb, flags, mask,
> + IFLA_BRPORT_GUARD, BR_BPDU_GUARD) ||
> + brport_nla_put_flag(skb, flags, mask,
> + IFLA_BRPORT_FAST_LEAVE,
> + BR_MULTICAST_FAST_LEAVE) ||
> + brport_nla_put_flag(skb, flags, mask,
> + IFLA_BRPORT_PROTECT, BR_ROOT_BLOCK) ||
> + brport_nla_put_flag(skb, flags, mask,
> + IFLA_BRPORT_LEARNING, BR_LEARNING) ||
> + brport_nla_put_flag(skb, flags, mask,
> + IFLA_BRPORT_LEARNING_SYNC, BR_LEARNING_SYNC) ||
> + brport_nla_put_flag(skb, flags, mask,
> + IFLA_BRPORT_UNICAST_FLOOD, BR_FLOOD) ||
> + brport_nla_put_flag(skb, flags, mask,
> + IFLA_BRPORT_PROXYARP, BR_PROXYARP)) {
> + nla_nest_cancel(skb, protinfo);
> + goto nla_put_failure;
> + }
> +
> + nla_nest_end(skb, protinfo);
> +
> return nlmsg_end(skb, nlh);
> nla_put_failure:
> nlmsg_cancel(skb, nlh);
> --
> 1.9.3
>
^ permalink raw reply
* Re: [patch net-next v3 07/17] rocker: introduce rocker switch driver
From: Andy Gospodarek @ 2014-11-25 17:51 UTC (permalink / raw)
To: Jiri Pirko
Cc: David Miller, jhs, netdev, nhorman, andy, tgraf, dborkman,
ogerlitz, jesse, pshelar, azhou, ben, stephen, jeffrey.t.kirsher,
vyasevic, xiyou.wangcong, john.r.fastabend, edumazet, sfeldma,
f.fainelli, roopa, linville, jasowang, ebiederm, nicolas.dichtel,
ryazanov.s.a, buytenh, aviadr, nbd, alexei.starovoitov,
Neil.Jerram, ronye, simon.horman, alexander.h.duyck, john.ronciak,
mleitner, shrijeet, bcrl
In-Reply-To: <20141125171514.GN1971@nanopsycho.orion>
On Tue, Nov 25, 2014 at 06:15:14PM +0100, Jiri Pirko wrote:
> Tue, Nov 25, 2014 at 06:10:19PM CET, davem@davemloft.net wrote:
> >From: Jamal Hadi Salim <jhs@mojatatu.com>
> >Date: Tue, 25 Nov 2014 10:57:57 -0500
> >
> >> On 11/25/14 05:28, Jiri Pirko wrote:
> >>> This patch introduces the first driver to benefit from the switchdev
> >>> infrastructure and to implement newly introduced switch ndos. This is
> >>> a
> >>> driver for emulated switch chip implemented in qemu:
> >>> https://github.com/sfeldma/qemu-rocker/
> >>>
> >>> This patch is a result of joint work with Scott Feldman.
> >>>
> >>> Signed-off-by: Scott Feldman <sfeldma@gmail.com>
> >>> Signed-off-by: Jiri Pirko <jiri@resnulli.us>
> >>> Reviewed-by: Thomas Graf <tgraf@suug.ch>
> >>> Reviewed-by: John Fastabend <john.r.fastabend@intel.com>
> >>
> >> Users should be a different patch set for easier review.
> >> I think you should separate rocker to be another patch series.
> >
> >I want to see how the new infrastructure is used in the same patch
> >series as the changes that add that infrastructure.
>
> Yep. For easier review, I will move the rocker patches to the tail and
> leave the other ones on head.
I appreciate that. I do plan to *try* and review those, but due to
their length have not done that.
^ permalink raw reply
* Re: [PATCH net-next] gre: Set inner mac header in gro complete
From: Eric Dumazet @ 2014-11-25 17:57 UTC (permalink / raw)
To: Tom Herbert; +Cc: davem, linux, netdev
In-Reply-To: <1416936611-22702-1-git-send-email-therbert@google.com>
On Tue, 2014-11-25 at 09:30 -0800, Tom Herbert wrote:
> Set the inner mac header to point to the GRE payload when
> doing GRO. This is needed if we proceed to send the packet
> through GRE GSO which now uses the inner mac header instead
> of inner network header to determine the length of encapsulation
> headers.
>
> Reported-by: Wolfgang Walter <linux@stwm.de>
> Signed-off-by: Tom Herbert <therbert@google.com>
> ---
Please Tom, add in your bug fixes something to ease backports.
Fixes: 14051f0452a2 ("gre: Use inner mac length when computing tunnel length")
^ permalink raw reply
* [PATCH rfc 0/4] timestamping updates
From: Willem de Bruijn @ 2014-11-25 17:58 UTC (permalink / raw)
To: netdev; +Cc: davem, luto, richardcochran, Willem de Bruijn
From: Willem de Bruijn <willemb@google.com>
The main goal for this patchset is to allow correlating timestamps
with the egress interface. That change requires a few others:
1/4: TCP sockets should not loop L2/L3/L4 headers
2/4: main feature
3/4: bugfix: TCP sockets should call the family specific .._recv_error
4/4: revise the test to verify these changes
Willem de Bruijn (4):
net-timestamp: pull headers for SOCK_STREAM
net-errqueue: add IP(V6)_PKTINFO support
net-timestamp: tcp sockets return v6 errors on v6 sockets
net-timestamp: expand txtimestamp test with payload and PKTINFO
.../networking/timestamping/txtimestamp.c | 89 +++++++++++++++++++---
net/core/skbuff.c | 6 +-
net/ipv4/ip_sockglue.c | 15 ++++
net/ipv4/tcp.c | 8 +-
net/ipv6/datagram.c | 22 ++++++
5 files changed, 125 insertions(+), 15 deletions(-)
--
2.1.0.rc2.206.gedb03e5
^ permalink raw reply
* [PATCH rfc 1/4] net-timestamp: pull headers for SOCK_STREAM
From: Willem de Bruijn @ 2014-11-25 17:58 UTC (permalink / raw)
To: netdev; +Cc: davem, luto, richardcochran, Willem de Bruijn
In-Reply-To: <1416938286-14147-1-git-send-email-willemb@google.com>
From: Willem de Bruijn <willemb@google.com>
When returning timestamped packets on the error queue, only return
the data that the application initially sent: not the protocol
headers.
This changes the ABI. The TCP interface is new enough that it should
be safe to do so. The UDP interface could be changed analogously with
+ else if (sk->sk_protocol == IPPROTO_UDP)
+ skb_pull(skb, skb_transport_offset(skb) + sizeof(struct udphdr));
Tested with Documentation/networking/timestamping/txtimestamp -l 60 -x
Signed-off-by: Willem de Bruijn <willemb@google.com>
---
net/core/skbuff.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index 92116df..70a8596 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -3580,6 +3580,7 @@ static void __skb_complete_tx_timestamp(struct sk_buff *skb,
int tstype)
{
struct sock_exterr_skb *serr;
+ bool is_tcp = sk->sk_protocol == IPPROTO_TCP;
int err;
serr = SKB_EXT_ERR(skb);
@@ -3589,10 +3590,13 @@ static void __skb_complete_tx_timestamp(struct sk_buff *skb,
serr->ee.ee_info = tstype;
if (sk->sk_tsflags & SOF_TIMESTAMPING_OPT_ID) {
serr->ee.ee_data = skb_shinfo(skb)->tskey;
- if (sk->sk_protocol == IPPROTO_TCP)
+ if (is_tcp)
serr->ee.ee_data -= sk->sk_tskey;
}
+ if (is_tcp)
+ skb_pull(skb, skb_transport_offset(skb) + tcp_hdrlen(skb));
+
err = sock_queue_err_skb(sk, skb);
if (err)
--
2.1.0.rc2.206.gedb03e5
^ permalink raw reply related
* [PATCH rfc 2/4] net-errqueue: add IP(V6)_PKTINFO support
From: Willem de Bruijn @ 2014-11-25 17:58 UTC (permalink / raw)
To: netdev; +Cc: davem, luto, richardcochran, Willem de Bruijn
In-Reply-To: <1416938286-14147-1-git-send-email-willemb@google.com>
From: Willem de Bruijn <willemb@google.com>
On INET and INET6 sockets with the IP_PKTINFO or IPV6_RECVPKTINFO
socket option set, return a matching cmsg also for packets queued
to the error queue.
These packets are transmitted packets looped back. The fields of
struct in[6]_pktinfo are changed to reflect that:
ifindex: index of the outgoing device, if configured
addr: destination address
spec_dst: source address (absent in v6)
On IPv6, the mechanism currently returns two IPV6_PKTINFO when
the option is enabled, because the existing code already supports
the feature. The legacy data does not produce a correct ifindex,
however. Perhaps I can revise the patch to only send the legacy
patch, but use the correct ifindex source in case of ERRQUEUE.
Signed-off-by: Willem de Bruijn <willemb@google.com>
---
net/ipv4/ip_sockglue.c | 15 +++++++++++++++
net/ipv6/datagram.c | 22 ++++++++++++++++++++++
2 files changed, 37 insertions(+)
diff --git a/net/ipv4/ip_sockglue.c b/net/ipv4/ip_sockglue.c
index b782657..615f783 100644
--- a/net/ipv4/ip_sockglue.c
+++ b/net/ipv4/ip_sockglue.c
@@ -399,6 +399,20 @@ void ip_local_error(struct sock *sk, int err, __be32 daddr, __be16 port, u32 inf
kfree_skb(skb);
}
+static void ip_recv_error_pktinfo(struct msghdr *msg, struct sock *sk,
+ struct sk_buff *skb)
+{
+ if (inet_sk(sk)->cmsg_flags & IP_CMSG_PKTINFO && skb->dev) {
+ struct in_pktinfo info = {0};
+
+ info.ipi_spec_dst.s_addr = ip_hdr(skb)->saddr;
+ info.ipi_addr.s_addr = ip_hdr(skb)->daddr;
+ info.ipi_ifindex = skb->dev->ifindex;
+
+ put_cmsg(msg, SOL_IP, IP_PKTINFO, sizeof(info), &info);
+ }
+}
+
/*
* Handle MSG_ERRQUEUE
*/
@@ -429,6 +443,7 @@ int ip_recv_error(struct sock *sk, struct msghdr *msg, int len, int *addr_len)
goto out_free_skb;
sock_recv_timestamp(msg, sk, skb);
+ ip_recv_error_pktinfo(msg, sk, skb);
serr = SKB_EXT_ERR(skb);
diff --git a/net/ipv6/datagram.c b/net/ipv6/datagram.c
index cc11396..7d2ef7c 100644
--- a/net/ipv6/datagram.c
+++ b/net/ipv6/datagram.c
@@ -325,6 +325,27 @@ void ipv6_local_rxpmtu(struct sock *sk, struct flowi6 *fl6, u32 mtu)
kfree_skb(skb);
}
+static void ipv6_recv_error_pktinfo(struct msghdr *msg, struct sock *sk,
+ struct sk_buff *skb)
+{
+ struct ipv6_pinfo *np = inet6_sk(sk);
+
+ if (np->rxopt.bits.rxinfo && skb->dev) {
+ struct in6_pktinfo info;
+
+ memset(&info, 0, sizeof(info));
+ if (skb->protocol == htons(ETH_P_IPV6))
+ info.ipi6_addr = ipv6_hdr(skb)->daddr;
+ else
+ ipv6_addr_set_v4mapped(ip_hdr(skb)->daddr,
+ &info.ipi6_addr);
+
+ info.ipi6_ifindex = skb->dev->ifindex;
+ net_info_ratelimited("yes: ifindex=%d\n", info.ipi6_ifindex);
+ put_cmsg(msg, SOL_IPV6, IPV6_PKTINFO, sizeof(info), &info);
+ }
+}
+
/*
* Handle MSG_ERRQUEUE
*/
@@ -356,6 +377,7 @@ int ipv6_recv_error(struct sock *sk, struct msghdr *msg, int len, int *addr_len)
goto out_free_skb;
sock_recv_timestamp(msg, sk, skb);
+ ipv6_recv_error_pktinfo(msg, sk, skb);
serr = SKB_EXT_ERR(skb);
--
2.1.0.rc2.206.gedb03e5
^ permalink raw reply related
* [PATCH rfc 3/4] net-timestamp: tcp sockets return v6 errors on v6 sockets
From: Willem de Bruijn @ 2014-11-25 17:58 UTC (permalink / raw)
To: netdev; +Cc: davem, luto, richardcochran, Willem de Bruijn
In-Reply-To: <1416938286-14147-1-git-send-email-willemb@google.com>
From: Willem de Bruijn <willemb@google.com>
TCP timestamping introduced MSG_ERRQUEUE handling for TCP sockets.
It always passed errorqueue requests onto ip_recv_error, but the
same tcp_recvmsg code may also be called for IPv6 sockets. In that
case, pass to ipv6_recv_error.
Tested by asking for PKTINFO with
Documentation/networking/timestamping/txtimestamp -I
Before this change, IPv6 sockets would return AF_INET/IP_PKTINFO
after the change, these sockets return AF_INET6/IPV6_PKTINFO
Signed-off-by: Willem de Bruijn <willemb@google.com>
---
net/ipv4/tcp.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
index c239f47..04ea9b2 100644
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c
@@ -1597,8 +1597,12 @@ int tcp_recvmsg(struct kiocb *iocb, struct sock *sk, struct msghdr *msg,
struct sk_buff *skb;
u32 urg_hole = 0;
- if (unlikely(flags & MSG_ERRQUEUE))
- return ip_recv_error(sk, msg, len, addr_len);
+ if (unlikely(flags & MSG_ERRQUEUE)) {
+ if (sk->sk_family == AF_INET6)
+ return ipv6_recv_error(sk, msg, len, addr_len);
+ else
+ return ip_recv_error(sk, msg, len, addr_len);
+ }
if (sk_can_busy_loop(sk) && skb_queue_empty(&sk->sk_receive_queue) &&
(sk->sk_state == TCP_ESTABLISHED))
--
2.1.0.rc2.206.gedb03e5
^ permalink raw reply related
* [PATCH rfc 4/4] net-timestamp: expand txtimestamp test with payload and PKTINFO
From: Willem de Bruijn @ 2014-11-25 17:58 UTC (permalink / raw)
To: netdev; +Cc: davem, luto, richardcochran, Willem de Bruijn
In-Reply-To: <1416938286-14147-1-git-send-email-willemb@google.com>
From: Willem de Bruijn <willemb@google.com>
Test:
new: flag -I requests and prints PKTINFO
new: flag -x prints payload (possibly truncated)
fix: remove pretty print that breaks common flag '-l 1'
Signed-off-by: Willem de Bruijn <willemb@google.com>
----
I did not modify the documentation yet, to avoid conflicting with
the other queued ee_info -> ee_data patch
---
.../networking/timestamping/txtimestamp.c | 89 +++++++++++++++++++---
1 file changed, 77 insertions(+), 12 deletions(-)
diff --git a/Documentation/networking/timestamping/txtimestamp.c b/Documentation/networking/timestamping/txtimestamp.c
index b32fc2a..8946143 100644
--- a/Documentation/networking/timestamping/txtimestamp.c
+++ b/Documentation/networking/timestamping/txtimestamp.c
@@ -46,6 +46,7 @@
#include <netpacket/packet.h>
#include <poll.h>
#include <stdarg.h>
+#include <stdbool.h>
#include <stdint.h>
#include <stdio.h>
#include <stdlib.h>
@@ -58,6 +59,14 @@
#include <time.h>
#include <unistd.h>
+/* ugly hack to work around netinet/in.h and linux/ipv6.h conflicts */
+#ifndef in6_pktinfo
+struct in6_pktinfo {
+ struct in6_addr ipi6_addr;
+ int ipi6_ifindex;
+};
+#endif
+
/* command line parameters */
static int cfg_proto = SOCK_STREAM;
static int cfg_ipproto = IPPROTO_TCP;
@@ -65,6 +74,8 @@ static int cfg_num_pkts = 4;
static int do_ipv4 = 1;
static int do_ipv6 = 1;
static int cfg_payload_len = 10;
+static bool cfg_show_payload;
+static bool cfg_do_pktinfo;
static uint16_t dest_port = 9000;
static struct sockaddr_in daddr;
@@ -131,6 +142,30 @@ static void print_timestamp(struct scm_timestamping *tss, int tstype,
__print_timestamp(tsname, &tss->ts[0], tskey, payload_len);
}
+/* TODO: convert to check_and_print payload once API is stable */
+static void print_payload(char *data, int len)
+{
+ int i;
+
+ if (len > 70)
+ len = 70;
+
+ fprintf(stderr, "payload: ");
+ for (i = 0; i < len; i++)
+ fprintf(stderr, "%02hhx ", data[i]);
+ fprintf(stderr, "\n");
+}
+
+static void print_pktinfo(int family, int ifindex, void *saddr, void *daddr)
+{
+ char sa[INET6_ADDRSTRLEN], da[INET6_ADDRSTRLEN];
+
+ fprintf(stderr, "pktinfo: ifindex=%u src=%s dst=%s\n",
+ ifindex,
+ saddr ? inet_ntop(family, saddr, sa, sizeof(sa)) : "unknown",
+ daddr ? inet_ntop(family, daddr, da, sizeof(da)) : "unknown");
+}
+
static void __poll(int fd)
{
struct pollfd pollfd;
@@ -156,10 +191,9 @@ static void __recv_errmsg_cmsg(struct msghdr *msg, int payload_len)
cm->cmsg_type == SCM_TIMESTAMPING) {
tss = (void *) CMSG_DATA(cm);
} else if ((cm->cmsg_level == SOL_IP &&
- cm->cmsg_type == IP_RECVERR) ||
- (cm->cmsg_level == SOL_IPV6 &&
- cm->cmsg_type == IPV6_RECVERR)) {
-
+ cm->cmsg_type == IP_RECVERR) ||
+ (cm->cmsg_level == SOL_IPV6 &&
+ cm->cmsg_type == IPV6_RECVERR)) {
serr = (void *) CMSG_DATA(cm);
if (serr->ee_errno != ENOMSG ||
serr->ee_origin != SO_EE_ORIGIN_TIMESTAMPING) {
@@ -168,6 +202,16 @@ static void __recv_errmsg_cmsg(struct msghdr *msg, int payload_len)
serr->ee_origin);
serr = NULL;
}
+ } else if (cm->cmsg_level == SOL_IP &&
+ cm->cmsg_type == IP_PKTINFO) {
+ struct in_pktinfo *info = (void *) CMSG_DATA(cm);
+ print_pktinfo(AF_INET, info->ipi_ifindex,
+ &info->ipi_spec_dst, &info->ipi_addr);
+ } else if (cm->cmsg_level == SOL_IPV6 &&
+ cm->cmsg_type == IPV6_PKTINFO) {
+ struct in6_pktinfo *info6 = (void *) CMSG_DATA(cm);
+ print_pktinfo(AF_INET6, info6->ipi6_ifindex,
+ NULL, &info6->ipi6_addr);
} else
fprintf(stderr, "unknown cmsg %d,%d\n",
cm->cmsg_level, cm->cmsg_type);
@@ -206,7 +250,11 @@ static int recv_errmsg(int fd)
if (ret == -1 && errno != EAGAIN)
error(1, errno, "recvmsg");
- __recv_errmsg_cmsg(&msg, ret);
+ if (ret > 0) {
+ __recv_errmsg_cmsg(&msg, ret);
+ if (cfg_show_payload)
+ print_payload(data, cfg_payload_len);
+ }
free(data);
return ret == -1;
@@ -215,9 +263,9 @@ static int recv_errmsg(int fd)
static void do_test(int family, unsigned int opt)
{
char *buf;
- int fd, i, val, total_len;
+ int fd, i, val = 1, total_len;
- if (family == IPPROTO_IPV6 && cfg_proto != SOCK_STREAM) {
+ if (family == AF_INET6 && cfg_proto != SOCK_STREAM) {
/* due to lack of checksum generation code */
fprintf(stderr, "test: skipping datagram over IPv6\n");
return;
@@ -239,7 +287,6 @@ static void do_test(int family, unsigned int opt)
error(1, errno, "socket");
if (cfg_proto == SOCK_STREAM) {
- val = 1;
if (setsockopt(fd, IPPROTO_TCP, TCP_NODELAY,
(char*) &val, sizeof(val)))
error(1, 0, "setsockopt no nagle");
@@ -253,6 +300,18 @@ static void do_test(int family, unsigned int opt)
}
}
+ if (cfg_do_pktinfo) {
+ if (family == AF_INET6) {
+ if (setsockopt(fd, SOL_IPV6, IPV6_RECVPKTINFO,
+ &val, sizeof(val)))
+ error(1, errno, "setsockopt pktinfo ipv6");
+ } else {
+ if (setsockopt(fd, SOL_IP, IP_PKTINFO,
+ &val, sizeof(val)))
+ error(1, errno, "setsockopt pktinfo ipv4");
+ }
+ }
+
opt |= SOF_TIMESTAMPING_SOFTWARE |
SOF_TIMESTAMPING_OPT_ID;
if (setsockopt(fd, SOL_SOCKET, SO_TIMESTAMPING,
@@ -262,8 +321,6 @@ static void do_test(int family, unsigned int opt)
for (i = 0; i < cfg_num_pkts; i++) {
memset(&ts_prev, 0, sizeof(ts_prev));
memset(buf, 'a' + i, total_len);
- buf[total_len - 2] = '\n';
- buf[total_len - 1] = '\0';
if (cfg_proto == SOCK_RAW) {
struct udphdr *udph;
@@ -324,11 +381,13 @@ static void __attribute__((noreturn)) usage(const char *filepath)
" -4: only IPv4\n"
" -6: only IPv6\n"
" -h: show this message\n"
+ " -I: request PKTINFO\n"
" -l N: send N bytes at a time\n"
" -r: use raw\n"
" -R: use raw (IP_HDRINCL)\n"
" -p N: connect to port N\n"
- " -u: use udp\n",
+ " -u: use udp\n"
+ " -x: show payload (up to 70 bytes)\n",
filepath);
exit(1);
}
@@ -338,7 +397,7 @@ static void parse_opt(int argc, char **argv)
int proto_count = 0;
char c;
- while ((c = getopt(argc, argv, "46hl:p:rRu")) != -1) {
+ while ((c = getopt(argc, argv, "46hIl:p:rRux")) != -1) {
switch (c) {
case '4':
do_ipv6 = 0;
@@ -346,6 +405,9 @@ static void parse_opt(int argc, char **argv)
case '6':
do_ipv4 = 0;
break;
+ case 'I':
+ cfg_do_pktinfo = true;
+ break;
case 'r':
proto_count++;
cfg_proto = SOCK_RAW;
@@ -367,6 +429,9 @@ static void parse_opt(int argc, char **argv)
case 'p':
dest_port = strtoul(optarg, NULL, 10);
break;
+ case 'x':
+ cfg_show_payload = true;
+ break;
case 'h':
default:
usage(argv[0]);
--
2.1.0.rc2.206.gedb03e5
^ permalink raw reply related
* Re: [PATCH] net-timestamp: Fix a documentation typo
From: Willem de Bruijn @ 2014-11-25 18:01 UTC (permalink / raw)
To: Andy Lutomirski; +Cc: Network Development, David S. Miller
In-Reply-To: <CA+FuTSe8pjpUUpStfEHsuZ2Row9=MHVPH0S-MjUxiauBSNMOsg@mail.gmail.com>
I just sent an initial patch set for comments.
On Mon, Nov 24, 2014 at 5:38 PM, Willem de Bruijn <willemb@google.com> wrote:
>>>> Is user code supposed to rely on this and, further, on the fact that the
>>>> counter starts at zero? If not, how else is user code supposed to match
>>>> outgoing data to timestamps?
>>>
>>> That is correct. The per-socket counter is reset when
>>> SOF_TIMESTAMPING_OPT_ID is set. On datagram sockets, it returns the
>>> packet number since the reset. On stream sockets, it returns the byte
>>> offset since the reset.
>>>
>>
>> It might be worth tweaking the docs at some point to make this clearer.
>
> Good point. The commit message is apparently more informative than the
> actual documentation.
I did not update the documentation yet, as that would conflict with your
patch, Andy.
>>>> Also, is it intentional that the payload data associated with the tx
>>>> timestamp is (I think) the full outgoing packet including lower-layer
>>>> headers?
>>>
>>> Absolutely not. I'll look into that right away. It doesn't on ACK, and
>>> should certainly not expose this info in the other cases, either.
>>
>> Then I won't start trying to decode it :)
>
> The datagram feature existed before I added the counter and stream
> support, so returning the entire packet in that case is legacy
> behavior, I suppose. I did not intend to expose network headers for
> the new stream socket interface, though.
Since the interface with headers is legacy and cannot be changed,
you could actually use it. The counter + PKTINFO hopefully give the
same information in a cleaner way.
>> TBH, all I looked at was the packet size, which matched the full
>> link-layer packet.
>>
>> Also, the address returned by recvmsg appeared to be garbage instead
>> of 0.0.0.0 (or something meaningful, whatever that would be).
>>
>>>
>>>> And, finally, would it be possible to attach IP_PKTINFO to the looped
>>>> timestamp? That way I could finally update my fancy ping program to
>>>> track which outgoing interface was used for a request.
>>>
>>> If socket option IP_PKTINFO is set, you want to receive in_pktinfo for
>>> any packet that happens to be queued onto the error queue? Both
>>> SKB_EXT_ERR(skb) and PKTINFO_SKB_CB(skb) use the control block to
>>> store data that is later encoded in a cmsg, so there may not be enough
>>> room to hold both. I'll take a look.
>>
>> I don't really care what the mechanism is, but it would be really nice
>> if I could see what interface the send timestamp is associated with.
>
> Okay. I'll see if I can cook something up.
I decided to make the feature independent from the timestamps. There
may be other uses for finding out the egress device. The two metadata
structures are looped back together with the same payload, though, so
it still allows correlation of all fields.
^ 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