* Re: [PATCH net-next 2/4] tc/act: remove unneeded RCU lock in action callback
From: Paolo Abeni @ 2018-07-13 15:00 UTC (permalink / raw)
To: Daniel Borkmann, netdev
Cc: Jamal Hadi Salim, Cong Wang, Jiri Pirko, Alexei Starovoitov,
Marcelo Ricardo Leitner
In-Reply-To: <b65d2de5-f2be-4ceb-d50e-61a15735b1b4@iogearbox.net>
On Fri, 2018-07-13 at 16:41 +0200, Daniel Borkmann wrote:
> On 07/13/2018 04:26 PM, Paolo Abeni wrote:
> > On Fri, 2018-07-13 at 16:08 +0200, Daniel Borkmann wrote:
> > > On 07/13/2018 11:55 AM, Paolo Abeni wrote:
> > > > Each lockless action currently does its own RCU locking in ->act().
> > > > This is allows using plain RCU accessor, even if the context
> > > > is really RCU BH.
> > > >
> > > > This change drops the per action RCU lock, replace the accessors
> > > > with _bh variant, cleans up a bit the surronding code and documents
> > > > the RCU status in the relevant header.
> > > > No functional nor performance change is intended.
> > > >
> > > > The goal of this patch is clarifying that the RCU critical section
> > > > used by the tc actions extends up to the classifier's caller.
> > > >
> > > > Signed-off-by: Paolo Abeni <pabeni@redhat.com>
> > >
> > > [...]
> > > > diff --git a/net/sched/act_bpf.c b/net/sched/act_bpf.c
> > > > index 06f743d8ed41..ac20266460c0 100644
> > > > --- a/net/sched/act_bpf.c
> > > > +++ b/net/sched/act_bpf.c
> > > > @@ -45,8 +45,7 @@ static int tcf_bpf(struct sk_buff *skb, const struct tc_action *act,
> > > > tcf_lastuse_update(&prog->tcf_tm);
> > > > bstats_cpu_update(this_cpu_ptr(prog->common.cpu_bstats), skb);
> > > >
> > > > - rcu_read_lock();
> > > > - filter = rcu_dereference(prog->filter);
> > > > + filter = rcu_dereference_bh(prog->filter);
> > > > if (at_ingress) {
> > > > __skb_push(skb, skb->mac_len);
> > > > bpf_compute_data_pointers(skb);
> > > > @@ -56,7 +55,6 @@ static int tcf_bpf(struct sk_buff *skb, const struct tc_action *act,
> > > > bpf_compute_data_pointers(skb);
> > > > filter_res = BPF_PROG_RUN(filter, skb);
> > > > }
> > > > - rcu_read_unlock();
> > >
> > > This conversion is not correct, BPF itself relies on RCU but not RCU-bh flavor.
> > > You might probably see a splat if you do e.g. a map lookup with this change in
> > > interpreter mode on tx side.
> >
> > Thank you for your review.
> >
> > I actually tested with lockdep, and lockdep is happy about it.
> >
> > The not so nice fact is that many TC modules already use plain RCU
> > primitives in the control path (call_rcu, kfree_rcu, etc.) and
> > rcu_derefence_bh() in the datapath (e.g. all the classifiers). AFACS,
> > despite the mix, this use is safe.
>
> Hmm, so out of __dev_queue_xmit() we do the RCU-bh read-side. We call
> into sch_handle_egress() which calls into tcf_classify() which may be
> a matchall one e.g. mall_classify(). It invokes tcf_exts_exec() that
> does the a->ops->act() which is the tcf_bpf() from here. If you then
> call a helper like bpf_map_lookup_elem(), there's a WARN_ON_ONCE() for
> !rcu_read_lock_held() since all of BPF is under normal RCU flavor. Why
> would that not trigger?
Because the basic sample I used did not call any other ebpf helper
beyond bpf_redirect(), I guess.
I see rcu_read_lock() is still needed here, thanks.
As a side note, after:
rcu_read_lock_bh()
rcu_read_lock();
both rcu_dereference() and rcu_derefernce_bh() are considered fine by
lockdep.
Cheers,
Paolo
^ permalink raw reply
* Re: [PATCH nf-next 1/2] ipv6: remove dependency of nf_defrag_ipv6 on ipv6 module
From: kbuild test robot @ 2018-07-13 14:55 UTC (permalink / raw)
To: Florian Westphal
Cc: kbuild-all, netfilter-devel, netdev, arnd, Florian Westphal
In-Reply-To: <20180712143547.2194-1-fw@strlen.de>
Hi Florian,
I love your patch! Perhaps something to improve:
[auto build test WARNING on nf-next/master]
url: https://github.com/0day-ci/linux/commits/Florian-Westphal/ipv6-remove-dependency-of-nf_defrag_ipv6-on-ipv6-module/20180713-064909
base: https://git.kernel.org/pub/scm/linux/kernel/git/pablo/nf-next.git master
reproduce:
# apt-get install sparse
make ARCH=x86_64 allmodconfig
make C=1 CF=-D__CHECK_ENDIAN__
sparse warnings: (new ones prefixed by >>)
>> net/ieee802154/6lowpan/reassembly.c:57:14: sparse: unknown member
>> net/ieee802154/6lowpan/reassembly.c:57:14: sparse: cast from unknown type
net/ieee802154/6lowpan/reassembly.c:58:15: sparse: using member 'q' in incomplete struct frag_queue
net/ieee802154/6lowpan/reassembly.c:58:15: sparse: cast from unknown type
net/ieee802154/6lowpan/reassembly.c:60:22: sparse: using member 'q' in incomplete struct frag_queue
net/ieee802154/6lowpan/reassembly.c:62:15: sparse: using member 'q' in incomplete struct frag_queue
net/ieee802154/6lowpan/reassembly.c:65:27: sparse: using member 'q' in incomplete struct frag_queue
net/ieee802154/6lowpan/reassembly.c:67:24: sparse: using member 'q' in incomplete struct frag_queue
net/ieee802154/6lowpan/reassembly.c:68:26: sparse: using member 'q' in incomplete struct frag_queue
>> net/ieee802154/6lowpan/reassembly.c:57:14: sparse: unknown expression (30 46)
net/ieee802154/6lowpan/reassembly.c:58:15: sparse: unknown expression (8 46)
>> net/ieee802154/6lowpan/reassembly.c:60:18: sparse: call with no type!
net/ieee802154/6lowpan/reassembly.c:62:18: sparse: unknown expression (8 46)
net/ieee802154/6lowpan/reassembly.c:65:23: sparse: call with no type!
net/ieee802154/6lowpan/reassembly.c:67:20: sparse: call with no type!
net/ieee802154/6lowpan/reassembly.c:68:22: sparse: call with no type!
In file included from include/linux/kernel.h:10:0,
from include/linux/list.h:9,
from include/linux/random.h:10,
from include/linux/net.h:22,
from net/ieee802154/6lowpan/reassembly.c:17:
net/ieee802154/6lowpan/reassembly.c: In function 'lowpan_frag_expire':
include/linux/kernel.h:964:51: error: dereferencing pointer to incomplete type 'struct frag_queue'
BUILD_BUG_ON_MSG(!__same_type(*(ptr), ((type *)0)->member) && 25- ^
include/linux/compiler.h:316:19: note: in definition of macro '__compiletime_assert'
bool __cond = !(condition); 28- ^~~~~~~~~
include/linux/compiler.h:339:2: note: in expansion of macro '_compiletime_assert'
_compiletime_assert(condition, msg, __compiletime_assert_, __LINE__)
^~~~~~~~~~~~~~~~~~~
include/linux/build_bug.h:45:37: note: in expansion of macro 'compiletime_assert'
#define BUILD_BUG_ON_MSG(cond, msg) compiletime_assert(!(cond), msg)
^~~~~~~~~~~~~~~~~~
include/linux/kernel.h:964:2: note: in expansion of macro 'BUILD_BUG_ON_MSG'
BUILD_BUG_ON_MSG(!__same_type(*(ptr), ((type *)0)->member) && 37- ^~~~~~~~~~~~~~~~
include/linux/kernel.h:964:20: note: in expansion of macro '__same_type'
BUILD_BUG_ON_MSG(!__same_type(*(ptr), ((type *)0)->member) && 40- ^~~~~~~~~~~
net/ieee802154/6lowpan/reassembly.c:57:7: note: in expansion of macro 'container_of'
fq = container_of(frag, struct frag_queue, q);
^~~~~~~~~~~~
In file included from include/linux/compiler_types.h:58:0,
from <command-line>:0:
include/linux/compiler-gcc.h:170:2: error: invalid use of undefined type 'struct frag_queue'
__builtin_offsetof(a, b)
^
include/linux/stddef.h:17:32: note: in expansion of macro '__compiler_offsetof'
#define offsetof(TYPE, MEMBER) __compiler_offsetof(TYPE, MEMBER)
^~~~~~~~~~~~~~~~~~~
include/linux/kernel.h:967:21: note: in expansion of macro 'offsetof'
((type *)(__mptr - offsetof(type, member))); })
^~~~~~~~
net/ieee802154/6lowpan/reassembly.c:57:7: note: in expansion of macro 'container_of'
fq = container_of(frag, struct frag_queue, q);
^~~~~~~~~~~~
net/ieee802154/6lowpan/reassembly.c: In function 'lowpan_net_frag_init':
net/ieee802154/6lowpan/reassembly.c:611:30: error: invalid application of 'sizeof' to incomplete type 'struct frag_queue'
lowpan_frags.qsize = sizeof(struct frag_queue);
^~~~~~
vim +57 net/ieee802154/6lowpan/reassembly.c
7240cdec6 net/ieee802154/reassembly.c Alexander Aring 2014-02-28 50
78802011f net/ieee802154/6lowpan/reassembly.c Kees Cook 2017-10-16 51 static void lowpan_frag_expire(struct timer_list *t)
7240cdec6 net/ieee802154/reassembly.c Alexander Aring 2014-02-28 52 {
78802011f net/ieee802154/6lowpan/reassembly.c Kees Cook 2017-10-16 53 struct inet_frag_queue *frag = from_timer(frag, t, timer);
7240cdec6 net/ieee802154/reassembly.c Alexander Aring 2014-02-28 54 struct frag_queue *fq;
7240cdec6 net/ieee802154/reassembly.c Alexander Aring 2014-02-28 55 struct net *net;
7240cdec6 net/ieee802154/reassembly.c Alexander Aring 2014-02-28 56
78802011f net/ieee802154/6lowpan/reassembly.c Kees Cook 2017-10-16 @57 fq = container_of(frag, struct frag_queue, q);
7240cdec6 net/ieee802154/reassembly.c Alexander Aring 2014-02-28 @58 net = container_of(fq->q.net, struct net, ieee802154_lowpan.frags);
7240cdec6 net/ieee802154/reassembly.c Alexander Aring 2014-02-28 59
177943260 net/ieee802154/reassembly.c Florian Westphal 2014-03-13 @60 spin_lock(&fq->q.lock);
177943260 net/ieee802154/reassembly.c Florian Westphal 2014-03-13 61
06aa8b8a0 net/ieee802154/reassembly.c Nikolay Aleksandrov 2014-08-01 62 if (fq->q.flags & INET_FRAG_COMPLETE)
177943260 net/ieee802154/reassembly.c Florian Westphal 2014-03-13 63 goto out;
177943260 net/ieee802154/reassembly.c Florian Westphal 2014-03-13 64
093ba7291 net/ieee802154/6lowpan/reassembly.c Eric Dumazet 2018-03-31 65 inet_frag_kill(&fq->q);
177943260 net/ieee802154/reassembly.c Florian Westphal 2014-03-13 66 out:
177943260 net/ieee802154/reassembly.c Florian Westphal 2014-03-13 67 spin_unlock(&fq->q.lock);
093ba7291 net/ieee802154/6lowpan/reassembly.c Eric Dumazet 2018-03-31 68 inet_frag_put(&fq->q);
7240cdec6 net/ieee802154/reassembly.c Alexander Aring 2014-02-28 69 }
7240cdec6 net/ieee802154/reassembly.c Alexander Aring 2014-02-28 70
:::::: The code at line 57 was first introduced by commit
:::::: 78802011fbe34331bdef6f2dfb1634011f0e4c32 inet: frags: Convert timers to use timer_setup()
:::::: TO: Kees Cook <keescook@chromium.org>
:::::: CC: David S. Miller <davem@davemloft.net>
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
^ permalink raw reply
* MACsec hardware offloading
From: Antoine Tenart @ 2018-07-13 14:46 UTC (permalink / raw)
To: davem, sd, f.fainelli, andrew
Cc: thomas.petazzoni, alexandre.belloni, allan.nielsen, netdev
Hello,
Linux has a software implementation of the MACsec standard but so far,
to my knowledge, no hardware offloading support was developed and sent
upstream. I am working on a PHY that has an embedded MACsec engine and
would like to add support for MACsec hardware offloading in Linux in
order to support this PHY capability.
If you think I'm missing someone who would be interested in
participating to the discussion, please feel free to Cc her/him.
My main idea would be to reuse and leverage the actual software
implementation in Linux. The advantages would be to reuse existing code,
data structure and to configure this using the exact same tools from
user-space. Also the MACsec state could be kept in the software
implementation (as it's done now), with stateless hardware ops.
Those hooks would be called from the (at least) genl_ops of the
software implementation.
I made a very simple and early version, more to get an idea of what
needed to be done than to actually having something ready, at:
https://github.com/atenart/linux/commit/2dc30abb2d349402ac78a851de09d6e0791e7bf1
One important point about adding MACsec offloading in Linux is this can
be done in either the MAC or the PHY. While I'll be working on making
this work in a PHY, I know for sure some MAC do have the same capability
(including for example the Intel ixgbe NIC). This means the offloading
interface should be done for both those devices in mind. It also means
we can have cases were for a given link both the PHY and the MAC have
the MACsec offloading capability. We'll probably have to make this a
runtime configuration option, so that the user can chose which of the
three implementation is responsible of the MACsec operations (PHY, MAC
or software). (We can also think of cases were different security
associations could be handled by different layers).
This is the design I have in mind. As of now I mainly worked on my PHY
hardware initialization and configuration (and still have work to do)
but I think it's time to discuss as well the generic MACsec offloading
plan as well. I'd love to have your opinion and suggestions, whether
you think this is the right approach or not.
I also do have some open questions:
- When having more than a single MACsec h/w offloading provider (say a
network engine and a PHY), there probably should be a way to decide
which one to use. The s/w implementation should be part of this choice
as well. I'm not sure how to do this and what should be the default
setting.
The users will also want to switch the layer being used to handle a
particular security association, or to move all the associations to
a given layer, at runtime.
- The MACsec representation to the user is a virtual interface. When
using the s/w implementation it's easy to know when to use MACsec on
the frames: it depends on which interface is used (the MACsec virtual
one, or the physical one, or another virtual interface not being a
child to the MACsec one). But with h/w offloading (at least when it's
done at the PHY level), it's impossible to know onto which interface
the frame was injected. This could be an issue as some frames might
need to be sent without the MACsec layer doing anything.
Thanks a lot!
Antoine
--
Antoine Ténart, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com
^ permalink raw reply
* Re: [PATCH net] net/ipv6: Do not allow device only routes via the multipath API
From: David Ahern @ 2018-07-13 14:45 UTC (permalink / raw)
To: Stefano Brivio, dsahern; +Cc: netdev, eric.dumazet
In-Reply-To: <20180713132845.64854ff3@epycfail>
On 7/13/18 7:28 AM, Stefano Brivio wrote:
> On Thu, 12 Jul 2018 14:48:23 -0700
> dsahern@kernel.org wrote:
>
>> @@ -4388,6 +4388,13 @@ static int ip6_route_multipath_add(struct fib6_config *cfg,
>> rt = NULL;
>> goto cleanup;
>> }
>> + if (!rt6_qualify_for_ecmp(rt)) {
>> + err = EINVAL;
>
> Shouldn't this be -EINVAL, just for consistency? E.g. we might return a
> -ENOMEM from ip6_route_info_append(), etc.
>
oops, yes, missing the '-'. thanks for catching that.
^ permalink raw reply
* [PATCH net-next] net: mscc: simplify retrieving the tag type from the frame header
From: Antoine Tenart @ 2018-07-13 14:57 UTC (permalink / raw)
To: davem
Cc: Antoine Tenart, f.fainelli, andrew, netdev, linux-kernel,
thomas.petazzoni, alexandre.belloni, quentin.schulz,
allan.nielsen
The tag type in the frame extraction header is only a bit wide. There's
no need to use GENMASK when retrieving the information. This patch
simplify the code by dropping GENMASK and using BIT instead.
Signed-off-by: Antoine Tenart <antoine.tenart@bootlin.com>
---
drivers/net/ethernet/mscc/ocelot_board.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/mscc/ocelot_board.c b/drivers/net/ethernet/mscc/ocelot_board.c
index 18df7d934e81..26bb3b18f3be 100644
--- a/drivers/net/ethernet/mscc/ocelot_board.c
+++ b/drivers/net/ethernet/mscc/ocelot_board.c
@@ -29,7 +29,7 @@ static int ocelot_parse_ifh(u32 *ifh, struct frame_info *info)
info->port = (ifh[2] & GENMASK(14, 11)) >> 11;
info->cpuq = (ifh[3] & GENMASK(27, 20)) >> 20;
- info->tag_type = (ifh[3] & GENMASK(16, 16)) >> 16;
+ info->tag_type = (ifh[3] & BIT(16)) >> 16;
info->vid = ifh[3] & GENMASK(11, 0);
return 0;
--
2.17.1
^ permalink raw reply related
* Re: [PATCH net-next 2/4] tc/act: remove unneeded RCU lock in action callback
From: Daniel Borkmann @ 2018-07-13 14:41 UTC (permalink / raw)
To: Paolo Abeni, netdev
Cc: Jamal Hadi Salim, Cong Wang, Jiri Pirko, Alexei Starovoitov,
Marcelo Ricardo Leitner
In-Reply-To: <3d8ee4dbbc106ac7621a7789c003e46dd883932e.camel@redhat.com>
On 07/13/2018 04:26 PM, Paolo Abeni wrote:
> On Fri, 2018-07-13 at 16:08 +0200, Daniel Borkmann wrote:
>> On 07/13/2018 11:55 AM, Paolo Abeni wrote:
>>> Each lockless action currently does its own RCU locking in ->act().
>>> This is allows using plain RCU accessor, even if the context
>>> is really RCU BH.
>>>
>>> This change drops the per action RCU lock, replace the accessors
>>> with _bh variant, cleans up a bit the surronding code and documents
>>> the RCU status in the relevant header.
>>> No functional nor performance change is intended.
>>>
>>> The goal of this patch is clarifying that the RCU critical section
>>> used by the tc actions extends up to the classifier's caller.
>>>
>>> Signed-off-by: Paolo Abeni <pabeni@redhat.com>
>>
>> [...]
>>> diff --git a/net/sched/act_bpf.c b/net/sched/act_bpf.c
>>> index 06f743d8ed41..ac20266460c0 100644
>>> --- a/net/sched/act_bpf.c
>>> +++ b/net/sched/act_bpf.c
>>> @@ -45,8 +45,7 @@ static int tcf_bpf(struct sk_buff *skb, const struct tc_action *act,
>>> tcf_lastuse_update(&prog->tcf_tm);
>>> bstats_cpu_update(this_cpu_ptr(prog->common.cpu_bstats), skb);
>>>
>>> - rcu_read_lock();
>>> - filter = rcu_dereference(prog->filter);
>>> + filter = rcu_dereference_bh(prog->filter);
>>> if (at_ingress) {
>>> __skb_push(skb, skb->mac_len);
>>> bpf_compute_data_pointers(skb);
>>> @@ -56,7 +55,6 @@ static int tcf_bpf(struct sk_buff *skb, const struct tc_action *act,
>>> bpf_compute_data_pointers(skb);
>>> filter_res = BPF_PROG_RUN(filter, skb);
>>> }
>>> - rcu_read_unlock();
>>
>> This conversion is not correct, BPF itself relies on RCU but not RCU-bh flavor.
>> You might probably see a splat if you do e.g. a map lookup with this change in
>> interpreter mode on tx side.
>
> Thank you for your review.
>
> I actually tested with lockdep, and lockdep is happy about it.
>
> The not so nice fact is that many TC modules already use plain RCU
> primitives in the control path (call_rcu, kfree_rcu, etc.) and
> rcu_derefence_bh() in the datapath (e.g. all the classifiers). AFACS,
> despite the mix, this use is safe.
Hmm, so out of __dev_queue_xmit() we do the RCU-bh read-side. We call
into sch_handle_egress() which calls into tcf_classify() which may be
a matchall one e.g. mall_classify(). It invokes tcf_exts_exec() that
does the a->ops->act() which is the tcf_bpf() from here. If you then
call a helper like bpf_map_lookup_elem(), there's a WARN_ON_ONCE() for
!rcu_read_lock_held() since all of BPF is under normal RCU flavor. Why
would that not trigger? RCU != RCU-bh. Only the rcu_read_lock_bh_held()
would hold true.
Thanks,
Daniel
^ permalink raw reply
* Re: [PATCH net-next 3/4] net/sched: refactor TC_ACT_REDIRECT handling
From: Paolo Abeni @ 2018-07-13 14:37 UTC (permalink / raw)
To: Daniel Borkmann, netdev
Cc: Jamal Hadi Salim, Cong Wang, Jiri Pirko, Alexei Starovoitov,
Marcelo Ricardo Leitner
In-Reply-To: <5937186c-0fab-953f-a305-e6379a52fda8@iogearbox.net>
On Fri, 2018-07-13 at 16:13 +0200, Daniel Borkmann wrote:
> On 07/13/2018 11:55 AM, Paolo Abeni wrote:
> > This patch changes the TC_ACT_REDIRECT code path to allow
> > providing the redirect parameters via the tcf_result argument.
> >
> > Such union is expanded to host the redirect device, the redirect
> > direction (ingress/egress) and the stats to be updated on error
> > conditions.
> >
> > Actions/classifiers using TC_ACT_REDIRECT can either:
> > * fill the tcf_result redirect related fields
> > * clear such fields and use the bpf per cpu redirect info
> >
> > skb_do_redirect now tries to fetch the relevant data from tcf_result
> > and fall back to access redirect info. It also updates the stats
> > accordingly to the redirect result, if provided by the caller.
> >
> > This will allow using the TC_ACT_REDIRECT action in more places in
> > the next patch.
> >
> > Signed-off-by: Paolo Abeni <pabeni@redhat.com>
> > ---
> > include/net/sch_generic.h | 15 ++++++++++++++-
> > net/core/dev.c | 4 ++--
> > net/core/filter.c | 29 +++++++++++++++++++++++------
> > net/core/lwt_bpf.c | 5 ++++-
> > net/sched/act_bpf.c | 4 +++-
> > net/sched/cls_bpf.c | 8 +++++---
> > 6 files changed, 51 insertions(+), 14 deletions(-)
> >
> > diff --git a/include/net/sch_generic.h b/include/net/sch_generic.h
> > index 056dc1083aa3..dd9e00d017b3 100644
> > --- a/include/net/sch_generic.h
> > +++ b/include/net/sch_generic.h
> > @@ -235,9 +235,22 @@ struct tcf_result {
> > u32 classid;
> > };
> > const struct tcf_proto *goto_tp;
> > +
> > + /* used by the TC_ACT_REDIRECT action */
> > + struct {
> > + /* device and direction, or 0 bpf redirect */
> > + long dev_ingress;
> > + struct gnet_stats_queue *qstats;
> > + };
> > };
> > };
> >
> > +#define TCF_RESULT_REDIR_DEV(res) \
> > + ((struct net_device *)((res)->dev_ingress & ~1))
> > +#define TCF_RESULT_REDIR_INGRESS(res) ((res)->dev_ingress & 1)
> > +#define TCF_RESULT_SET_REDIRECT(res, dev, ingress) \
> > + ((res)->dev_ingress = (long)(dev) | (!!(ingress)))
> > +
> > struct tcf_proto_ops {
> > struct list_head head;
> > char kind[IFNAMSIZ];
> > @@ -543,7 +556,7 @@ struct Qdisc *qdisc_create_dflt(struct netdev_queue *dev_queue,
> > struct netlink_ext_ack *extack);
> > void __qdisc_calculate_pkt_len(struct sk_buff *skb,
> > const struct qdisc_size_table *stab);
> > -int skb_do_redirect(struct sk_buff *);
> > +int skb_do_redirect(struct sk_buff *skb, struct tcf_result *res);
> >
> > static inline void skb_reset_tc(struct sk_buff *skb)
> > {
> > diff --git a/net/core/dev.c b/net/core/dev.c
> > index 14a748ee8cc9..a283dbfde30c 100644
> > --- a/net/core/dev.c
> > +++ b/net/core/dev.c
> > @@ -3538,7 +3538,7 @@ sch_handle_egress(struct sk_buff *skb, int *ret, struct net_device *dev)
> > return NULL;
> > case TC_ACT_REDIRECT:
> > /* No need to push/pop skb's mac_header here on egress! */
> > - skb_do_redirect(skb);
> > + skb_do_redirect(skb, &cl_res);
> > *ret = NET_XMIT_SUCCESS;
> > return NULL;
> > default:
> > @@ -4600,7 +4600,7 @@ sch_handle_ingress(struct sk_buff *skb, struct packet_type **pt_prev, int *ret,
> > * redirecting to another netdev
> > */
> > __skb_push(skb, skb->mac_len);
> > - skb_do_redirect(skb);
> > + skb_do_redirect(skb, &cl_res);
> > return NULL;
> > default:
> > break;
> > diff --git a/net/core/filter.c b/net/core/filter.c
> > index b9ec916f4e3a..4f64cf5189e6 100644
> > --- a/net/core/filter.c
> > +++ b/net/core/filter.c
> > @@ -2062,19 +2062,36 @@ BPF_CALL_2(bpf_redirect, u32, ifindex, u64, flags)
> > return TC_ACT_REDIRECT;
> > }
> >
> > -int skb_do_redirect(struct sk_buff *skb)
> > +int skb_do_redirect(struct sk_buff *skb, struct tcf_result *res)
> > {
> > - struct redirect_info *ri = this_cpu_ptr(&redirect_info);
> > + struct gnet_stats_queue *stats;
> > struct net_device *dev;
> > + int ret, flags;
> >
> > - dev = dev_get_by_index_rcu(dev_net(skb->dev), ri->ifindex);
> > - ri->ifindex = 0;
> > + if (!res->dev_ingress) {
> > + struct redirect_info *ri = this_cpu_ptr(&redirect_info);
> > +
> > + dev = dev_get_by_index_rcu(dev_net(skb->dev), ri->ifindex);
> > + flags = ri->flags;
> > + ri->ifindex = 0;
> > + stats = NULL;
> > + } else {
> > + dev = TCF_RESULT_REDIR_DEV(res);
> > + flags = TCF_RESULT_REDIR_INGRESS(res) ? BPF_F_INGRESS : 0;
> > + stats = res->qstats;
> > + }
> > if (unlikely(!dev)) {
> > kfree_skb(skb);
> > - return -EINVAL;
> > + ret = -EINVAL;
> > + goto out;
> > }
> >
> > - return __bpf_redirect(skb, dev, ri->flags);
> > + ret = __bpf_redirect(skb, dev, flags);
> > +
> > +out:
> > + if (ret && stats)
> > + qstats_overlimit_inc(res->qstats);
> > + return ret;
> > }
> >
> > static const struct bpf_func_proto bpf_redirect_proto = {
> > diff --git a/net/core/lwt_bpf.c b/net/core/lwt_bpf.c
> > index e7e626fb87bb..8dde1093994a 100644
> > --- a/net/core/lwt_bpf.c
> > +++ b/net/core/lwt_bpf.c
> > @@ -65,7 +65,10 @@ static int run_lwt_bpf(struct sk_buff *skb, struct bpf_lwt_prog *lwt,
> > lwt->name ? : "<unknown>");
> > ret = BPF_OK;
> > } else {
> > - ret = skb_do_redirect(skb);
> > + struct tcf_result res;
> > +
> > + res.dev_ingress = 0;
> > + ret = skb_do_redirect(skb, &res);
> > if (ret == 0)
> > ret = BPF_REDIRECT;
> > }
> > diff --git a/net/sched/act_bpf.c b/net/sched/act_bpf.c
> > index ac20266460c0..6fd46b691181 100644
> > --- a/net/sched/act_bpf.c
> > +++ b/net/sched/act_bpf.c
> > @@ -67,10 +67,12 @@ static int tcf_bpf(struct sk_buff *skb, const struct tc_action *act,
> > * returned.
> > */
> > switch (filter_res) {
> > + case TC_ACT_REDIRECT:
> > + res->dev_ingress = 0;
> > + /* fall-through */
> > case TC_ACT_PIPE:
> > case TC_ACT_RECLASSIFY:
> > case TC_ACT_OK:
> > - case TC_ACT_REDIRECT:
> > action = filter_res;
> > break;
> > case TC_ACT_SHOT:
> > diff --git a/net/sched/cls_bpf.c b/net/sched/cls_bpf.c
> > index 66e0ac9811f9..f0fb7ded8fe2 100644
> > --- a/net/sched/cls_bpf.c
> > +++ b/net/sched/cls_bpf.c
> > @@ -65,14 +65,16 @@ static const struct nla_policy bpf_policy[TCA_BPF_MAX + 1] = {
> > .len = sizeof(struct sock_filter) * BPF_MAXINSNS },
> > };
> >
> > -static int cls_bpf_exec_opcode(int code)
> > +static int cls_bpf_exec_opcode(int code, struct tcf_result *res)
> > {
> > switch (code) {
> > + case TC_ACT_REDIRECT:
> > + res->dev_ingress = 0;
> > + /* fall-through */
> > case TC_ACT_OK:
> > case TC_ACT_SHOT:
> > case TC_ACT_STOLEN:
> > case TC_ACT_TRAP:
> > - case TC_ACT_REDIRECT:
> > case TC_ACT_UNSPEC:
> > return code;
> > default:
> > @@ -113,7 +115,7 @@ static int cls_bpf_classify(struct sk_buff *skb, const struct tcf_proto *tp,
> > res->classid = TC_H_MAJ(prog->res.classid) |
> > qdisc_skb_cb(skb)->tc_classid;
> >
> > - ret = cls_bpf_exec_opcode(filter_res);
> > + ret = cls_bpf_exec_opcode(filter_res, res);
> > if (ret == TC_ACT_UNSPEC)
> > continue;
> > break;
> >
>
> Can't we just export the struct redirect_info and let others like
> act_mirred use it, then we wouldn't need all these extra changes in
> fast path?
Thank you for the feedback.
The use of tcf_result allows passing to the redirect helper the stats
to be updated, and avoids an additional dev lookup for the mirred
datapath.
Some changes are necessary to make the skb_do_redirect() function more
generic, and code duplication could be reduced with some additional
helper. Do you have so strong opinion against that?
Thanks,
Paolo
^ permalink raw reply
* [PATCH v2 nf-next 2/2] netfilter: fix IPV6=m CONNTRACK=y link failure
From: Florian Westphal @ 2018-07-13 14:27 UTC (permalink / raw)
To: netfilter-devel; +Cc: netdev, Florian Westphal
In-Reply-To: <20180713142754.23349-1-fw@strlen.de>
IPV6=m
DEFRAG_IPV6=m
CONNTRACK=y yields:
net/netfilter/nf_conntrack_proto.o: In function `nf_ct_netns_do_get':
net/netfilter/nf_conntrack_proto.c:802: undefined reference to `nf_defrag_ipv6_enable'
net/netfilter/nf_conntrack_proto.o:(.rodata+0x640): undefined reference to `nf_conntrack_l4proto_icmpv6'
After previous patch, DEFRAG_IPV6 and IPV6 are no longer retain any
dependencies, so we can tell Kconfig DEFRAG_IPV6 needs to be built-in as
well, this resolves missing nf_defrag_ipv6_enable.
Second error can be fixed via makefile, just make sure conntrack_proto_ipv6
is part of conntrack module.
based on earlier patch from Arnd Bergmann.
Fixes: 66c524acfb5186 ("netfilter: conntrack: remove l3proto abstraction")
Reported-by: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Florian Westphal <fw@strlen.de>
---
no changes.
net/ipv6/netfilter/Kconfig | 7 ++-----
net/netfilter/Kconfig | 2 +-
net/netfilter/Makefile | 2 +-
3 files changed, 4 insertions(+), 7 deletions(-)
diff --git a/net/ipv6/netfilter/Kconfig b/net/ipv6/netfilter/Kconfig
index 07516d5c2f80..339d0762b027 100644
--- a/net/ipv6/netfilter/Kconfig
+++ b/net/ipv6/netfilter/Kconfig
@@ -5,10 +5,6 @@
menu "IPv6: Netfilter Configuration"
depends on INET && IPV6 && NETFILTER
-config NF_DEFRAG_IPV6
- tristate
- default n
-
config NF_SOCKET_IPV6
tristate "IPv6 socket lookup support"
help
@@ -349,6 +345,7 @@ config IP6_NF_TARGET_NPT
endif # IP6_NF_NAT
endif # IP6_NF_IPTABLES
-
endmenu
+config NF_DEFRAG_IPV6
+ tristate
diff --git a/net/netfilter/Kconfig b/net/netfilter/Kconfig
index 6c65d756e603..e0ab50c58dc4 100644
--- a/net/netfilter/Kconfig
+++ b/net/netfilter/Kconfig
@@ -50,7 +50,7 @@ config NF_CONNTRACK
tristate "Netfilter connection tracking support"
default m if NETFILTER_ADVANCED=n
select NF_DEFRAG_IPV4
- select NF_DEFRAG_IPV6 if IPV6
+ select NF_DEFRAG_IPV6 if IPV6 != n
help
Connection tracking keeps a record of what packets have passed
through your machine, in order to figure out how they are related
diff --git a/net/netfilter/Makefile b/net/netfilter/Makefile
index 0b3851e825fa..53bd1ed1228a 100644
--- a/net/netfilter/Makefile
+++ b/net/netfilter/Makefile
@@ -6,7 +6,7 @@ nf_conntrack-y := nf_conntrack_core.o nf_conntrack_standalone.o nf_conntrack_exp
nf_conntrack_proto_icmp.o \
nf_conntrack_extend.o nf_conntrack_acct.o nf_conntrack_seqadj.o
-nf_conntrack-$(CONFIG_IPV6) += nf_conntrack_proto_icmpv6.o
+nf_conntrack-$(subst m,y,$(CONFIG_IPV6)) += nf_conntrack_proto_icmpv6.o
nf_conntrack-$(CONFIG_NF_CONNTRACK_TIMEOUT) += nf_conntrack_timeout.o
nf_conntrack-$(CONFIG_NF_CONNTRACK_TIMESTAMP) += nf_conntrack_timestamp.o
nf_conntrack-$(CONFIG_NF_CONNTRACK_EVENTS) += nf_conntrack_ecache.o
--
2.16.4
^ permalink raw reply related
* [PATCH v2 nf-next 1/2] ipv6: remove dependency of nf_defrag_ipv6 on ipv6 module
From: Florian Westphal @ 2018-07-13 14:27 UTC (permalink / raw)
To: netfilter-devel; +Cc: netdev, Florian Westphal
IPV6=m
DEFRAG_IPV6=m
CONNTRACK=y yields:
net/netfilter/nf_conntrack_proto.o: In function `nf_ct_netns_do_get':
net/netfilter/nf_conntrack_proto.c:802: undefined reference to `nf_defrag_ipv6_enable'
net/netfilter/nf_conntrack_proto.o:(.rodata+0x640): undefined reference to `nf_conntrack_l4proto_icmpv6'
Setting DEFRAG_IPV6=y causes undefined references to ip6_rhash_params
ip6_frag_init and ip6_expire_frag_queue so it would be needed to force
IPV6=y too.
This patch gets rid of the 'followup linker error' by removing
the dependency of ipv6.ko symbols from netfilter ipv6 defrag.
Shared code is placed into a header, then used from both.
Signed-off-by: Florian Westphal <fw@strlen.de>
---
v2: fix build failure with 6lowpan (missing include)
include/net/ipv6.h | 28 --------
include/net/ipv6_frag.h | 102 ++++++++++++++++++++++++++++++
net/ieee802154/6lowpan/reassembly.c | 2 +-
net/ipv6/netfilter/nf_conntrack_reasm.c | 17 +++--
net/ipv6/netfilter/nf_defrag_ipv6_hooks.c | 3 +-
net/ipv6/reassembly.c | 92 ++-------------------------
net/openvswitch/conntrack.c | 1 +
7 files changed, 124 insertions(+), 121 deletions(-)
create mode 100644 include/net/ipv6_frag.h
diff --git a/include/net/ipv6.h b/include/net/ipv6.h
index 16475c269749..0012c0a6c86e 100644
--- a/include/net/ipv6.h
+++ b/include/net/ipv6.h
@@ -561,34 +561,6 @@ static inline bool ipv6_prefix_equal(const struct in6_addr *addr1,
}
#endif
-struct inet_frag_queue;
-
-enum ip6_defrag_users {
- IP6_DEFRAG_LOCAL_DELIVER,
- IP6_DEFRAG_CONNTRACK_IN,
- __IP6_DEFRAG_CONNTRACK_IN = IP6_DEFRAG_CONNTRACK_IN + USHRT_MAX,
- IP6_DEFRAG_CONNTRACK_OUT,
- __IP6_DEFRAG_CONNTRACK_OUT = IP6_DEFRAG_CONNTRACK_OUT + USHRT_MAX,
- IP6_DEFRAG_CONNTRACK_BRIDGE_IN,
- __IP6_DEFRAG_CONNTRACK_BRIDGE_IN = IP6_DEFRAG_CONNTRACK_BRIDGE_IN + USHRT_MAX,
-};
-
-void ip6_frag_init(struct inet_frag_queue *q, const void *a);
-extern const struct rhashtable_params ip6_rhash_params;
-
-/*
- * Equivalent of ipv4 struct ip
- */
-struct frag_queue {
- struct inet_frag_queue q;
-
- int iif;
- __u16 nhoffset;
- u8 ecn;
-};
-
-void ip6_expire_frag_queue(struct net *net, struct frag_queue *fq);
-
static inline bool ipv6_addr_any(const struct in6_addr *a)
{
#if defined(CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS) && BITS_PER_LONG == 64
diff --git a/include/net/ipv6_frag.h b/include/net/ipv6_frag.h
new file mode 100644
index 000000000000..ff3be4ca9471
--- /dev/null
+++ b/include/net/ipv6_frag.h
@@ -0,0 +1,102 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _IPV6_FRAG_H
+#define _IPV6_FRAG_H
+#include <linux/kernel.h>
+#include <net/addrconf.h>
+#include <net/ipv6.h>
+#include <net/inet_frag.h>
+
+enum ip6_defrag_users {
+ IP6_DEFRAG_LOCAL_DELIVER,
+ IP6_DEFRAG_CONNTRACK_IN,
+ __IP6_DEFRAG_CONNTRACK_IN = IP6_DEFRAG_CONNTRACK_IN + USHRT_MAX,
+ IP6_DEFRAG_CONNTRACK_OUT,
+ __IP6_DEFRAG_CONNTRACK_OUT = IP6_DEFRAG_CONNTRACK_OUT + USHRT_MAX,
+ IP6_DEFRAG_CONNTRACK_BRIDGE_IN,
+ __IP6_DEFRAG_CONNTRACK_BRIDGE_IN = IP6_DEFRAG_CONNTRACK_BRIDGE_IN + USHRT_MAX,
+};
+
+/*
+ * Equivalent of ipv4 struct ip
+ */
+struct frag_queue {
+ struct inet_frag_queue q;
+
+ int iif;
+ __u16 nhoffset;
+ u8 ecn;
+};
+
+static inline void ip6frag_init(struct inet_frag_queue *q, const void *a)
+{
+ struct frag_queue *fq = container_of(q, struct frag_queue, q);
+ const struct frag_v6_compare_key *key = a;
+
+ q->key.v6 = *key;
+ fq->ecn = 0;
+}
+
+static inline u32 ip6frag_key_hashfn(const void *data, u32 len, u32 seed)
+{
+ return jhash2(data,
+ sizeof(struct frag_v6_compare_key) / sizeof(u32), seed);
+}
+
+static inline u32 ip6frag_obj_hashfn(const void *data, u32 len, u32 seed)
+{
+ const struct inet_frag_queue *fq = data;
+
+ return jhash2((const u32 *)&fq->key.v6,
+ sizeof(struct frag_v6_compare_key) / sizeof(u32), seed);
+}
+
+static inline int
+ip6frag_obj_cmpfn(struct rhashtable_compare_arg *arg, const void *ptr)
+{
+ const struct frag_v6_compare_key *key = arg->key;
+ const struct inet_frag_queue *fq = ptr;
+
+ return !!memcmp(&fq->key, key, sizeof(*key));
+}
+
+static inline void
+ip6frag_expire_frag_queue(struct net *net, struct frag_queue *fq)
+{
+ struct net_device *dev = NULL;
+ struct sk_buff *head;
+
+ rcu_read_lock();
+ spin_lock(&fq->q.lock);
+
+ if (fq->q.flags & INET_FRAG_COMPLETE)
+ goto out;
+
+ inet_frag_kill(&fq->q);
+
+ dev = dev_get_by_index_rcu(net, fq->iif);
+ if (!dev)
+ goto out;
+
+ __IP6_INC_STATS(net, __in6_dev_get(dev), IPSTATS_MIB_REASMFAILS);
+ __IP6_INC_STATS(net, __in6_dev_get(dev), IPSTATS_MIB_REASMTIMEOUT);
+
+ /* Don't send error if the first segment did not arrive. */
+ head = fq->q.fragments;
+ if (!(fq->q.flags & INET_FRAG_FIRST_IN) || !head)
+ goto out;
+
+ head->dev = dev;
+ skb_get(head);
+ spin_unlock(&fq->q.lock);
+
+ icmpv6_send(head, ICMPV6_TIME_EXCEED, ICMPV6_EXC_FRAGTIME, 0);
+ kfree_skb(head);
+ goto out_rcu_unlock;
+
+out:
+ spin_unlock(&fq->q.lock);
+out_rcu_unlock:
+ rcu_read_unlock();
+ inet_frag_put(&fq->q);
+}
+#endif
diff --git a/net/ieee802154/6lowpan/reassembly.c b/net/ieee802154/6lowpan/reassembly.c
index 2cc224106b69..ec7a5da56129 100644
--- a/net/ieee802154/6lowpan/reassembly.c
+++ b/net/ieee802154/6lowpan/reassembly.c
@@ -25,7 +25,7 @@
#include <net/ieee802154_netdev.h>
#include <net/6lowpan.h>
-#include <net/ipv6.h>
+#include <net/ipv6_frag.h>
#include <net/inet_frag.h>
#include "6lowpan_i.h"
diff --git a/net/ipv6/netfilter/nf_conntrack_reasm.c b/net/ipv6/netfilter/nf_conntrack_reasm.c
index 5e0332014c17..a44156b54ec9 100644
--- a/net/ipv6/netfilter/nf_conntrack_reasm.c
+++ b/net/ipv6/netfilter/nf_conntrack_reasm.c
@@ -33,9 +33,8 @@
#include <net/sock.h>
#include <net/snmp.h>
-#include <net/inet_frag.h>
+#include <net/ipv6_frag.h>
-#include <net/ipv6.h>
#include <net/protocol.h>
#include <net/transp_v6.h>
#include <net/rawv6.h>
@@ -151,7 +150,7 @@ static void nf_ct_frag6_expire(struct timer_list *t)
fq = container_of(frag, struct frag_queue, q);
net = container_of(fq->q.net, struct net, nf_frag.frags);
- ip6_expire_frag_queue(net, fq);
+ ip6frag_expire_frag_queue(net, fq);
}
/* Creation primitives. */
@@ -622,16 +621,24 @@ static struct pernet_operations nf_ct_net_ops = {
.exit = nf_ct_net_exit,
};
+static const struct rhashtable_params nfct_rhash_params = {
+ .head_offset = offsetof(struct inet_frag_queue, node),
+ .hashfn = ip6frag_key_hashfn,
+ .obj_hashfn = ip6frag_obj_hashfn,
+ .obj_cmpfn = ip6frag_obj_cmpfn,
+ .automatic_shrinking = true,
+};
+
int nf_ct_frag6_init(void)
{
int ret = 0;
- nf_frags.constructor = ip6_frag_init;
+ nf_frags.constructor = ip6frag_init;
nf_frags.destructor = NULL;
nf_frags.qsize = sizeof(struct frag_queue);
nf_frags.frag_expire = nf_ct_frag6_expire;
nf_frags.frags_cache_name = nf_frags_cache_name;
- nf_frags.rhash_params = ip6_rhash_params;
+ nf_frags.rhash_params = nfct_rhash_params;
ret = inet_frags_init(&nf_frags);
if (ret)
goto out;
diff --git a/net/ipv6/netfilter/nf_defrag_ipv6_hooks.c b/net/ipv6/netfilter/nf_defrag_ipv6_hooks.c
index e631be25337e..72dd3e202375 100644
--- a/net/ipv6/netfilter/nf_defrag_ipv6_hooks.c
+++ b/net/ipv6/netfilter/nf_defrag_ipv6_hooks.c
@@ -14,8 +14,7 @@
#include <linux/skbuff.h>
#include <linux/icmp.h>
#include <linux/sysctl.h>
-#include <net/ipv6.h>
-#include <net/inet_frag.h>
+#include <net/ipv6_frag.h>
#include <linux/netfilter_ipv6.h>
#include <linux/netfilter_bridge.h>
diff --git a/net/ipv6/reassembly.c b/net/ipv6/reassembly.c
index b939b94e7e91..6edd2ac8ae4b 100644
--- a/net/ipv6/reassembly.c
+++ b/net/ipv6/reassembly.c
@@ -57,7 +57,7 @@
#include <net/rawv6.h>
#include <net/ndisc.h>
#include <net/addrconf.h>
-#include <net/inet_frag.h>
+#include <net/ipv6_frag.h>
#include <net/inet_ecn.h>
static const char ip6_frag_cache_name[] = "ip6-frags";
@@ -72,61 +72,6 @@ static struct inet_frags ip6_frags;
static int ip6_frag_reasm(struct frag_queue *fq, struct sk_buff *prev,
struct net_device *dev);
-void ip6_frag_init(struct inet_frag_queue *q, const void *a)
-{
- struct frag_queue *fq = container_of(q, struct frag_queue, q);
- const struct frag_v6_compare_key *key = a;
-
- q->key.v6 = *key;
- fq->ecn = 0;
-}
-EXPORT_SYMBOL(ip6_frag_init);
-
-void ip6_expire_frag_queue(struct net *net, struct frag_queue *fq)
-{
- struct net_device *dev = NULL;
- struct sk_buff *head;
-
- rcu_read_lock();
- spin_lock(&fq->q.lock);
-
- if (fq->q.flags & INET_FRAG_COMPLETE)
- goto out;
-
- inet_frag_kill(&fq->q);
-
- dev = dev_get_by_index_rcu(net, fq->iif);
- if (!dev)
- goto out;
-
- __IP6_INC_STATS(net, __in6_dev_get(dev), IPSTATS_MIB_REASMFAILS);
- __IP6_INC_STATS(net, __in6_dev_get(dev), IPSTATS_MIB_REASMTIMEOUT);
-
- /* Don't send error if the first segment did not arrive. */
- head = fq->q.fragments;
- if (!(fq->q.flags & INET_FRAG_FIRST_IN) || !head)
- goto out;
-
- /* But use as source device on which LAST ARRIVED
- * segment was received. And do not use fq->dev
- * pointer directly, device might already disappeared.
- */
- head->dev = dev;
- skb_get(head);
- spin_unlock(&fq->q.lock);
-
- icmpv6_send(head, ICMPV6_TIME_EXCEED, ICMPV6_EXC_FRAGTIME, 0);
- kfree_skb(head);
- goto out_rcu_unlock;
-
-out:
- spin_unlock(&fq->q.lock);
-out_rcu_unlock:
- rcu_read_unlock();
- inet_frag_put(&fq->q);
-}
-EXPORT_SYMBOL(ip6_expire_frag_queue);
-
static void ip6_frag_expire(struct timer_list *t)
{
struct inet_frag_queue *frag = from_timer(frag, t, timer);
@@ -136,7 +81,7 @@ static void ip6_frag_expire(struct timer_list *t)
fq = container_of(frag, struct frag_queue, q);
net = container_of(fq->q.net, struct net, ipv6.frags);
- ip6_expire_frag_queue(net, fq);
+ ip6frag_expire_frag_queue(net, fq);
}
static struct frag_queue *
@@ -696,42 +641,19 @@ static struct pernet_operations ip6_frags_ops = {
.exit = ipv6_frags_exit_net,
};
-static u32 ip6_key_hashfn(const void *data, u32 len, u32 seed)
-{
- return jhash2(data,
- sizeof(struct frag_v6_compare_key) / sizeof(u32), seed);
-}
-
-static u32 ip6_obj_hashfn(const void *data, u32 len, u32 seed)
-{
- const struct inet_frag_queue *fq = data;
-
- return jhash2((const u32 *)&fq->key.v6,
- sizeof(struct frag_v6_compare_key) / sizeof(u32), seed);
-}
-
-static int ip6_obj_cmpfn(struct rhashtable_compare_arg *arg, const void *ptr)
-{
- const struct frag_v6_compare_key *key = arg->key;
- const struct inet_frag_queue *fq = ptr;
-
- return !!memcmp(&fq->key, key, sizeof(*key));
-}
-
-const struct rhashtable_params ip6_rhash_params = {
+static const struct rhashtable_params ip6_rhash_params = {
.head_offset = offsetof(struct inet_frag_queue, node),
- .hashfn = ip6_key_hashfn,
- .obj_hashfn = ip6_obj_hashfn,
- .obj_cmpfn = ip6_obj_cmpfn,
+ .hashfn = ip6frag_key_hashfn,
+ .obj_hashfn = ip6frag_obj_hashfn,
+ .obj_cmpfn = ip6frag_obj_cmpfn,
.automatic_shrinking = true,
};
-EXPORT_SYMBOL(ip6_rhash_params);
int __init ipv6_frag_init(void)
{
int ret;
- ip6_frags.constructor = ip6_frag_init;
+ ip6_frags.constructor = ip6frag_init;
ip6_frags.destructor = NULL;
ip6_frags.qsize = sizeof(struct frag_queue);
ip6_frags.frag_expire = ip6_frag_expire;
diff --git a/net/openvswitch/conntrack.c b/net/openvswitch/conntrack.c
index e05bd3e53f0f..3ede65344e4d 100644
--- a/net/openvswitch/conntrack.c
+++ b/net/openvswitch/conntrack.c
@@ -26,6 +26,7 @@
#include <net/netfilter/nf_conntrack_seqadj.h>
#include <net/netfilter/nf_conntrack_zones.h>
#include <net/netfilter/ipv6/nf_defrag_ipv6.h>
+#include <net/ipv6_frag.h>
#ifdef CONFIG_NF_NAT_NEEDED
#include <linux/netfilter/nf_nat.h>
--
2.16.4
^ permalink raw reply related
* [bpf-next PATCH] samples/bpf: xdp_redirect_cpu handle parsing of double VLAN tagged packets
From: Jesper Dangaard Brouer @ 2018-07-13 14:35 UTC (permalink / raw)
To: netdev
Cc: florian.maury-cv, Daniel Borkmann, marek, Alexei Starovoitov,
Jesper Dangaard Brouer
People noticed that the code match on IEEE 802.1ad (ETH_P_8021AD) ethertype,
and this implies Q-in-Q or double tagged VLANs. Thus, we better parse
the next VLAN header too. It is even marked as a TODO.
This is relevant for real world use-cases, as XDP cpumap redirect can be
used when the NIC RSS hashing is broken. E.g. the ixgbe driver HW cannot
handle double tagged VLAN packets, and places everything into a single
RX queue. Using cpumap redirect, users can redistribute traffic across
CPUs to solve this, which is faster than the network stacks RPS solution.
It is left as an exerise how to distribute the packets across CPUs. It
would be convenient to use the RX hash, but that is not _yet_ exposed
to XDP programs. For now, users can code their own hash, as I've demonstrated
in the Suricata code (where Q-in-Q is handled correctly).
Reported-by: Florian Maury <florian.maury-cv@x-cli.eu>
Reported-by: Marek Majkowski <marek@cloudflare.com>
Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>
---
Suricata code pointer:
https://github.com/OISF/suricata/blob/master/ebpf/xdp_filter.c#L234-L245
I'm controlled via Twitter:
https://twitter.com/majek04/status/1017492418227331073
samples/bpf/xdp_redirect_cpu_kern.c | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/samples/bpf/xdp_redirect_cpu_kern.c b/samples/bpf/xdp_redirect_cpu_kern.c
index 303e9e7161f3..8cb703671b04 100644
--- a/samples/bpf/xdp_redirect_cpu_kern.c
+++ b/samples/bpf/xdp_redirect_cpu_kern.c
@@ -134,7 +134,16 @@ bool parse_eth(struct ethhdr *eth, void *data_end,
return false;
eth_type = vlan_hdr->h_vlan_encapsulated_proto;
}
- /* TODO: Handle double VLAN tagged packet */
+ /* Handle double VLAN tagged packet */
+ if (eth_type == htons(ETH_P_8021Q) || eth_type == htons(ETH_P_8021AD)) {
+ struct vlan_hdr *vlan_hdr;
+
+ vlan_hdr = (void *)eth + offset;
+ offset += sizeof(*vlan_hdr);
+ if ((void *)eth + offset > data_end)
+ return false;
+ eth_type = vlan_hdr->h_vlan_encapsulated_proto;
+ }
*eth_proto = ntohs(eth_type);
*l3_offset = offset;
^ permalink raw reply related
* Re: [PATCH net-next 2/4] tc/act: remove unneeded RCU lock in action callback
From: Paolo Abeni @ 2018-07-13 14:26 UTC (permalink / raw)
To: Daniel Borkmann, netdev
Cc: Jamal Hadi Salim, Cong Wang, Jiri Pirko, Alexei Starovoitov,
Marcelo Ricardo Leitner
In-Reply-To: <6a61aa2e-2653-eb9b-99c1-16566f353e77@iogearbox.net>
On Fri, 2018-07-13 at 16:08 +0200, Daniel Borkmann wrote:
> Hi Paolo,
>
> On 07/13/2018 11:55 AM, Paolo Abeni wrote:
> > Each lockless action currently does its own RCU locking in ->act().
> > This is allows using plain RCU accessor, even if the context
> > is really RCU BH.
> >
> > This change drops the per action RCU lock, replace the accessors
> > with _bh variant, cleans up a bit the surronding code and documents
> > the RCU status in the relevant header.
> > No functional nor performance change is intended.
> >
> > The goal of this patch is clarifying that the RCU critical section
> > used by the tc actions extends up to the classifier's caller.
> >
> > Signed-off-by: Paolo Abeni <pabeni@redhat.com>
>
> [...]
> > diff --git a/net/sched/act_bpf.c b/net/sched/act_bpf.c
> > index 06f743d8ed41..ac20266460c0 100644
> > --- a/net/sched/act_bpf.c
> > +++ b/net/sched/act_bpf.c
> > @@ -45,8 +45,7 @@ static int tcf_bpf(struct sk_buff *skb, const struct tc_action *act,
> > tcf_lastuse_update(&prog->tcf_tm);
> > bstats_cpu_update(this_cpu_ptr(prog->common.cpu_bstats), skb);
> >
> > - rcu_read_lock();
> > - filter = rcu_dereference(prog->filter);
> > + filter = rcu_dereference_bh(prog->filter);
> > if (at_ingress) {
> > __skb_push(skb, skb->mac_len);
> > bpf_compute_data_pointers(skb);
> > @@ -56,7 +55,6 @@ static int tcf_bpf(struct sk_buff *skb, const struct tc_action *act,
> > bpf_compute_data_pointers(skb);
> > filter_res = BPF_PROG_RUN(filter, skb);
> > }
> > - rcu_read_unlock();
>
> This conversion is not correct, BPF itself relies on RCU but not RCU-bh flavor.
> You might probably see a splat if you do e.g. a map lookup with this change in
> interpreter mode on tx side.
Thank you for your review.
I actually tested with lockdep, and lockdep is happy about it.
The not so nice fact is that many TC modules already use plain RCU
primitives in the control path (call_rcu, kfree_rcu, etc.) and
rcu_derefence_bh() in the datapath (e.g. all the classifiers). AFACS,
despite the mix, this use is safe.
Cheers,
Paolo
^ permalink raw reply
* Re: [net-next PATCH] net: ipv4: fix listify ip_rcv_finish in case of forwarding
From: Edward Cree @ 2018-07-13 14:19 UTC (permalink / raw)
To: Or Gerlitz, Jesper Dangaard Brouer; +Cc: Saeed Mahameed, netdev@vger.kernel.org
In-Reply-To: <CAJ3xEMiUbjSM4-EZkm-_b+EtAS4NZOA+Whcv3V3=UT+CwmOvQA@mail.gmail.com>
On 12/07/18 21:10, Or Gerlitz wrote:
> On Wed, Jul 11, 2018 at 11:06 PM, Jesper Dangaard Brouer
> <brouer@redhat.com> wrote:
>> One reason I didn't "just" send a patch, is that Edward so-fare only
>> implemented netif_receive_skb_list() and not napi_gro_receive_list().
> sfc does't support gro?! doesn't make sense.. Edward?
sfc has a flag EFX_RX_PKT_TCP set according to bits in the RX event, we
call napi_{get,gro}_frags() (via efx_rx_packet_gro()) for TCP packets and
netif_receive_skb() (or now the list handling) (via efx_rx_deliver()) for
non-TCP packets. So we avoid the GRO overhead for non-TCP workloads.
> Same TCP performance
>
> with GRO and no rx-batching
>
> or
>
> without GRO and yes rx-batching
>
> is by far not intuitive result
I'm also surprised by this. If I can find the time I'll try to do similar
experiments on sfc.
Jesper, are the CPU utilisations similar in both cases? You're sure your
stream isn't TX-limited?
-Ed
^ permalink raw reply
* Re: [PATCH net-next 3/4] net/sched: refactor TC_ACT_REDIRECT handling
From: Daniel Borkmann @ 2018-07-13 14:13 UTC (permalink / raw)
To: Paolo Abeni, netdev
Cc: Jamal Hadi Salim, Cong Wang, Jiri Pirko, Alexei Starovoitov,
Marcelo Ricardo Leitner
In-Reply-To: <20ab3afa3843dc325570cdc502e296f1d4c0da91.1531473946.git.pabeni@redhat.com>
On 07/13/2018 11:55 AM, Paolo Abeni wrote:
> This patch changes the TC_ACT_REDIRECT code path to allow
> providing the redirect parameters via the tcf_result argument.
>
> Such union is expanded to host the redirect device, the redirect
> direction (ingress/egress) and the stats to be updated on error
> conditions.
>
> Actions/classifiers using TC_ACT_REDIRECT can either:
> * fill the tcf_result redirect related fields
> * clear such fields and use the bpf per cpu redirect info
>
> skb_do_redirect now tries to fetch the relevant data from tcf_result
> and fall back to access redirect info. It also updates the stats
> accordingly to the redirect result, if provided by the caller.
>
> This will allow using the TC_ACT_REDIRECT action in more places in
> the next patch.
>
> Signed-off-by: Paolo Abeni <pabeni@redhat.com>
> ---
> include/net/sch_generic.h | 15 ++++++++++++++-
> net/core/dev.c | 4 ++--
> net/core/filter.c | 29 +++++++++++++++++++++++------
> net/core/lwt_bpf.c | 5 ++++-
> net/sched/act_bpf.c | 4 +++-
> net/sched/cls_bpf.c | 8 +++++---
> 6 files changed, 51 insertions(+), 14 deletions(-)
>
> diff --git a/include/net/sch_generic.h b/include/net/sch_generic.h
> index 056dc1083aa3..dd9e00d017b3 100644
> --- a/include/net/sch_generic.h
> +++ b/include/net/sch_generic.h
> @@ -235,9 +235,22 @@ struct tcf_result {
> u32 classid;
> };
> const struct tcf_proto *goto_tp;
> +
> + /* used by the TC_ACT_REDIRECT action */
> + struct {
> + /* device and direction, or 0 bpf redirect */
> + long dev_ingress;
> + struct gnet_stats_queue *qstats;
> + };
> };
> };
>
> +#define TCF_RESULT_REDIR_DEV(res) \
> + ((struct net_device *)((res)->dev_ingress & ~1))
> +#define TCF_RESULT_REDIR_INGRESS(res) ((res)->dev_ingress & 1)
> +#define TCF_RESULT_SET_REDIRECT(res, dev, ingress) \
> + ((res)->dev_ingress = (long)(dev) | (!!(ingress)))
> +
> struct tcf_proto_ops {
> struct list_head head;
> char kind[IFNAMSIZ];
> @@ -543,7 +556,7 @@ struct Qdisc *qdisc_create_dflt(struct netdev_queue *dev_queue,
> struct netlink_ext_ack *extack);
> void __qdisc_calculate_pkt_len(struct sk_buff *skb,
> const struct qdisc_size_table *stab);
> -int skb_do_redirect(struct sk_buff *);
> +int skb_do_redirect(struct sk_buff *skb, struct tcf_result *res);
>
> static inline void skb_reset_tc(struct sk_buff *skb)
> {
> diff --git a/net/core/dev.c b/net/core/dev.c
> index 14a748ee8cc9..a283dbfde30c 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -3538,7 +3538,7 @@ sch_handle_egress(struct sk_buff *skb, int *ret, struct net_device *dev)
> return NULL;
> case TC_ACT_REDIRECT:
> /* No need to push/pop skb's mac_header here on egress! */
> - skb_do_redirect(skb);
> + skb_do_redirect(skb, &cl_res);
> *ret = NET_XMIT_SUCCESS;
> return NULL;
> default:
> @@ -4600,7 +4600,7 @@ sch_handle_ingress(struct sk_buff *skb, struct packet_type **pt_prev, int *ret,
> * redirecting to another netdev
> */
> __skb_push(skb, skb->mac_len);
> - skb_do_redirect(skb);
> + skb_do_redirect(skb, &cl_res);
> return NULL;
> default:
> break;
> diff --git a/net/core/filter.c b/net/core/filter.c
> index b9ec916f4e3a..4f64cf5189e6 100644
> --- a/net/core/filter.c
> +++ b/net/core/filter.c
> @@ -2062,19 +2062,36 @@ BPF_CALL_2(bpf_redirect, u32, ifindex, u64, flags)
> return TC_ACT_REDIRECT;
> }
>
> -int skb_do_redirect(struct sk_buff *skb)
> +int skb_do_redirect(struct sk_buff *skb, struct tcf_result *res)
> {
> - struct redirect_info *ri = this_cpu_ptr(&redirect_info);
> + struct gnet_stats_queue *stats;
> struct net_device *dev;
> + int ret, flags;
>
> - dev = dev_get_by_index_rcu(dev_net(skb->dev), ri->ifindex);
> - ri->ifindex = 0;
> + if (!res->dev_ingress) {
> + struct redirect_info *ri = this_cpu_ptr(&redirect_info);
> +
> + dev = dev_get_by_index_rcu(dev_net(skb->dev), ri->ifindex);
> + flags = ri->flags;
> + ri->ifindex = 0;
> + stats = NULL;
> + } else {
> + dev = TCF_RESULT_REDIR_DEV(res);
> + flags = TCF_RESULT_REDIR_INGRESS(res) ? BPF_F_INGRESS : 0;
> + stats = res->qstats;
> + }
> if (unlikely(!dev)) {
> kfree_skb(skb);
> - return -EINVAL;
> + ret = -EINVAL;
> + goto out;
> }
>
> - return __bpf_redirect(skb, dev, ri->flags);
> + ret = __bpf_redirect(skb, dev, flags);
> +
> +out:
> + if (ret && stats)
> + qstats_overlimit_inc(res->qstats);
> + return ret;
> }
>
> static const struct bpf_func_proto bpf_redirect_proto = {
> diff --git a/net/core/lwt_bpf.c b/net/core/lwt_bpf.c
> index e7e626fb87bb..8dde1093994a 100644
> --- a/net/core/lwt_bpf.c
> +++ b/net/core/lwt_bpf.c
> @@ -65,7 +65,10 @@ static int run_lwt_bpf(struct sk_buff *skb, struct bpf_lwt_prog *lwt,
> lwt->name ? : "<unknown>");
> ret = BPF_OK;
> } else {
> - ret = skb_do_redirect(skb);
> + struct tcf_result res;
> +
> + res.dev_ingress = 0;
> + ret = skb_do_redirect(skb, &res);
> if (ret == 0)
> ret = BPF_REDIRECT;
> }
> diff --git a/net/sched/act_bpf.c b/net/sched/act_bpf.c
> index ac20266460c0..6fd46b691181 100644
> --- a/net/sched/act_bpf.c
> +++ b/net/sched/act_bpf.c
> @@ -67,10 +67,12 @@ static int tcf_bpf(struct sk_buff *skb, const struct tc_action *act,
> * returned.
> */
> switch (filter_res) {
> + case TC_ACT_REDIRECT:
> + res->dev_ingress = 0;
> + /* fall-through */
> case TC_ACT_PIPE:
> case TC_ACT_RECLASSIFY:
> case TC_ACT_OK:
> - case TC_ACT_REDIRECT:
> action = filter_res;
> break;
> case TC_ACT_SHOT:
> diff --git a/net/sched/cls_bpf.c b/net/sched/cls_bpf.c
> index 66e0ac9811f9..f0fb7ded8fe2 100644
> --- a/net/sched/cls_bpf.c
> +++ b/net/sched/cls_bpf.c
> @@ -65,14 +65,16 @@ static const struct nla_policy bpf_policy[TCA_BPF_MAX + 1] = {
> .len = sizeof(struct sock_filter) * BPF_MAXINSNS },
> };
>
> -static int cls_bpf_exec_opcode(int code)
> +static int cls_bpf_exec_opcode(int code, struct tcf_result *res)
> {
> switch (code) {
> + case TC_ACT_REDIRECT:
> + res->dev_ingress = 0;
> + /* fall-through */
> case TC_ACT_OK:
> case TC_ACT_SHOT:
> case TC_ACT_STOLEN:
> case TC_ACT_TRAP:
> - case TC_ACT_REDIRECT:
> case TC_ACT_UNSPEC:
> return code;
> default:
> @@ -113,7 +115,7 @@ static int cls_bpf_classify(struct sk_buff *skb, const struct tcf_proto *tp,
> res->classid = TC_H_MAJ(prog->res.classid) |
> qdisc_skb_cb(skb)->tc_classid;
>
> - ret = cls_bpf_exec_opcode(filter_res);
> + ret = cls_bpf_exec_opcode(filter_res, res);
> if (ret == TC_ACT_UNSPEC)
> continue;
> break;
>
Can't we just export the struct redirect_info and let others like
act_mirred use it, then we wouldn't need all these extra changes in
fast path?
Thanks,
Daniel
^ permalink raw reply
* Re: [PATCH net-next 2/4] tc/act: remove unneeded RCU lock in action callback
From: Daniel Borkmann @ 2018-07-13 14:08 UTC (permalink / raw)
To: Paolo Abeni, netdev
Cc: Jamal Hadi Salim, Cong Wang, Jiri Pirko, Alexei Starovoitov,
Marcelo Ricardo Leitner
In-Reply-To: <117cef422749024d62465c85c5d6e66c5b055360.1531473946.git.pabeni@redhat.com>
Hi Paolo,
On 07/13/2018 11:55 AM, Paolo Abeni wrote:
> Each lockless action currently does its own RCU locking in ->act().
> This is allows using plain RCU accessor, even if the context
> is really RCU BH.
>
> This change drops the per action RCU lock, replace the accessors
> with _bh variant, cleans up a bit the surronding code and documents
> the RCU status in the relevant header.
> No functional nor performance change is intended.
>
> The goal of this patch is clarifying that the RCU critical section
> used by the tc actions extends up to the classifier's caller.
>
> Signed-off-by: Paolo Abeni <pabeni@redhat.com>
[...]
> diff --git a/net/sched/act_bpf.c b/net/sched/act_bpf.c
> index 06f743d8ed41..ac20266460c0 100644
> --- a/net/sched/act_bpf.c
> +++ b/net/sched/act_bpf.c
> @@ -45,8 +45,7 @@ static int tcf_bpf(struct sk_buff *skb, const struct tc_action *act,
> tcf_lastuse_update(&prog->tcf_tm);
> bstats_cpu_update(this_cpu_ptr(prog->common.cpu_bstats), skb);
>
> - rcu_read_lock();
> - filter = rcu_dereference(prog->filter);
> + filter = rcu_dereference_bh(prog->filter);
> if (at_ingress) {
> __skb_push(skb, skb->mac_len);
> bpf_compute_data_pointers(skb);
> @@ -56,7 +55,6 @@ static int tcf_bpf(struct sk_buff *skb, const struct tc_action *act,
> bpf_compute_data_pointers(skb);
> filter_res = BPF_PROG_RUN(filter, skb);
> }
> - rcu_read_unlock();
This conversion is not correct, BPF itself relies on RCU but not RCU-bh flavor.
You might probably see a splat if you do e.g. a map lookup with this change in
interpreter mode on tx side.
> /* A BPF program may overwrite the default action opcode.
> * Similarly as in cls_bpf, if filter_res == -1 we use the
Thanks,
Daniel
^ permalink raw reply
* Re: [PATCH net-next v6 00/11] Modify action API for implementing lockless actions
From: Vlad Buslov @ 2018-07-13 13:40 UTC (permalink / raw)
To: Cong Wang
Cc: David Miller, Linux Kernel Network Developers, Jamal Hadi Salim,
Jiri Pirko, Alexei Starovoitov, Daniel Borkmann,
Yevgeny Kliteynik
In-Reply-To: <CAM_iQpUtybeq1T5hKgTRhZwaRJf1g-YBfR_+G3jfWoifi2+8CQ@mail.gmail.com>
On Fri 13 Jul 2018 at 03:54, Cong Wang <xiyou.wangcong@gmail.com> wrote:
> On Sat, Jul 7, 2018 at 8:43 PM David Miller <davem@davemloft.net> wrote:
>>
>> From: Vlad Buslov <vladbu@mellanox.com>
>> Date: Thu, 5 Jul 2018 17:24:22 +0300
>>
>> > Currently, all netlink protocol handlers for updating rules, actions and
>> > qdiscs are protected with single global rtnl lock which removes any
>> > possibility for parallelism. This patch set is a first step to remove
>> > rtnl lock dependency from TC rules update path.
>> ...
>>
>> I'll apply this for now, I reviewed it a few more times and I see
>> where you are going with this.
>
> Dear David,
>
> I don't understand why you even believe the claim of lockless
> updaters here, it at least should raise a red flag when you see any
> kinda of this claim.
>
> I know you don't trust me, how about thinking it in this way:
>
> Why does RCU still require a lock for RCU writers? (Or at least
> RCU recommends a lock, if anyone really wants to point out some
> lockless algorithm here.)
>
> or:
>
> If writers could really go lockless as easily as Vlad claims, how could
> even Paul E. McKenney never bring it into RCU?
>
> Maybe Vlad is much cleverer than any of us here, and maybe he really
> discovers a very brilliant algorithm to allow TC actions to be updated
> locklessly, why not wait until he shows a proof (either code or a paper)?
> Is there a rush? I don't see it.
>
> In fact, I discussed this with Vlad a little bit at netdev TC workshop.
> I never see any brilliant algorithm from him from his slides, and I was
> told by him he used "copy and replace" to archive parallel updaters, I
> told him that is basically how RCU works and RCU writers have to be
> sync'ed with a lock (or at least recommended).
>
> Also, to confirm my judgement, I checked this with Paul privately too.
> Paul said you have to be extremely careful to go lockless, it is very hard
> to be bug free for lockless, although he _never_ says it is impossible.
>
> My _personal_ bet is that, lockless updates for TC filters or actions
> are impossible unless there are more things hiding behind "copy and
> replace", for example, some brilliant lockless algorithm. If lockless is
> really impossible in this circumstance, then many of your efforts in
> this patchset are vain, by the way.
>
> I _do_ believe you can break RTNL down to per device, per filter or per
> action, but no matter how small the locking scope is, there is still a lock.
> With a lock, there is no need to make things friendly to lockless, like
> making an integer increment inside an action to be atomic (your patch
> 02/11).
>
> Please _do_ prove my personal judgement is wrong, by showing your
> final code or a formal paper/article. I am very *happy* to be proved
> to be wrong here, I am very open to change my mind here.
>
> Vlad, we need your proof. Please prove I am wrong, seriously!!! :)
>
> Thanks to anyone for proving me I am wrong just in case!!! :)
Dear Cong,
I never claimed to have some new brilliant algorithm that completely
removed any locks from rules update path. Obviously, fine-grained
locking is introduced when necessary. I'm sorry if my liberal usage of
term "lockless" confused you. I guess I should be more specific. I'm
fully agree with you that totally removing any and all locks from rules
update path would require some engineering marvel.
^ permalink raw reply
* Re: [PATCH net-next] TCP: make seq # error messages more readable
From: Eric Dumazet @ 2018-07-13 13:38 UTC (permalink / raw)
To: Randy Dunlap, netdev@vger.kernel.org, David Miller, Eric Dumazet
Cc: 積丹尼 Dan Jacobson
In-Reply-To: <48586224-eb36-2489-735c-4946cb0b1c2b@infradead.org>
On 07/12/2018 05:48 PM, Randy Dunlap wrote:
> From: Randy Dunlap <rdunlap@infradead.org>
>
> Attempt to make cryptic TCP seq number error messages clearer by
> (1) adding the function name, (2) identifying the errors as "seq # bug",
> and (3) grouping the field identifiers and values by separating them
> with commas.
>
> E.g., the following message is changed from:
>
> recvmsg bug 2: copied 73BCB6CD seq 70F17CBE rcvnxt 73BCB9AA fl 0
> WARNING: CPU: 2 PID: 1501 at /linux/net/ipv4/tcp.c:1881 tcp_recvmsg+0x649/0xb90
>
> to:
>
> tcp_recvmsg: TCP recvmsg seq # bug 2: copied 73BCB6CD, seq 70F17CBE, rcvnxt 73BCB9AA, fl 0
> WARNING: CPU: 2 PID: 1501 at /linux/net/ipv4/tcp.c:2011 tcp_recvmsg+0x694/0xba0
>
Hi Randy
It is not clear what this patch improves.
Do we really to mention tcp twice ?
Thanks.
^ permalink raw reply
* Re: [PATCH bpf 0/4] Consistent sendmsg error reporting in AF_XDP
From: Daniel Borkmann @ 2018-07-13 13:38 UTC (permalink / raw)
To: Magnus Karlsson, bjorn.topel, ast, netdev, eric.dumazet; +Cc: qi.z.zhang, pavel
In-Reply-To: <1531296772-28850-1-git-send-email-magnus.karlsson@intel.com>
On 07/11/2018 10:12 AM, Magnus Karlsson wrote:
> This patch set adjusts the AF_XDP TX error reporting so that it becomes
> consistent between copy mode and zero-copy. First some background:
>
> Copy-mode for TX uses the SKB path in which the action of sending the
> packet is performed from process context using the sendmsg
> syscall. Completions are usually done asynchronously from NAPI mode by
> using a TX interrupt. In this mode, send errors can be returned back
> through the syscall.
>
> In zero-copy mode both the sending of the packet and the completions
> are done asynchronously from NAPI mode for performance reasons. In
> this mode, the sendmsg syscall only makes sure that the TX NAPI loop
> will be run that performs both the actions of sending and
> completing. In this mode it is therefore not possible to return errors
> through the sendmsg syscall as the sending is done from the NAPI
> loop. Note that it is possible to implement a synchronous send with
> our API, but in our benchmarks that made the TX performance drop by
> nearly half due to synchronization requirements and cache line
> bouncing. But for some netdevs this might be preferable so let us
> leave it up to the implementation to decide.
>
> The problem is that the current code base returns some errors in
> copy-mode that are not possible to return in zero-copy mode. This
> patch set aligns them so that the two modes always return the same
> error code. We achieve this by removing some of the errors returned by
> sendmsg in copy-mode (and in one case adding an error message for
> zero-copy mode) and offering alternative error detection methods that
> are consistent between the two modes.
Looks good, applied to bpf, thanks Magnus!
^ permalink raw reply
* Re: [PATCH net-next 0/4] Further ARM BPF jit compiler improvements
From: Daniel Borkmann @ 2018-07-13 13:30 UTC (permalink / raw)
To: Russell King - ARM Linux, netdev, linux-arm-kernel
In-Reply-To: <20180712205003.GU17271@n2100.armlinux.org.uk>
On 07/12/2018 10:50 PM, Russell King - ARM Linux wrote:
> Four further jit compiler improves for 32-bit ARM.
>
> arch/arm/net/bpf_jit_32.c | 120 ++++++++++++++++++++++++++++------------------
> 1 file changed, 73 insertions(+), 47 deletions(-)
Applied to bpf-next, thanks Russell! (Fyi, pull-req with the two sets included will
go out to David Miller for net-next later today.)
^ permalink raw reply
* Re: [PATCH net-next v6 01/11] net: sched: use rcu for action cookie update
From: Vlad Buslov @ 2018-07-13 13:30 UTC (permalink / raw)
To: Cong Wang
Cc: Linux Kernel Network Developers, David Miller, Jamal Hadi Salim,
Jiri Pirko, Alexei Starovoitov, Daniel Borkmann,
Yevgeny Kliteynik, Jiri Pirko
In-Reply-To: <CAM_iQpVF-nDzuvewu9G_-EB4=jmAaND+eUr5D=9oF=65muAVRw@mail.gmail.com>
On Fri 13 Jul 2018 at 03:52, Cong Wang <xiyou.wangcong@gmail.com> wrote:
> On Thu, Jul 5, 2018 at 7:24 AM Vlad Buslov <vladbu@mellanox.com> wrote:
>>
>> Implement functions to atomically update and free action cookie
>> using rcu mechanism.
>
> Without stating any reason..... Is this even a changelog?
Yes, it is.
>
>>
>> Reviewed-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
>
> Dear Marcelo, how did it pass your review? See below:
>
>
>> +static void tcf_set_action_cookie(struct tc_cookie __rcu **old_cookie,
>> + struct tc_cookie *new_cookie)
>> +{
>> + struct tc_cookie *old;
>> +
>> + old = xchg(old_cookie, new_cookie);
>
>
> This is an incorrect use of RCU, obviously should be rcu_assign_pointer()
> here.
Could you please explain your concern in more details? Similar pattern
is already widely used in kernel for re-assigning rcu pointers. For
example, Eric Dumazet uses it in 1c0d32fde5bd ("net_sched:
gen_estimator: complete rewrite of rate estimators"):
void gen_kill_estimator(struct net_rate_estimator __rcu **rate_est)
{
struct net_rate_estimator *est;
est = xchg((__force struct net_rate_estimator **)rate_est, NULL);
if (est) {
del_timer_sync(&est->timer);
kfree_rcu(est, rcu);
}
}
Tom Herbert uses same idiom in a8c5f90fb59a ("ip_tunnel: Ops
registration for secondary encap (fou, gue)"):
int ip_tunnel_encap_add_ops(const struct ip_tunnel_encap_ops *ops,
unsigned int num)
{
if (num >= MAX_IPTUN_ENCAP_OPS)
return -ERANGE;
return !cmpxchg((const struct ip_tunnel_encap_ops **)
&iptun_encaps[num],
NULL, ops) ? 0 : -1;
}
Again, Eric uses xchg to re-assign rcu pointer in 45f6fad84cc3 ("ipv6:
add complete rcu protection around np->opt"):
struct ipv6_txoptions *ipv6_update_options(struct sock *sk,
struct ipv6_txoptions *opt)
{
if (inet_sk(sk)->is_icsk) {
if (opt &&
!((1 << sk->sk_state) & (TCPF_LISTEN | TCPF_CLOSE)) &&
inet_sk(sk)->inet_daddr != LOOPBACK4_IPV6) {
struct inet_connection_sock *icsk = inet_csk(sk);
icsk->icsk_ext_hdr_len = opt->opt_flen + opt->opt_nflen;
icsk->icsk_sync_mss(sk, icsk->icsk_pmtu_cookie);
}
}
opt = xchg((__force struct ipv6_txoptions **)&inet6_sk(sk)->opt,
opt);
sk_dst_reset(sk);
return opt;
}
>
>
>> @@ -65,10 +83,7 @@ static void free_tcf(struct tc_action *p)
>> free_percpu(p->cpu_bstats);
>> free_percpu(p->cpu_qstats);
>>
>> - if (p->act_cookie) {
>> - kfree(p->act_cookie->data);
>> - kfree(p->act_cookie);
>> - }
>> + tcf_set_action_cookie(&p->act_cookie, NULL);
>
> So, this is called in free_tcf(), where the action is already
> invisible from readers so it is ready to be freed.
>
> The question is:
>
> If the action itself is already ready to be freed, why do you
> need RCU here? What could still read 'act->act_cookie'
> while 'act' is already invisible?
>
> Its last refcnt is already gone, the fast path RCU readers
> are gone too given filters use rcu work already.
>
> Standalone action dump? Again, the last refcnt is already
> gone.
It is not necessary here, I just used tcf_set_action_cookie() that
already implements cookie pointer cleanup to prevent code duplication.
I'm open to changing it, if you concerned with performance impact of
using atomic operation for re-assigning cookie pointer.
>
> Marcelo, Vlad, Jiri, please explain.
>
> Thanks!
Thank you for reviewing my code!
^ permalink raw reply
* Re: [PATCH nf-next 1/2] ipv6: remove dependency of nf_defrag_ipv6 on ipv6 module
From: kbuild test robot @ 2018-07-13 13:26 UTC (permalink / raw)
To: Florian Westphal
Cc: kbuild-all, netfilter-devel, netdev, arnd, Florian Westphal
In-Reply-To: <20180712143547.2194-1-fw@strlen.de>
Hi Florian,
I love your patch! Perhaps something to improve:
[auto build test WARNING on nf-next/master]
url: https://github.com/0day-ci/linux/commits/Florian-Westphal/ipv6-remove-dependency-of-nf_defrag_ipv6-on-ipv6-module/20180713-064909
base: https://git.kernel.org/pub/scm/linux/kernel/git/pablo/nf-next.git master
reproduce:
# apt-get install sparse
make ARCH=x86_64 allmodconfig
make C=1 CF=-D__CHECK_ENDIAN__
sparse warnings: (new ones prefixed by >>)
>> net/ieee802154/6lowpan/reassembly.c:57:14: sparse: unknown member
>> net/ieee802154/6lowpan/reassembly.c:57:14: sparse: cast from unknown type
net/ieee802154/6lowpan/reassembly.c:58:15: sparse: using member 'q' in incomplete struct frag_queue
net/ieee802154/6lowpan/reassembly.c:58:15: sparse: cast from unknown type
net/ieee802154/6lowpan/reassembly.c:60:22: sparse: using member 'q' in incomplete struct frag_queue
net/ieee802154/6lowpan/reassembly.c:62:15: sparse: using member 'q' in incomplete struct frag_queue
net/ieee802154/6lowpan/reassembly.c:65:27: sparse: using member 'q' in incomplete struct frag_queue
net/ieee802154/6lowpan/reassembly.c:67:24: sparse: using member 'q' in incomplete struct frag_queue
net/ieee802154/6lowpan/reassembly.c:68:26: sparse: using member 'q' in incomplete struct frag_queue
>> net/ieee802154/6lowpan/reassembly.c:57:14: sparse: unknown expression (30 46)
net/ieee802154/6lowpan/reassembly.c:58:15: sparse: unknown expression (8 46)
>> net/ieee802154/6lowpan/reassembly.c:60:18: sparse: call with no type!
net/ieee802154/6lowpan/reassembly.c:62:18: sparse: unknown expression (8 46)
net/ieee802154/6lowpan/reassembly.c:65:23: sparse: call with no type!
net/ieee802154/6lowpan/reassembly.c:67:20: sparse: call with no type!
net/ieee802154/6lowpan/reassembly.c:68:22: sparse: call with no type!
In file included from include/linux/kernel.h:10:0,
from include/linux/list.h:9,
from include/linux/random.h:10,
from include/linux/net.h:22,
from net/ieee802154/6lowpan/reassembly.c:17:
net/ieee802154/6lowpan/reassembly.c: In function 'lowpan_frag_expire':
include/linux/kernel.h:964:51: error: dereferencing pointer to incomplete type 'struct frag_queue'
BUILD_BUG_ON_MSG(!__same_type(*(ptr), ((type *)0)->member) && 25- ^
include/linux/compiler.h:316:19: note: in definition of macro '__compiletime_assert'
bool __cond = !(condition); 28- ^~~~~~~~~
include/linux/compiler.h:339:2: note: in expansion of macro '_compiletime_assert'
_compiletime_assert(condition, msg, __compiletime_assert_, __LINE__)
^~~~~~~~~~~~~~~~~~~
include/linux/build_bug.h:45:37: note: in expansion of macro 'compiletime_assert'
#define BUILD_BUG_ON_MSG(cond, msg) compiletime_assert(!(cond), msg)
^~~~~~~~~~~~~~~~~~
include/linux/kernel.h:964:2: note: in expansion of macro 'BUILD_BUG_ON_MSG'
BUILD_BUG_ON_MSG(!__same_type(*(ptr), ((type *)0)->member) && 37- ^~~~~~~~~~~~~~~~
include/linux/kernel.h:964:20: note: in expansion of macro '__same_type'
BUILD_BUG_ON_MSG(!__same_type(*(ptr), ((type *)0)->member) && 40- ^~~~~~~~~~~
net/ieee802154/6lowpan/reassembly.c:57:7: note: in expansion of macro 'container_of'
fq = container_of(frag, struct frag_queue, q);
^~~~~~~~~~~~
In file included from include/linux/compiler_types.h:58:0,
from <command-line>:0:
include/linux/compiler-gcc.h:170:2: error: invalid use of undefined type 'struct frag_queue'
__builtin_offsetof(a, b)
^
include/linux/stddef.h:17:32: note: in expansion of macro '__compiler_offsetof'
#define offsetof(TYPE, MEMBER) __compiler_offsetof(TYPE, MEMBER)
^~~~~~~~~~~~~~~~~~~
include/linux/kernel.h:967:21: note: in expansion of macro 'offsetof'
((type *)(__mptr - offsetof(type, member))); })
^~~~~~~~
net/ieee802154/6lowpan/reassembly.c:57:7: note: in expansion of macro 'container_of'
fq = container_of(frag, struct frag_queue, q);
^~~~~~~~~~~~
net/ieee802154/6lowpan/reassembly.c: In function 'lowpan_net_frag_init':
net/ieee802154/6lowpan/reassembly.c:611:30: error: invalid application of 'sizeof' to incomplete type 'struct frag_queue'
lowpan_frags.qsize = sizeof(struct frag_queue);
^~~~~~
vim +57 net/ieee802154/6lowpan/reassembly.c
7240cdec6 net/ieee802154/reassembly.c Alexander Aring 2014-02-28 50
78802011f net/ieee802154/6lowpan/reassembly.c Kees Cook 2017-10-16 51 static void lowpan_frag_expire(struct timer_list *t)
7240cdec6 net/ieee802154/reassembly.c Alexander Aring 2014-02-28 52 {
78802011f net/ieee802154/6lowpan/reassembly.c Kees Cook 2017-10-16 53 struct inet_frag_queue *frag = from_timer(frag, t, timer);
7240cdec6 net/ieee802154/reassembly.c Alexander Aring 2014-02-28 54 struct frag_queue *fq;
7240cdec6 net/ieee802154/reassembly.c Alexander Aring 2014-02-28 55 struct net *net;
7240cdec6 net/ieee802154/reassembly.c Alexander Aring 2014-02-28 56
78802011f net/ieee802154/6lowpan/reassembly.c Kees Cook 2017-10-16 @57 fq = container_of(frag, struct frag_queue, q);
7240cdec6 net/ieee802154/reassembly.c Alexander Aring 2014-02-28 @58 net = container_of(fq->q.net, struct net, ieee802154_lowpan.frags);
7240cdec6 net/ieee802154/reassembly.c Alexander Aring 2014-02-28 59
177943260 net/ieee802154/reassembly.c Florian Westphal 2014-03-13 @60 spin_lock(&fq->q.lock);
177943260 net/ieee802154/reassembly.c Florian Westphal 2014-03-13 61
06aa8b8a0 net/ieee802154/reassembly.c Nikolay Aleksandrov 2014-08-01 62 if (fq->q.flags & INET_FRAG_COMPLETE)
177943260 net/ieee802154/reassembly.c Florian Westphal 2014-03-13 63 goto out;
177943260 net/ieee802154/reassembly.c Florian Westphal 2014-03-13 64
093ba7291 net/ieee802154/6lowpan/reassembly.c Eric Dumazet 2018-03-31 65 inet_frag_kill(&fq->q);
177943260 net/ieee802154/reassembly.c Florian Westphal 2014-03-13 66 out:
177943260 net/ieee802154/reassembly.c Florian Westphal 2014-03-13 67 spin_unlock(&fq->q.lock);
093ba7291 net/ieee802154/6lowpan/reassembly.c Eric Dumazet 2018-03-31 68 inet_frag_put(&fq->q);
7240cdec6 net/ieee802154/reassembly.c Alexander Aring 2014-02-28 69 }
7240cdec6 net/ieee802154/reassembly.c Alexander Aring 2014-02-28 70
:::::: The code at line 57 was first introduced by commit
:::::: 78802011fbe34331bdef6f2dfb1634011f0e4c32 inet: frags: Convert timers to use timer_setup()
:::::: TO: Kees Cook <keescook@chromium.org>
:::::: CC: David S. Miller <davem@davemloft.net>
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
^ permalink raw reply
* Re: [PATCH 00/14] ARM BPF jit compiler improvements
From: Daniel Borkmann @ 2018-07-13 13:24 UTC (permalink / raw)
To: Russell King - ARM Linux; +Cc: netdev, linux-arm-kernel
In-Reply-To: <20180712213547.GW17271@n2100.armlinux.org.uk>
On 07/12/2018 11:35 PM, Russell King - ARM Linux wrote:
> On Thu, Jul 12, 2018 at 11:12:45PM +0200, Daniel Borkmann wrote:
>> On 07/12/2018 11:02 PM, Russell King - ARM Linux wrote:
>>> On Thu, Jul 12, 2018 at 09:02:41PM +0200, Daniel Borkmann wrote:
>>>> Applied to bpf-next, thanks a lot Russell!
>>>
>>> Thanks, I've just sent four more patches, which is the sum total of
>>> what I'm intending to send for BPF improvements for the next merge
>>> window.
>>
>> Great, thanks a lot for the batch of improvements, Russell!
>>
>> Did you manage to get the BPF kselftest suite working on arm32 under
>> tools/testing/selftests/bpf/? In particular the test_verfier with
>> bpf_jit_enabled set to 1 and test_kmod.sh has a bigger number of
>> runtime tests that would stress it.
>
> I have a big issue with almost all of the tools/ subdirectory, and
> that is that it isn't "portable".
>
> It seems that cross-build environments just weren't considered when
> the tools subdirectory was created - it appears to require the entire
> kernel tree and build tree to be accessible on the target in order
> to build almost everything there. (I also exclusively do split-object
> builds, I never do an in-source-tree build.)
>
> At least perf has the ability to ask Kbuild to package it up as a
> tar.* file. That can be easily transported to the target as a
> self-contained buildable tree, and then be able to built from that.
>
> My cross-build environment for the kernel is just for building
> kernels, it does not have the facilities to build for userspace - I
> have a wide range of userspaces across targets, with a multitude of
> different glibc versions, and even when they're compatible versions,
> they're built differently.
>
> As far as I can see, basically, most tools/ stuff requires too much
> effort to work around this to be of any use to me. Even if I did
> unpick it from the kernel source tree by hand, that would be wasted
> effort, because I'd need to repeat that same process whenever
> anything there gets updated.
Right, that's unfortunate, although there is one option which you could
try out. The test_kmod.sh does nothing more than insmodding the test_bpf
kernel module built from lib/test_bpf.c. This one at least has all the
cBPF tests and a couple of eBPF ones (though most for the latter have
been moved to test_verfier). You can enable it via CONFIG_TEST_BPF=m and
then load it with bpf_jit_enabled set to 1. Hope that helps a bit.
Thanks,
Daniel
^ permalink raw reply
* Re: [PATCH v3 net-next] net/sched: add skbprio scheduler
From: Marcelo Ricardo Leitner @ 2018-07-13 13:04 UTC (permalink / raw)
To: Cong Wang
Cc: Michel Machado, Nishanth Devarajan, Jamal Hadi Salim, Jiri Pirko,
David Miller, Linux Kernel Network Developers, Cody Doucette
In-Reply-To: <CAM_iQpWakUAx8aLiSg23h4dZHuGxw5DdNacFg3AF944-Q4PzHw@mail.gmail.com>
On Thu, Jul 12, 2018 at 11:05:45PM -0700, Cong Wang wrote:
> On Wed, Jul 11, 2018 at 12:33 PM Marcelo Ricardo Leitner
> <marcelo.leitner@gmail.com> wrote:
> >
> > On Tue, Jul 10, 2018 at 07:25:53PM -0700, Cong Wang wrote:
> > > On Mon, Jul 9, 2018 at 2:40 PM Marcelo Ricardo Leitner
> > > <marcelo.leitner@gmail.com> wrote:
> > > >
> > > > On Mon, Jul 09, 2018 at 05:03:31PM -0400, Michel Machado wrote:
> > > > > Changing TC_PRIO_MAX from 15 to 63 risks breaking backward compatibility
> > > > > with applications.
> > > >
> > > > If done, it needs to be done carefully, indeed. I don't know if it's
> > > > doable, neither I know how hard is your requirement for 64 different
> > > > priorities.
> > >
> > > struct tc_prio_qopt {
> > > int bands; /* Number of bands */
> > > __u8 priomap[TC_PRIO_MAX+1]; /* Map: logical priority -> PRIO band */
> > > };
> > >
> > > How would you do it carefully?
> >
> > quick shot, multiplex v1 and v2 formats based on bands and sizeof():
> >
> > #define TCQ_PRIO_BANDS_V1 16
> > #define TCQ_PRIO_BANDS_V2 64
> > #define TC_PRIO_MAX_V2 64
> >
> > struct tc_prio_qopt_v2 {
> > int bands; /* Number of bands */
> > __u8 priomap[TC_PRIO_MAX_V2+1]; /* Map: logical priority -> PRIO band */
> > };
> >
>
> Good try, but:
>
> 1. You don't take padding into account, although the difference
> between 16 and 64 is big here. If it were 16 and 20, almost certainly
> wouldn't work.
It still would work, no matter how much padding you have, as currently
you can't use more than 3 bands.
>
> 2. What if I compile a new iproute2 on an old kernel? The iproute2
> will use V2, while old kernel has no knowledge of V2, so it only
> copies a part of V2 in the end....
Yes, and that's not a problem:
- Either bands is > 3 and it will return EINVAL, protecting from
reading beyond the buffer.
- Or 2 <= bands <= 3 and it will handle it as a _v1 struct, and use
only the original size.
iproute2 (or other app) may still use _v1 if it wants, btw.
^ permalink raw reply
* Re: [PATCH nf-next 1/2] ipv6: remove dependency of nf_defrag_ipv6 on ipv6 module
From: kbuild test robot @ 2018-07-13 13:00 UTC (permalink / raw)
To: Florian Westphal
Cc: kbuild-all, netfilter-devel, netdev, arnd, Florian Westphal
In-Reply-To: <20180712143547.2194-1-fw@strlen.de>
[-- Attachment #1: Type: text/plain, Size: 7639 bytes --]
Hi Florian,
I love your patch! Yet something to improve:
[auto build test ERROR on nf-next/master]
url: https://github.com/0day-ci/linux/commits/Florian-Westphal/ipv6-remove-dependency-of-nf_defrag_ipv6-on-ipv6-module/20180713-064909
base: https://git.kernel.org/pub/scm/linux/kernel/git/pablo/nf-next.git master
config: x86_64-fedora-25 (attached as .config)
compiler: gcc-7 (Debian 7.3.0-16) 7.3.0
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64
All error/warnings (new ones prefixed by >>):
In file included from include/linux/kernel.h:10:0,
from include/linux/list.h:9,
from include/linux/random.h:10,
from include/linux/net.h:22,
from net/ieee802154/6lowpan/reassembly.c:17:
net/ieee802154/6lowpan/reassembly.c: In function 'lowpan_frag_expire':
>> include/linux/kernel.h:964:51: error: dereferencing pointer to incomplete type 'struct frag_queue'
BUILD_BUG_ON_MSG(!__same_type(*(ptr), ((type *)0)->member) && \
^
include/linux/compiler.h:316:19: note: in definition of macro '__compiletime_assert'
bool __cond = !(condition); \
^~~~~~~~~
include/linux/compiler.h:339:2: note: in expansion of macro '_compiletime_assert'
_compiletime_assert(condition, msg, __compiletime_assert_, __LINE__)
^~~~~~~~~~~~~~~~~~~
include/linux/build_bug.h:45:37: note: in expansion of macro 'compiletime_assert'
#define BUILD_BUG_ON_MSG(cond, msg) compiletime_assert(!(cond), msg)
^~~~~~~~~~~~~~~~~~
include/linux/kernel.h:964:2: note: in expansion of macro 'BUILD_BUG_ON_MSG'
BUILD_BUG_ON_MSG(!__same_type(*(ptr), ((type *)0)->member) && \
^~~~~~~~~~~~~~~~
include/linux/kernel.h:964:20: note: in expansion of macro '__same_type'
BUILD_BUG_ON_MSG(!__same_type(*(ptr), ((type *)0)->member) && \
^~~~~~~~~~~
>> net/ieee802154/6lowpan/reassembly.c:57:7: note: in expansion of macro 'container_of'
fq = container_of(frag, struct frag_queue, q);
^~~~~~~~~~~~
In file included from include/linux/compiler_types.h:58:0,
from <command-line>:0:
>> include/linux/compiler-gcc.h:170:2: error: invalid use of undefined type 'struct frag_queue'
__builtin_offsetof(a, b)
^
include/linux/stddef.h:17:32: note: in expansion of macro '__compiler_offsetof'
#define offsetof(TYPE, MEMBER) __compiler_offsetof(TYPE, MEMBER)
^~~~~~~~~~~~~~~~~~~
include/linux/kernel.h:967:21: note: in expansion of macro 'offsetof'
((type *)(__mptr - offsetof(type, member))); })
^~~~~~~~
>> net/ieee802154/6lowpan/reassembly.c:57:7: note: in expansion of macro 'container_of'
fq = container_of(frag, struct frag_queue, q);
^~~~~~~~~~~~
net/ieee802154/6lowpan/reassembly.c: In function 'lowpan_net_frag_init':
>> net/ieee802154/6lowpan/reassembly.c:611:30: error: invalid application of 'sizeof' to incomplete type 'struct frag_queue'
lowpan_frags.qsize = sizeof(struct frag_queue);
^~~~~~
--
In file included from include/linux/kernel.h:10:0,
from include/linux/list.h:9,
from include/linux/random.h:10,
from include/linux/net.h:22,
from net/ieee802154//6lowpan/reassembly.c:17:
net/ieee802154//6lowpan/reassembly.c: In function 'lowpan_frag_expire':
>> include/linux/kernel.h:964:51: error: dereferencing pointer to incomplete type 'struct frag_queue'
BUILD_BUG_ON_MSG(!__same_type(*(ptr), ((type *)0)->member) && \
^
include/linux/compiler.h:316:19: note: in definition of macro '__compiletime_assert'
bool __cond = !(condition); \
^~~~~~~~~
include/linux/compiler.h:339:2: note: in expansion of macro '_compiletime_assert'
_compiletime_assert(condition, msg, __compiletime_assert_, __LINE__)
^~~~~~~~~~~~~~~~~~~
include/linux/build_bug.h:45:37: note: in expansion of macro 'compiletime_assert'
#define BUILD_BUG_ON_MSG(cond, msg) compiletime_assert(!(cond), msg)
^~~~~~~~~~~~~~~~~~
include/linux/kernel.h:964:2: note: in expansion of macro 'BUILD_BUG_ON_MSG'
BUILD_BUG_ON_MSG(!__same_type(*(ptr), ((type *)0)->member) && \
^~~~~~~~~~~~~~~~
include/linux/kernel.h:964:20: note: in expansion of macro '__same_type'
BUILD_BUG_ON_MSG(!__same_type(*(ptr), ((type *)0)->member) && \
^~~~~~~~~~~
net/ieee802154//6lowpan/reassembly.c:57:7: note: in expansion of macro 'container_of'
fq = container_of(frag, struct frag_queue, q);
^~~~~~~~~~~~
In file included from include/linux/compiler_types.h:58:0,
from <command-line>:0:
>> include/linux/compiler-gcc.h:170:2: error: invalid use of undefined type 'struct frag_queue'
__builtin_offsetof(a, b)
^
include/linux/stddef.h:17:32: note: in expansion of macro '__compiler_offsetof'
#define offsetof(TYPE, MEMBER) __compiler_offsetof(TYPE, MEMBER)
^~~~~~~~~~~~~~~~~~~
include/linux/kernel.h:967:21: note: in expansion of macro 'offsetof'
((type *)(__mptr - offsetof(type, member))); })
^~~~~~~~
net/ieee802154//6lowpan/reassembly.c:57:7: note: in expansion of macro 'container_of'
fq = container_of(frag, struct frag_queue, q);
^~~~~~~~~~~~
net/ieee802154//6lowpan/reassembly.c: In function 'lowpan_net_frag_init':
net/ieee802154//6lowpan/reassembly.c:611:30: error: invalid application of 'sizeof' to incomplete type 'struct frag_queue'
lowpan_frags.qsize = sizeof(struct frag_queue);
^~~~~~
vim +964 include/linux/kernel.h
cf14f27f Alexei Starovoitov 2018-03-28 954
^1da177e Linus Torvalds 2005-04-16 955 /**
^1da177e Linus Torvalds 2005-04-16 956 * container_of - cast a member of a structure out to the containing structure
^1da177e Linus Torvalds 2005-04-16 957 * @ptr: the pointer to the member.
^1da177e Linus Torvalds 2005-04-16 958 * @type: the type of the container struct this is embedded in.
^1da177e Linus Torvalds 2005-04-16 959 * @member: the name of the member within the struct.
^1da177e Linus Torvalds 2005-04-16 960 *
^1da177e Linus Torvalds 2005-04-16 961 */
^1da177e Linus Torvalds 2005-04-16 962 #define container_of(ptr, type, member) ({ \
c7acec71 Ian Abbott 2017-07-12 963 void *__mptr = (void *)(ptr); \
c7acec71 Ian Abbott 2017-07-12 @964 BUILD_BUG_ON_MSG(!__same_type(*(ptr), ((type *)0)->member) && \
c7acec71 Ian Abbott 2017-07-12 965 !__same_type(*(ptr), void), \
c7acec71 Ian Abbott 2017-07-12 966 "pointer type mismatch in container_of()"); \
c7acec71 Ian Abbott 2017-07-12 967 ((type *)(__mptr - offsetof(type, member))); })
^1da177e Linus Torvalds 2005-04-16 968
:::::: The code at line 964 was first introduced by commit
:::::: c7acec713d14c6ce8a20154f9dfda258d6bcad3b kernel.h: handle pointers to arrays better in container_of()
:::::: TO: Ian Abbott <abbotti@mev.co.uk>
:::::: CC: Linus Torvalds <torvalds@linux-foundation.org>
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 48168 bytes --]
^ permalink raw reply
* Re: [PATCH v3 net-next] net/sched: add skbprio scheduler
From: Marcelo Ricardo Leitner @ 2018-07-13 13:00 UTC (permalink / raw)
To: Cong Wang
Cc: Michel Machado, Nishanth Devarajan, Jamal Hadi Salim, Jiri Pirko,
David Miller, Linux Kernel Network Developers, Cody Doucette
In-Reply-To: <CAM_iQpU6rsFEF9hHcN7JUqgtQvX9bR=teW6nCHMdoW_UJ8KSZw@mail.gmail.com>
On Thu, Jul 12, 2018 at 10:07:30PM -0700, Cong Wang wrote:
> On Wed, Jul 11, 2018 at 11:37 AM Marcelo Ricardo Leitner
> <marcelo.leitner@gmail.com> wrote:
> >
> > On Tue, Jul 10, 2018 at 07:32:43PM -0700, Cong Wang wrote:
> > > On Mon, Jul 9, 2018 at 12:53 PM Marcelo Ricardo Leitner
> > > <marcelo.leitner@gmail.com> wrote:
> > > >
> > > > On Mon, Jul 09, 2018 at 02:18:33PM -0400, Michel Machado wrote:
> > > > >
> > > > > 2. sch_prio.c does not have a global limit on the number of packets on
> > > > > all its queues, only a limit per queue.
> > > >
> > > > It can be useful to sch_prio.c as well, why not?
> > > > prio_enqueue()
> > > > {
> > > > ...
> > > > + if (count > sch->global_limit)
> > > > + prio_tail_drop(sch); /* to be implemented */
> > > > ret = qdisc_enqueue(skb, qdisc, to_free);
> > > >
> > >
> > > Isn't the whole point of sch_prio offloading the queueing to
> > > each class? If you need a limit, there is one for each child
> > > qdisc if you use for example pfifo or bfifo (depending on you
> > > want to limit bytes or packets).
> >
> > Yes, but Michel wants to drop from other lower priorities if needed,
> > and that's not possible if you handle the limit already in a child
> > qdisc as they don't know about their siblings. The idea in the example
> > above is to discard it from whatever lower priority is needed, then
> > queue it. (ok, the example missed to check the priority level)
>
> So it disproves your point of adding a flag to sch_prio, right?
I don't see how?
>
> Also, you have to re-introduce qdisc->ops->drop() if you really want
> to go this direction.
Again, yes. What's the deal with it?
>
> >
> > As for the different units, sch_prio holds a count of how many packets
> > are queued on its children, and that's what would be used for the limit.
> >
> > >
> > > Also, what's your plan for backward compatibility here?
> >
> > say:
> > if (sch->global_limit && count > sch->global_limit)
> > as in, only do the limit check/enforcing if needed.
>
> Obviously doesn't work, users could pass 0 to effectively
> disable the qdisc from enqueue'ing any packet.
If you only had considered the right 'limit' variable, you would be
right here.
^ permalink raw reply
* Re: [PATCH net] skbuff: Unconditionally copy pfmemalloc in __skb_clone()
From: Sabrina Dubroca @ 2018-07-13 12:45 UTC (permalink / raw)
To: Stefano Brivio
Cc: David S. Miller, Mel Gorman, Eric Dumazet, Patrick Talbert,
netdev
In-Reply-To: <bf1a276ec28a2a0e6b94b43eec3975e64d1c63ae.1531479681.git.sbrivio@redhat.com>
2018-07-13, 13:21:07 +0200, Stefano Brivio wrote:
> Commit 8b7008620b84 ("net: Don't copy pfmemalloc flag in
> __copy_skb_header()") introduced a different handling for the
> pfmemalloc flag in copy and clone paths.
>
> In __skb_clone(), now, the flag is set only if it was set in the
> original skb, but not cleared if it wasn't. This is wrong and
> might lead to socket buffers being flagged with pfmemalloc even
> if the skb data wasn't allocated from pfmemalloc reserves. Copy
> the flag instead of ORing it.
>
> Reported-by: Sabrina Dubroca <sd@queasysnail.net>
> Fixes: 8b7008620b84 ("net: Don't copy pfmemalloc flag in __copy_skb_header()")
> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
Thanks,
Tested-by: Sabrina Dubroca <sd@queasysnail.net>
--
Sabrina
^ 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