* Re: [PATCH v2] can: m_can: Move accessing of message ram to after clocks are enabled
From: Marc Kleine-Budde @ 2018-07-23 8:07 UTC (permalink / raw)
To: Faiz Abbas, linux-kernel, netdev, linux-can, linux-omap; +Cc: wg, davem
In-Reply-To: <20180703111710.27084-1-faiz_abbas@ti.com>
[-- Attachment #1.1: Type: text/plain, Size: 571 bytes --]
On 07/03/2018 01:17 PM, Faiz Abbas wrote:
> MCAN message ram should only be accessed once clocks are enabled.
> Therefore, move the call to parse/init the message ram to after
> clocks are enabled.
>
> Signed-off-by: Faiz Abbas <faiz_abbas@ti.com>
applied to linux-can.
tnx,
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Industrial Linux Solutions | Phone: +49-231-2826-924 |
Vertretung West/Dortmund | Fax: +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* [PATCH bpf] xsk: fix poll/POLLIN premature returns
From: Björn Töpel @ 2018-07-23 9:43 UTC (permalink / raw)
To: bjorn.topel, magnus.karlsson, ast, daniel, netdev
Cc: Björn Töpel, qi.z.zhang
From: Björn Töpel <bjorn.topel@intel.com>
Polling for the ingress queues relies on reading the producer/consumer
pointers of the Rx queue.
Prior this commit, a cached consumer pointer could be used, instead of
the actual consumer pointer and therefore report POLLIN prematurely.
This patch makes sure that the non-cached consumer pointer is used
instead.
Reported-by: Qi Zhang <qi.z.zhang@intel.com>
Tested-by: Qi Zhang <qi.z.zhang@intel.com>
Fixes: c497176cb2e4 ("xsk: add Rx receive functions and poll support")
Signed-off-by: Björn Töpel <bjorn.topel@intel.com>
---
net/xdp/xsk_queue.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/xdp/xsk_queue.h b/net/xdp/xsk_queue.h
index 52ecaf770642..8a64b150be54 100644
--- a/net/xdp/xsk_queue.h
+++ b/net/xdp/xsk_queue.h
@@ -250,7 +250,7 @@ static inline bool xskq_full_desc(struct xsk_queue *q)
static inline bool xskq_empty_desc(struct xsk_queue *q)
{
- return xskq_nb_free(q, q->prod_tail, 1) == q->nentries;
+ return xskq_nb_free(q, q->prod_tail, q->nentries) == q->nentries;
}
void xskq_set_umem(struct xsk_queue *q, struct xdp_umem_props *umem_props);
--
2.17.1
^ permalink raw reply related
* Re: [PATCH] netlink: fix memory leak of dump
From: Pablo Neira Ayuso @ 2018-07-23 10:43 UTC (permalink / raw)
To: shaochun chen
Cc: Florian Westphal, David Miller, kadlec, johannes.berg, jason,
ktkhai, lucien.xin, xiyou.wangcong, dsahern, netfilter-devel, tom,
netdev, linux-kernel
In-Reply-To: <CAKDdixBBd08eKvk-hJtwPy46u2uzihRCh_KdEMF3xG1ZJ6oNsg@mail.gmail.com>
On Mon, Jul 23, 2018 at 06:34:36PM +0800, shaochun chen wrote:
> I have a question: we will try_module_get in __netlink_dump_start(),
> but why we need to call try_module_get again in nft_netlink_dump_start ??
Because they refer to two different modules. nfnetlink is multiplexing
all netfilter subsystem through one single netlink bus.
At the time that decision was made, there were concerns about netlink
running out of busses.
^ permalink raw reply
* Re: [PATCH bpf] xdp: add NULL pointer check in __xdp_return()
From: Björn Töpel @ 2018-07-23 9:41 UTC (permalink / raw)
To: ap420073
Cc: kafai, Daniel Borkmann, ast, Björn Töpel,
Jesper Dangaard Brouer, Netdev
In-Reply-To: <CAMArcTU8W1hKv00emOzdRx8ZrjDO4BBpwd5ZPzbekOhqCT0MJw@mail.gmail.com>
Den lör 21 juli 2018 kl 14:58 skrev Taehee Yoo <ap420073@gmail.com>:
>
> 2018-07-21 2:18 GMT+09:00 Martin KaFai Lau <kafai@fb.com>:
> > On Sat, Jul 21, 2018 at 01:04:45AM +0900, Taehee Yoo wrote:
> >> rhashtable_lookup() can return NULL. so that NULL pointer
> >> check routine should be added.
> >>
> >> Fixes: 02b55e5657c3 ("xdp: add MEM_TYPE_ZERO_COPY")
> >> Signed-off-by: Taehee Yoo <ap420073@gmail.com>
> >> ---
> >> net/core/xdp.c | 3 ++-
> >> 1 file changed, 2 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/net/core/xdp.c b/net/core/xdp.c
> >> index 9d1f220..1c12bc7 100644
> >> --- a/net/core/xdp.c
> >> +++ b/net/core/xdp.c
> >> @@ -345,7 +345,8 @@ static void __xdp_return(void *data, struct xdp_mem_info *mem, bool napi_direct,
> >> rcu_read_lock();
> >> /* mem->id is valid, checked in xdp_rxq_info_reg_mem_model() */
> >> xa = rhashtable_lookup(mem_id_ht, &mem->id, mem_id_rht_params);
> >> - xa->zc_alloc->free(xa->zc_alloc, handle);
> >> + if (xa)
> >> + xa->zc_alloc->free(xa->zc_alloc, handle);
> > hmm...It is not clear to me the "!xa" case don't have to be handled?
>
> Thank you for reviewing!
>
> Returning NULL pointer is bug case such as calling after use
> xdp_rxq_info_unreg().
> so that, I think it can't handle at that moment.
> we can make __xdp_return to add WARN_ON_ONCE() or
> add return error code to driver.
> But I'm not sure if these is useful information.
>
> I might have misunderstood scenario of MEM_TYPE_ZERO_COPY
> because there is no use case of MEM_TYPE_ZERO_COPY yet.
>
Taehee, again, sorry for the slow response and thanks for patch!
If xa is NULL, the driver has a buggy/broken implementation. What
would be a proper way of dealing with this? BUG?
Björn
> Thanks!
>
> >
> >> rcu_read_unlock();
> >> default:
> >> /* Not possible, checked in xdp_rxq_info_reg_mem_model() */
> >> --
> >> 2.9.3
> >>
^ permalink raw reply
* Re: [PATCH bpf] xdp: add NULL pointer check in __xdp_return()
From: Björn Töpel @ 2018-07-23 9:39 UTC (permalink / raw)
To: Jakub Kicinski
Cc: Daniel Borkmann, Björn Töpel, Jesper Dangaard Brouer,
kafai, ap420073, ast, Netdev, Karlsson, Magnus
In-Reply-To: <20180720130538.10c22c61@cakuba.netronome.com>
Den fre 20 juli 2018 kl 22:08 skrev Jakub Kicinski
<jakub.kicinski@netronome.com>:
>
> On Fri, 20 Jul 2018 10:18:21 -0700, Martin KaFai Lau wrote:
> > On Sat, Jul 21, 2018 at 01:04:45AM +0900, Taehee Yoo wrote:
> > > rhashtable_lookup() can return NULL. so that NULL pointer
> > > check routine should be added.
> > >
> > > Fixes: 02b55e5657c3 ("xdp: add MEM_TYPE_ZERO_COPY")
> > > Signed-off-by: Taehee Yoo <ap420073@gmail.com>
> > > ---
> > > net/core/xdp.c | 3 ++-
> > > 1 file changed, 2 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/net/core/xdp.c b/net/core/xdp.c
> > > index 9d1f220..1c12bc7 100644
> > > --- a/net/core/xdp.c
> > > +++ b/net/core/xdp.c
> > > @@ -345,7 +345,8 @@ static void __xdp_return(void *data, struct xdp_mem_info *mem, bool napi_direct,
> > > rcu_read_lock();
> > > /* mem->id is valid, checked in xdp_rxq_info_reg_mem_model() */
> > > xa = rhashtable_lookup(mem_id_ht, &mem->id, mem_id_rht_params);
> > > - xa->zc_alloc->free(xa->zc_alloc, handle);
> > > + if (xa)
> > > + xa->zc_alloc->free(xa->zc_alloc, handle);
> > hmm...It is not clear to me the "!xa" case don't have to be handled?
>
> Actually I have a more fundamental question about this interface I've
> been meaning to ask.
>
> IIUC free() can happen on any CPU at any time, when whatever device,
> socket or CPU this got redirected to completed the TX. IOW there may
> be multiple producers. Drivers would need to create spin lock a'la the
> a9744f7ca200 ("xsk: fix potential race in SKB TX completion code") fix?
>
Jakub, apologies for the slow response. I'm still in
"holiday/hammock&beer mode", but will be back in a week. :-P
The idea with the xdp_return_* functions are that an xdp_buff and
xdp_frame can have custom allocations schemes. The difference beween
struct xdp_buff and struct xdp_frame is lifetime. The xdp_buff
lifetime is within the napi context, whereas xdp_frame can have a
lifetime longer/outside the napi context. E.g. for a XDP_REDIRECT
scenario an xdp_buff is converted to a xdp_frame. The conversion is
done in include/net/xdp.h:convert_to_xdp_frame.
Currently, the zero-copy MEM_TYPE_ZERO_COPY memtype can *only* be used
for xdp_buff, meaning that the lifetime is constrained to a napi
context. Further, given an xdp_buff with memtype MEM_TYPE_ZERO_COPY,
doing XDP_REDIRECT to a target that is *not* an AF_XDP socket would
mean converting the xdp_buff to an xdp_frame. The xdp_frame can then
be free'd on any CPU.
Note that the xsk_rcv* functions is always called from an napi
context, and therefore is using the xdp_return_buff calls.
To answer your question -- no, this fix is *not* needed, because the
xdp_buff napi constrained, and the xdp_buff will only be free'd on one
CPU.
> We need some form of internal kernel circulation which would be MPSC.
> I'm currently hacking up the XSK code to tell me whether the frame was
> consumed by the correct XSK, and always clone the frame otherwise
> (claiming to be the "traditional" MEM_TYPE_PAGE_ORDER0).
>
> I feel like I'm missing something about the code. Is redirect of
> ZC/UMEM frame outside the xsk not possible and the only returns we will
> see are from net/xdp/xsk.c? That would work, but I don't see such a
> check. Help would be appreciated.
>
Right now, this is the case (refer to the TODO in
convert_to_xdp_frame), i.e. you cannot redirect an ZC/UMEM allocated
xdp_buff to a target that is not an xsk. This must, obviously, change
so that an xdp_buff (of MEM_TYPE_ZERO_COPY) can be converted to an
xdp_frame. The xdp_frame must be able to be free'd from multiple CPUs,
so here the a more sophisticated allocation scheme is required.
> Also the fact that XSK bufs can't be freed, only completed, adds to the
> pain of implementing AF_XDP, we'd certainly need some form of "give
> back the frame, but I may need it later" SPSC mechanism, otherwise
> driver writers will have tough time. Unless, again, I'm missing
> something about the code :)
>
Yup, moving the recycling scheme from driver to "generic" is a good
idea! I need to finish up those i40e zerocopy patches first though...
(...and I'm very excited that you're doing nfp support for AF_XDP!!!)
Björn
> > > rcu_read_unlock();
> > > default:
> > > /* Not possible, checked in xdp_rxq_info_reg_mem_model() */
>
^ permalink raw reply
* Re: [PATCH] netlink: fix memory leak of dump
From: shaochun chen @ 2018-07-23 10:34 UTC (permalink / raw)
To: Florian Westphal
Cc: Pablo Neira Ayuso, David Miller, kadlec, johannes.berg, jason,
ktkhai, lucien.xin, xiyou.wangcong, dsahern, netfilter-devel, tom,
netdev, linux-kernel
In-Reply-To: <CAKDdixCx5za07R9KdyohsZjJqzdTpE3mR1+4TwY5xjeBGVeTjg@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 2959 bytes --]
I have a question: we will try_module_get in __netlink_dump_start(),
but why we need to call try_module_get again in nft_netlink_dump_start ??
2018-07-23 17:52 GMT+08:00 shaochun chen <cscnull@gmail.com>:
> allocate memory in ->start(), it's not convenient for users.
> if call ->done() isn't ok for clean memory when netlink_dump_start() fail,
> maybe we should have another function ->clean() to clean memory.
>
> 2018-07-23 17:28 GMT+08:00 Florian Westphal <fw@strlen.de>:
>
>> Pablo Neira Ayuso <pablo@netfilter.org> wrote:
>> > > diff --git a/net/netfilter/nf_tables_api.c
>> b/net/netfilter/nf_tables_api.c
>> > > --- a/net/netfilter/nf_tables_api.c
>> > > +++ b/net/netfilter/nf_tables_api.c
>> > > @@ -5010,6 +5013,22 @@ nft_obj_filter_alloc(const struct nlattr *
>> const nla[])
>> > > return filter;
>> > > }
>> > >
>> > > +static int nf_tables_dump_obj_start(struct netlink_callback *cb)
>> > > +{
>> > > + const struct nlattr * const *nla = cb->data;
>>
>> On-Stack input.
>> I can't see how its wrong, ->start() happens from same context as
>> netlink_dump_start so its valid.
>>
>> > > + struct nft_obj_filter *filter = NULL;
>> > > +
>> > > + if (nla[NFTA_OBJ_TABLE] ||
>> > > + nla[NFTA_OBJ_TYPE]) {
>> > > + filter = nft_obj_filter_alloc(nla);
>> > > + if (IS_ERR(filter))
>> > > + return -ENOMEM;
>> > > + }
>> > > +
>> > > + cb->data = filter;
>>
>> And this replaced the on-stack input with dynamically
>> allocated one, which will be free'd via ->done().
>>
>> > > /* called with rcu_read_lock held */
>> > > static int nf_tables_getobj(struct net *net, struct sock *nlsk,
>> > > struct sk_buff *skb, const struct nlmsghdr
>> *nlh,
>> > > @@ -5028,21 +5047,13 @@ static int nf_tables_getobj(struct net *net,
>> struct sock *nlsk,
>> > >
>> > > if (nlh->nlmsg_flags & NLM_F_DUMP) {
>> > > struct netlink_dump_control c = {
>> > > + .start = nf_tables_dump_obj_start,
>> > > .dump = nf_tables_dump_obj,
>> > > .done = nf_tables_dump_obj_done,
>> > > .module = THIS_MODULE,
>> > > + .data = (void *)nla,
>> >
>> > You cannot do this.
>> >
>> > nla is allocated in this stack.
>>
>> Yes.
>>
>> > the nla will not be available in the second recv(), it won't be there.
>>
>> Its replaced in ->start().
>>
>> As David pointed out, once ->start() returns 0 we set cb_running, i.e.
>> only after successful ->start() netlink core will call ->dump() again.
>>
>> So I see no problem setting ->data to onstack cookie and then
>> duplicating it to heap via kmemdup in ->start().
>>
>> As far as I can see netlink core offers all functionality already,
>> so we only need to switch netfilter to make use of it.
>>
>> If you disagree please let me know, otherwise I will cook up
>> a patch along this pattern for net/netfilter/*.
>>
>> Thanks.
>>
>
>
[-- Attachment #2: Type: text/html, Size: 4929 bytes --]
^ permalink raw reply
* Re: [PATCH] netlink: fix memory leak of dump
From: Florian Westphal @ 2018-07-23 9:28 UTC (permalink / raw)
To: Pablo Neira Ayuso
Cc: Florian Westphal, David Miller, cscnull, kadlec, johannes.berg,
Jason, ktkhai, lucien.xin, xiyou.wangcong, dsahern,
netfilter-devel, tom, netdev, linux-kernel
In-Reply-To: <20180723091551.mwhltw4ujm4bylvj@salvia>
Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> > diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
> > --- a/net/netfilter/nf_tables_api.c
> > +++ b/net/netfilter/nf_tables_api.c
> > @@ -5010,6 +5013,22 @@ nft_obj_filter_alloc(const struct nlattr * const nla[])
> > return filter;
> > }
> >
> > +static int nf_tables_dump_obj_start(struct netlink_callback *cb)
> > +{
> > + const struct nlattr * const *nla = cb->data;
On-Stack input.
I can't see how its wrong, ->start() happens from same context as
netlink_dump_start so its valid.
> > + struct nft_obj_filter *filter = NULL;
> > +
> > + if (nla[NFTA_OBJ_TABLE] ||
> > + nla[NFTA_OBJ_TYPE]) {
> > + filter = nft_obj_filter_alloc(nla);
> > + if (IS_ERR(filter))
> > + return -ENOMEM;
> > + }
> > +
> > + cb->data = filter;
And this replaced the on-stack input with dynamically
allocated one, which will be free'd via ->done().
> > /* called with rcu_read_lock held */
> > static int nf_tables_getobj(struct net *net, struct sock *nlsk,
> > struct sk_buff *skb, const struct nlmsghdr *nlh,
> > @@ -5028,21 +5047,13 @@ static int nf_tables_getobj(struct net *net, struct sock *nlsk,
> >
> > if (nlh->nlmsg_flags & NLM_F_DUMP) {
> > struct netlink_dump_control c = {
> > + .start = nf_tables_dump_obj_start,
> > .dump = nf_tables_dump_obj,
> > .done = nf_tables_dump_obj_done,
> > .module = THIS_MODULE,
> > + .data = (void *)nla,
>
> You cannot do this.
>
> nla is allocated in this stack.
Yes.
> the nla will not be available in the second recv(), it won't be there.
Its replaced in ->start().
As David pointed out, once ->start() returns 0 we set cb_running, i.e.
only after successful ->start() netlink core will call ->dump() again.
So I see no problem setting ->data to onstack cookie and then
duplicating it to heap via kmemdup in ->start().
As far as I can see netlink core offers all functionality already,
so we only need to switch netfilter to make use of it.
If you disagree please let me know, otherwise I will cook up
a patch along this pattern for net/netfilter/*.
Thanks.
^ permalink raw reply
* Re: [PATCH] netlink: fix memory leak of dump
From: Pablo Neira Ayuso @ 2018-07-23 9:15 UTC (permalink / raw)
To: Florian Westphal
Cc: David Miller, cscnull, kadlec, johannes.berg, Jason, ktkhai,
lucien.xin, xiyou.wangcong, dsahern, netfilter-devel, tom, netdev,
linux-kernel
In-Reply-To: <20180722180910.wcwhantwpm2nfxet@breakpoint.cc>
On Sun, Jul 22, 2018 at 08:09:10PM +0200, Florian Westphal wrote:
> David Miller <davem@davemloft.net> wrote:
> > From: Florian Westphal <fw@strlen.de>
> > Date: Sun, 22 Jul 2018 18:39:25 +0200
> >
> > > 3. change meaning of ->done() so its always called once ->start()
> > > was invoked (and returned 0), this requires audit of all
> > > places that provide .done to make sure they won't trip.
> > >
> > > 3) seems to be what Tom intended when he added .start, so probably
> > > best to investigate that first.
> >
> > Hmmm...
> >
> > Any time ->start() succeeds, we set cb_running to true.
>
> Right.
>
> > From that point forward, ->done() will be called at some point at all
> > of the locations that check if cb_running is true and set it to false.
>
> Also right, thanks for pointing this out, I missed fact that netlink
> core restarts a dump after this.
>
> So 3) is already true which means we should try to see if we can move
> all dump-related extra magic into ->start().
>
> Shaochun, can you see if this is possible?
>
> Something along these lines (totally untested), which makes this
> a netfilter fix:
>
> diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
> --- a/net/netfilter/nf_tables_api.c
> +++ b/net/netfilter/nf_tables_api.c
> @@ -5010,6 +5013,22 @@ nft_obj_filter_alloc(const struct nlattr * const nla[])
> return filter;
> }
>
> +static int nf_tables_dump_obj_start(struct netlink_callback *cb)
> +{
> + const struct nlattr * const *nla = cb->data;
> + struct nft_obj_filter *filter = NULL;
> +
> + if (nla[NFTA_OBJ_TABLE] ||
> + nla[NFTA_OBJ_TYPE]) {
> + filter = nft_obj_filter_alloc(nla);
> + if (IS_ERR(filter))
> + return -ENOMEM;
> + }
> +
> + cb->data = filter;
> + return 0;
> +}
> +
> /* called with rcu_read_lock held */
> static int nf_tables_getobj(struct net *net, struct sock *nlsk,
> struct sk_buff *skb, const struct nlmsghdr *nlh,
> @@ -5028,21 +5047,13 @@ static int nf_tables_getobj(struct net *net, struct sock *nlsk,
>
> if (nlh->nlmsg_flags & NLM_F_DUMP) {
> struct netlink_dump_control c = {
> + .start = nf_tables_dump_obj_start,
> .dump = nf_tables_dump_obj,
> .done = nf_tables_dump_obj_done,
> .module = THIS_MODULE,
> + .data = (void *)nla,
You cannot do this.
nla is allocated in this stack.
the nla will not be available in the second recv(), it won't be there.
^ permalink raw reply
* Re: [PATCH net-next v6 3/4] net: vhost: factor out busy polling logic to vhost_net_busy_poll()
From: Toshiaki Makita @ 2018-07-23 9:57 UTC (permalink / raw)
To: xiangxia.m.yue, jasowang; +Cc: netdev, virtualization, mst
In-Reply-To: <1532196242-2998-4-git-send-email-xiangxia.m.yue@gmail.com>
On 2018/07/22 3:04, xiangxia.m.yue@gmail.com wrote:
> From: Tonghao Zhang <xiangxia.m.yue@gmail.com>
>
> Factor out generic busy polling logic and will be
> used for in tx path in the next patch. And with the patch,
> qemu can set differently the busyloop_timeout for rx queue.
>
> Signed-off-by: Tonghao Zhang <xiangxia.m.yue@gmail.com>
> ---
...
> +static void vhost_net_busy_poll_vq_check(struct vhost_net *net,
> + struct vhost_virtqueue *rvq,
> + struct vhost_virtqueue *tvq,
> + bool rx)
> +{
> + struct socket *sock = rvq->private_data;
> +
> + if (rx) {
> + if (!vhost_vq_avail_empty(&net->dev, tvq)) {
> + vhost_poll_queue(&tvq->poll);
> + } else if (unlikely(vhost_enable_notify(&net->dev, tvq))) {
> + vhost_disable_notify(&net->dev, tvq);
> + vhost_poll_queue(&tvq->poll);
> + }
> + } else if ((sock && sk_has_rx_data(sock->sk)) &&
> + !vhost_vq_avail_empty(&net->dev, rvq)) {
> + vhost_poll_queue(&rvq->poll);
Now we wait for vq_avail for rx as well, I think you cannot skip
vhost_enable_notify() on tx. Probably you might want to do:
} else if (sock && sk_has_rx_data(sock->sk)) {
if (!vhost_vq_avail_empty(&net->dev, rvq)) {
vhost_poll_queue(&rvq->poll);
} else if (unlikely(vhost_enable_notify(&net->dev, rvq))) {
vhost_disable_notify(&net->dev, rvq);
vhost_poll_queue(&rvq->poll);
}
}
Also it's better to care vhost_net_disable_vq()/vhost_net_enable_vq() on tx?
--
Toshiaki Makita
^ permalink raw reply
* Re: [PATCH] netlink: fix memory leak of dump
From: shaochun chen @ 2018-07-23 9:52 UTC (permalink / raw)
To: Florian Westphal
Cc: Pablo Neira Ayuso, David Miller, kadlec, johannes.berg, jason,
ktkhai, lucien.xin, xiyou.wangcong, dsahern, netfilter-devel, tom,
netdev, linux-kernel
In-Reply-To: <20180723092818.ztsfsnqzxgzrauim@breakpoint.cc>
[-- Attachment #1: Type: text/plain, Size: 2666 bytes --]
allocate memory in ->start(), it's not convenient for users.
if call ->done() isn't ok for clean memory when netlink_dump_start() fail,
maybe we should have another function ->clean() to clean memory.
2018-07-23 17:28 GMT+08:00 Florian Westphal <fw@strlen.de>:
> Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> > > diff --git a/net/netfilter/nf_tables_api.c
> b/net/netfilter/nf_tables_api.c
> > > --- a/net/netfilter/nf_tables_api.c
> > > +++ b/net/netfilter/nf_tables_api.c
> > > @@ -5010,6 +5013,22 @@ nft_obj_filter_alloc(const struct nlattr *
> const nla[])
> > > return filter;
> > > }
> > >
> > > +static int nf_tables_dump_obj_start(struct netlink_callback *cb)
> > > +{
> > > + const struct nlattr * const *nla = cb->data;
>
> On-Stack input.
> I can't see how its wrong, ->start() happens from same context as
> netlink_dump_start so its valid.
>
> > > + struct nft_obj_filter *filter = NULL;
> > > +
> > > + if (nla[NFTA_OBJ_TABLE] ||
> > > + nla[NFTA_OBJ_TYPE]) {
> > > + filter = nft_obj_filter_alloc(nla);
> > > + if (IS_ERR(filter))
> > > + return -ENOMEM;
> > > + }
> > > +
> > > + cb->data = filter;
>
> And this replaced the on-stack input with dynamically
> allocated one, which will be free'd via ->done().
>
> > > /* called with rcu_read_lock held */
> > > static int nf_tables_getobj(struct net *net, struct sock *nlsk,
> > > struct sk_buff *skb, const struct nlmsghdr
> *nlh,
> > > @@ -5028,21 +5047,13 @@ static int nf_tables_getobj(struct net *net,
> struct sock *nlsk,
> > >
> > > if (nlh->nlmsg_flags & NLM_F_DUMP) {
> > > struct netlink_dump_control c = {
> > > + .start = nf_tables_dump_obj_start,
> > > .dump = nf_tables_dump_obj,
> > > .done = nf_tables_dump_obj_done,
> > > .module = THIS_MODULE,
> > > + .data = (void *)nla,
> >
> > You cannot do this.
> >
> > nla is allocated in this stack.
>
> Yes.
>
> > the nla will not be available in the second recv(), it won't be there.
>
> Its replaced in ->start().
>
> As David pointed out, once ->start() returns 0 we set cb_running, i.e.
> only after successful ->start() netlink core will call ->dump() again.
>
> So I see no problem setting ->data to onstack cookie and then
> duplicating it to heap via kmemdup in ->start().
>
> As far as I can see netlink core offers all functionality already,
> so we only need to switch netfilter to make use of it.
>
> If you disagree please let me know, otherwise I will cook up
> a patch along this pattern for net/netfilter/*.
>
> Thanks.
>
[-- Attachment #2: Type: text/html, Size: 3733 bytes --]
^ permalink raw reply
* Re: [PATCH] netlink: fix memory leak of dump
From: Pablo Neira Ayuso @ 2018-07-23 9:47 UTC (permalink / raw)
To: Florian Westphal
Cc: David Miller, cscnull, kadlec, johannes.berg, Jason, ktkhai,
lucien.xin, xiyou.wangcong, dsahern, netfilter-devel, tom, netdev,
linux-kernel
In-Reply-To: <20180723094228.mejwuvoyitolqktv@breakpoint.cc>
On Mon, Jul 23, 2018 at 11:42:28AM +0200, Florian Westphal wrote:
> Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> > > As David pointed out, once ->start() returns 0 we set cb_running, i.e.
> > > only after successful ->start() netlink core will call ->dump() again.
> > >
> > > So I see no problem setting ->data to onstack cookie and then
> > > duplicating it to heap via kmemdup in ->start().
> > >
> > > As far as I can see netlink core offers all functionality already,
> > > so we only need to switch netfilter to make use of it.
> > >
> > > If you disagree please let me know, otherwise I will cook up
> > > a patch along this pattern for net/netfilter/*.
> >
> > Why not just call ->done from netlink_dump_start() when it fails?
>
> Not sure thats safe for all users, we will also still need to call
> it in nft_netlink_dump_start and we need to play guess game wrt
> EINTR (which can mean 'dump was now started, do not send ack').
We can also add another scratchpad area, similar to the ->cb[x] area
that can be initialized before netlink_dump_start()? So we don't need
the data pointer.
By passing the array of attributes, we'll need to do attribute parsing
over and over again from each netlink_dump() call.
^ permalink raw reply
* Re: [PATCH] netlink: fix memory leak of dump
From: Florian Westphal @ 2018-07-23 9:42 UTC (permalink / raw)
To: Pablo Neira Ayuso
Cc: Florian Westphal, David Miller, cscnull, kadlec, johannes.berg,
Jason, ktkhai, lucien.xin, xiyou.wangcong, dsahern,
netfilter-devel, tom, netdev, linux-kernel
In-Reply-To: <20180723093855.c4ncixsnkee6r34v@salvia>
Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> > As David pointed out, once ->start() returns 0 we set cb_running, i.e.
> > only after successful ->start() netlink core will call ->dump() again.
> >
> > So I see no problem setting ->data to onstack cookie and then
> > duplicating it to heap via kmemdup in ->start().
> >
> > As far as I can see netlink core offers all functionality already,
> > so we only need to switch netfilter to make use of it.
> >
> > If you disagree please let me know, otherwise I will cook up
> > a patch along this pattern for net/netfilter/*.
>
> Why not just call ->done from netlink_dump_start() when it fails?
Not sure thats safe for all users, we will also still need to call
it in nft_netlink_dump_start and we need to play guess game wrt
EINTR (which can mean 'dump was now started, do not send ack').
^ permalink raw reply
* Re: [PATCH] netlink: fix memory leak of dump
From: Pablo Neira Ayuso @ 2018-07-23 9:38 UTC (permalink / raw)
To: Florian Westphal
Cc: David Miller, cscnull, kadlec, johannes.berg, Jason, ktkhai,
lucien.xin, xiyou.wangcong, dsahern, netfilter-devel, tom, netdev,
linux-kernel
In-Reply-To: <20180723092818.ztsfsnqzxgzrauim@breakpoint.cc>
On Mon, Jul 23, 2018 at 11:28:18AM +0200, Florian Westphal wrote:
> Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> > > diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
> > > --- a/net/netfilter/nf_tables_api.c
> > > +++ b/net/netfilter/nf_tables_api.c
> > > @@ -5010,6 +5013,22 @@ nft_obj_filter_alloc(const struct nlattr * const nla[])
> > > return filter;
> > > }
> > >
> > > +static int nf_tables_dump_obj_start(struct netlink_callback *cb)
> > > +{
> > > + const struct nlattr * const *nla = cb->data;
>
> On-Stack input.
> I can't see how its wrong, ->start() happens from same context as
> netlink_dump_start so its valid.
>
> > > + struct nft_obj_filter *filter = NULL;
> > > +
> > > + if (nla[NFTA_OBJ_TABLE] ||
> > > + nla[NFTA_OBJ_TYPE]) {
> > > + filter = nft_obj_filter_alloc(nla);
> > > + if (IS_ERR(filter))
> > > + return -ENOMEM;
> > > + }
> > > +
> > > + cb->data = filter;
>
> And this replaced the on-stack input with dynamically
> allocated one, which will be free'd via ->done().
>
> > > /* called with rcu_read_lock held */
> > > static int nf_tables_getobj(struct net *net, struct sock *nlsk,
> > > struct sk_buff *skb, const struct nlmsghdr *nlh,
> > > @@ -5028,21 +5047,13 @@ static int nf_tables_getobj(struct net *net, struct sock *nlsk,
> > >
> > > if (nlh->nlmsg_flags & NLM_F_DUMP) {
> > > struct netlink_dump_control c = {
> > > + .start = nf_tables_dump_obj_start,
> > > .dump = nf_tables_dump_obj,
> > > .done = nf_tables_dump_obj_done,
> > > .module = THIS_MODULE,
> > > + .data = (void *)nla,
> >
> > You cannot do this.
> >
> > nla is allocated in this stack.
>
> Yes.
>
> > the nla will not be available in the second recv(), it won't be there.
>
> Its replaced in ->start().
>
> As David pointed out, once ->start() returns 0 we set cb_running, i.e.
> only after successful ->start() netlink core will call ->dump() again.
>
> So I see no problem setting ->data to onstack cookie and then
> duplicating it to heap via kmemdup in ->start().
>
> As far as I can see netlink core offers all functionality already,
> so we only need to switch netfilter to make use of it.
>
> If you disagree please let me know, otherwise I will cook up
> a patch along this pattern for net/netfilter/*.
Why not just call ->done from netlink_dump_start() when it fails?
^ permalink raw reply
* Re: [PATCH] Add a missing break in parse_station_flags
From: Kalle Valo @ 2018-07-23 9:30 UTC (permalink / raw)
To: Bernd Edlinger
Cc: Johannes Berg, David S. Miller, linux-wireless@vger.kernel.org,
netdev@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <AM5PR0701MB265751E3A716081292C5224FE4450@AM5PR0701MB2657.eurprd07.prod.outlook.com>
Bernd Edlinger <bernd.edlinger@hotmail.de> writes:
> I was looking at usually suppressed gcc warnings,
> [-Wimplicit-fallthrough=] in this case:
>
> The code definitely looks like a break is missing here.
> However I am not able to test the NL80211_IFTYPE_MESH_POINT,
> nor do I actually know what might be :)
> So please use this patch with caution and only if you are
> able to do some testing.
>
> Signed-off-by: Bernd Edlinger <bernd.edlinger@hotmail.de>
> ---
> net/wireless/nl80211.c | 1 +
> 1 file changed, 1 insertion(+)
You should use prefix "nl80211: " in the title.
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches#subject
--
Kalle Valo
^ permalink raw reply
* [PATCH 2/2 net-next v3] net: bridge: add support for backup port
From: Nikolay Aleksandrov @ 2018-07-23 8:16 UTC (permalink / raw)
To: netdev
Cc: roopa, davem, anuradhak, wkok, bridge, stephen, makita.toshiaki,
Nikolay Aleksandrov
In-Reply-To: <20180723081659.21986-1-nikolay@cumulusnetworks.com>
This patch adds a new port attribute - IFLA_BRPORT_BACKUP_PORT, which
allows to set a backup port to be used for known unicast traffic if the
port has gone carrier down. The backup pointer is rcu protected and set
only under RTNL, a counter is maintained so when deleting a port we know
how many other ports reference it as a backup and we remove it from all.
Also the pointer is in the first cache line which is hot at the time of
the check and thus in the common case we only add one more test.
The backup port will be used only for the non-flooding case since
it's a part of the bridge and the flooded packets will be forwarded to it
anyway. To remove the forwarding just send a 0/non-existing backup port.
This is used to avoid numerous scalability problems when using MLAG most
notably if we have thousands of fdbs one would need to change all of them
on port carrier going down which takes too long and causes a storm of fdb
notifications (and again when the port comes back up). In a Multi-chassis
Link Aggregation setup usually hosts are connected to two different
switches which act as a single logical switch. Those switches usually have
a control and backup link between them called peerlink which might be used
for communication in case a host loses connectivity to one of them.
We need a fast way to failover in case a host port goes down and currently
none of the solutions (like bond) cannot fulfill the requirements because
the participating ports are actually the "master" devices and must have the
same peerlink as their backup interface and at the same time all of them
must participate in the bridge device. As Roopa noted it's normal practice
in routing called fast re-route where a precalculated backup path is used
when the main one is down.
Another use case of this is with EVPN, having a single vxlan device which
is backup of every port. Due to the nature of master devices it's not
currently possible to use one device as a backup for many and still have
all of them participate in the bridge (which is master itself).
More detailed information about MLAG is available at the link below.
https://docs.cumulusnetworks.com/display/DOCS/Multi-Chassis+Link+Aggregation+-+MLAG
Further explanation and a diagram by Roopa:
Two switches acting in a MLAG pair are connected by the peerlink
interface which is a bridge port.
the config on one of the switches looks like the below. The other
switch also has a similar config.
eth0 is connected to one port on the server. And the server is
connected to both switches.
br0 -- team0---eth0
|
-- switch-peerlink
Signed-off-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
---
v3: No code changes, added Roopa's explanation and diagram
include/uapi/linux/if_link.h | 1 +
net/bridge/br_forward.c | 16 ++++++++++++-
net/bridge/br_if.c | 53 ++++++++++++++++++++++++++++++++++++++++++++
net/bridge/br_netlink.c | 30 ++++++++++++++++++++++++-
net/bridge/br_private.h | 3 +++
net/bridge/br_sysfs_if.c | 33 +++++++++++++++++++++++++++
6 files changed, 134 insertions(+), 2 deletions(-)
diff --git a/include/uapi/linux/if_link.h b/include/uapi/linux/if_link.h
index 8759cfb8aa2e..01b5069a73a5 100644
--- a/include/uapi/linux/if_link.h
+++ b/include/uapi/linux/if_link.h
@@ -334,6 +334,7 @@ enum {
IFLA_BRPORT_GROUP_FWD_MASK,
IFLA_BRPORT_NEIGH_SUPPRESS,
IFLA_BRPORT_ISOLATED,
+ IFLA_BRPORT_BACKUP_PORT,
__IFLA_BRPORT_MAX
};
#define IFLA_BRPORT_MAX (__IFLA_BRPORT_MAX - 1)
diff --git a/net/bridge/br_forward.c b/net/bridge/br_forward.c
index 9019f326fe81..5372e2042adf 100644
--- a/net/bridge/br_forward.c
+++ b/net/bridge/br_forward.c
@@ -142,7 +142,20 @@ static int deliver_clone(const struct net_bridge_port *prev,
void br_forward(const struct net_bridge_port *to,
struct sk_buff *skb, bool local_rcv, bool local_orig)
{
- if (to && should_deliver(to, skb)) {
+ if (unlikely(!to))
+ goto out;
+
+ /* redirect to backup link if the destination port is down */
+ if (rcu_access_pointer(to->backup_port) && !netif_carrier_ok(to->dev)) {
+ struct net_bridge_port *backup_port;
+
+ backup_port = rcu_dereference(to->backup_port);
+ if (unlikely(!backup_port))
+ goto out;
+ to = backup_port;
+ }
+
+ if (should_deliver(to, skb)) {
if (local_rcv)
deliver_clone(to, skb, local_orig);
else
@@ -150,6 +163,7 @@ void br_forward(const struct net_bridge_port *to,
return;
}
+out:
if (!local_rcv)
kfree_skb(skb);
}
diff --git a/net/bridge/br_if.c b/net/bridge/br_if.c
index e7c8d55212aa..0363f1bdc401 100644
--- a/net/bridge/br_if.c
+++ b/net/bridge/br_if.c
@@ -170,6 +170,58 @@ void br_manage_promisc(struct net_bridge *br)
}
}
+int nbp_backup_change(struct net_bridge_port *p,
+ struct net_device *backup_dev)
+{
+ struct net_bridge_port *old_backup = rtnl_dereference(p->backup_port);
+ struct net_bridge_port *backup_p = NULL;
+
+ ASSERT_RTNL();
+
+ if (backup_dev) {
+ if (!br_port_exists(backup_dev))
+ return -ENOENT;
+
+ backup_p = br_port_get_rtnl(backup_dev);
+ if (backup_p->br != p->br)
+ return -EINVAL;
+ }
+
+ if (p == backup_p)
+ return -EINVAL;
+
+ if (old_backup == backup_p)
+ return 0;
+
+ /* if the backup link is already set, clear it */
+ if (old_backup)
+ old_backup->backup_redirected_cnt--;
+
+ if (backup_p)
+ backup_p->backup_redirected_cnt++;
+ rcu_assign_pointer(p->backup_port, backup_p);
+
+ return 0;
+}
+
+static void nbp_backup_clear(struct net_bridge_port *p)
+{
+ nbp_backup_change(p, NULL);
+ if (p->backup_redirected_cnt) {
+ struct net_bridge_port *cur_p;
+
+ list_for_each_entry(cur_p, &p->br->port_list, list) {
+ struct net_bridge_port *backup_p;
+
+ backup_p = rtnl_dereference(cur_p->backup_port);
+ if (backup_p == p)
+ nbp_backup_change(cur_p, NULL);
+ }
+ }
+
+ WARN_ON(rcu_access_pointer(p->backup_port) || p->backup_redirected_cnt);
+}
+
static void nbp_update_port_count(struct net_bridge *br)
{
struct net_bridge_port *p;
@@ -295,6 +347,7 @@ static void del_nbp(struct net_bridge_port *p)
nbp_vlan_flush(p);
br_fdb_delete_by_port(br, p, 0, 1);
switchdev_deferred_process();
+ nbp_backup_clear(p);
nbp_update_port_count(br);
diff --git a/net/bridge/br_netlink.c b/net/bridge/br_netlink.c
index 9f5eb05b0373..ec2b58a09f76 100644
--- a/net/bridge/br_netlink.c
+++ b/net/bridge/br_netlink.c
@@ -169,13 +169,15 @@ static inline size_t br_nlmsg_size(struct net_device *dev, u32 filter_mask)
+ nla_total_size(1) /* IFLA_OPERSTATE */
+ nla_total_size(br_port_info_size()) /* IFLA_PROTINFO */
+ nla_total_size(br_get_link_af_size_filtered(dev,
- filter_mask)); /* IFLA_AF_SPEC */
+ filter_mask)) /* IFLA_AF_SPEC */
+ + nla_total_size(4); /* IFLA_BRPORT_BACKUP_PORT */
}
static int br_port_fill_attrs(struct sk_buff *skb,
const struct net_bridge_port *p)
{
u8 mode = !!(p->flags & BR_HAIRPIN_MODE);
+ struct net_bridge_port *backup_p;
u64 timerval;
if (nla_put_u8(skb, IFLA_BRPORT_STATE, p->state) ||
@@ -237,6 +239,14 @@ static int br_port_fill_attrs(struct sk_buff *skb,
return -EMSGSIZE;
#endif
+ /* we might be called only with br->lock */
+ rcu_read_lock();
+ backup_p = rcu_dereference(p->backup_port);
+ if (backup_p)
+ nla_put_u32(skb, IFLA_BRPORT_BACKUP_PORT,
+ backup_p->dev->ifindex);
+ rcu_read_unlock();
+
return 0;
}
@@ -663,6 +673,7 @@ static const struct nla_policy br_port_policy[IFLA_BRPORT_MAX + 1] = {
[IFLA_BRPORT_GROUP_FWD_MASK] = { .type = NLA_U16 },
[IFLA_BRPORT_NEIGH_SUPPRESS] = { .type = NLA_U8 },
[IFLA_BRPORT_ISOLATED] = { .type = NLA_U8 },
+ [IFLA_BRPORT_BACKUP_PORT] = { .type = NLA_U32 },
};
/* Change the state of the port and notify spanning tree */
@@ -817,6 +828,23 @@ static int br_setport(struct net_bridge_port *p, struct nlattr *tb[])
if (err)
return err;
+ if (tb[IFLA_BRPORT_BACKUP_PORT]) {
+ struct net_device *backup_dev = NULL;
+ u32 backup_ifindex;
+
+ backup_ifindex = nla_get_u32(tb[IFLA_BRPORT_BACKUP_PORT]);
+ if (backup_ifindex) {
+ backup_dev = __dev_get_by_index(dev_net(p->dev),
+ backup_ifindex);
+ if (!backup_dev)
+ return -ENOENT;
+ }
+
+ err = nbp_backup_change(p, backup_dev);
+ if (err)
+ return err;
+ }
+
br_port_flags_change(p, old_flags ^ p->flags);
return 0;
}
diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
index cf0005d2a4d0..11ed2029985f 100644
--- a/net/bridge/br_private.h
+++ b/net/bridge/br_private.h
@@ -237,6 +237,7 @@ struct net_bridge_port {
#ifdef CONFIG_BRIDGE_VLAN_FILTERING
struct net_bridge_vlan_group __rcu *vlgrp;
#endif
+ struct net_bridge_port __rcu *backup_port;
/* STP */
u8 priority;
@@ -281,6 +282,7 @@ struct net_bridge_port {
int offload_fwd_mark;
#endif
u16 group_fwd_mask;
+ u16 backup_redirected_cnt;
};
#define kobj_to_brport(obj) container_of(obj, struct net_bridge_port, kobj)
@@ -597,6 +599,7 @@ netdev_features_t br_features_recompute(struct net_bridge *br,
netdev_features_t features);
void br_port_flags_change(struct net_bridge_port *port, unsigned long mask);
void br_manage_promisc(struct net_bridge *br);
+int nbp_backup_change(struct net_bridge_port *p, struct net_device *backup_dev);
/* br_input.c */
int br_handle_frame_finish(struct net *net, struct sock *sk, struct sk_buff *skb);
diff --git a/net/bridge/br_sysfs_if.c b/net/bridge/br_sysfs_if.c
index 4ac940067754..7c87a2fe5248 100644
--- a/net/bridge/br_sysfs_if.c
+++ b/net/bridge/br_sysfs_if.c
@@ -191,6 +191,38 @@ static int store_group_fwd_mask(struct net_bridge_port *p,
static BRPORT_ATTR(group_fwd_mask, 0644, show_group_fwd_mask,
store_group_fwd_mask);
+static ssize_t show_backup_port(struct net_bridge_port *p, char *buf)
+{
+ struct net_bridge_port *backup_p;
+ int ret = 0;
+
+ rcu_read_lock();
+ backup_p = rcu_dereference(p->backup_port);
+ if (backup_p)
+ ret = sprintf(buf, "%s\n", backup_p->dev->name);
+ rcu_read_unlock();
+
+ return ret;
+}
+
+static int store_backup_port(struct net_bridge_port *p, char *buf)
+{
+ struct net_device *backup_dev = NULL;
+ char *nl = strchr(buf, '\n');
+
+ if (nl)
+ *nl = '\0';
+
+ if (strlen(buf) > 0) {
+ backup_dev = __dev_get_by_name(dev_net(p->dev), buf);
+ if (!backup_dev)
+ return -ENOENT;
+ }
+
+ return nbp_backup_change(p, backup_dev);
+}
+static BRPORT_ATTR_RAW(backup_port, 0644, show_backup_port, store_backup_port);
+
BRPORT_ATTR_FLAG(hairpin_mode, BR_HAIRPIN_MODE);
BRPORT_ATTR_FLAG(bpdu_guard, BR_BPDU_GUARD);
BRPORT_ATTR_FLAG(root_block, BR_ROOT_BLOCK);
@@ -254,6 +286,7 @@ static const struct brport_attribute *brport_attrs[] = {
&brport_attr_group_fwd_mask,
&brport_attr_neigh_suppress,
&brport_attr_isolated,
+ &brport_attr_backup_port,
NULL
};
--
2.11.0
^ permalink raw reply related
* [PATCH 1/2 net-next v3] net: bridge: add support for raw sysfs port options
From: Nikolay Aleksandrov @ 2018-07-23 8:16 UTC (permalink / raw)
To: netdev
Cc: roopa, davem, anuradhak, wkok, bridge, stephen, makita.toshiaki,
Nikolay Aleksandrov
In-Reply-To: <20180723081659.21986-1-nikolay@cumulusnetworks.com>
This patch adds a new alternative store callback for port sysfs options
which takes a raw value (buf) and can use it directly. It is needed for the
backup port sysfs support since we have to pass the device by its name.
Signed-off-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
---
There are a few checkpatch warnings here because of the old code, I've
noted them in my todo and will send separate patch for simple_strtoul
and the function prototypes.
v3: no change
v2: Use kstrndup/kfree instead of casting the const buf. In order
to avoid using GFP_ATOMIC or always allocating I kept the spinlock inside
each branch.
net/bridge/br_sysfs_if.c | 56 ++++++++++++++++++++++++++++++++++++------------
1 file changed, 42 insertions(+), 14 deletions(-)
diff --git a/net/bridge/br_sysfs_if.c b/net/bridge/br_sysfs_if.c
index ab4c7f8adf68..4ac940067754 100644
--- a/net/bridge/br_sysfs_if.c
+++ b/net/bridge/br_sysfs_if.c
@@ -25,6 +25,15 @@ struct brport_attribute {
struct attribute attr;
ssize_t (*show)(struct net_bridge_port *, char *);
int (*store)(struct net_bridge_port *, unsigned long);
+ int (*store_raw)(struct net_bridge_port *, char *);
+};
+
+#define BRPORT_ATTR_RAW(_name, _mode, _show, _store) \
+const struct brport_attribute brport_attr_##_name = { \
+ .attr = {.name = __stringify(_name), \
+ .mode = _mode }, \
+ .show = _show, \
+ .store_raw = _store, \
};
#define BRPORT_ATTR(_name, _mode, _show, _store) \
@@ -269,27 +278,46 @@ static ssize_t brport_store(struct kobject *kobj,
struct brport_attribute *brport_attr = to_brport_attr(attr);
struct net_bridge_port *p = kobj_to_brport(kobj);
ssize_t ret = -EINVAL;
- char *endp;
unsigned long val;
+ char *endp;
if (!ns_capable(dev_net(p->dev)->user_ns, CAP_NET_ADMIN))
return -EPERM;
- val = simple_strtoul(buf, &endp, 0);
- if (endp != buf) {
- if (!rtnl_trylock())
- return restart_syscall();
- if (p->dev && p->br && brport_attr->store) {
- spin_lock_bh(&p->br->lock);
- ret = brport_attr->store(p, val);
- spin_unlock_bh(&p->br->lock);
- if (!ret) {
- br_ifinfo_notify(RTM_NEWLINK, NULL, p);
- ret = count;
- }
+ if (!rtnl_trylock())
+ return restart_syscall();
+
+ if (!p->dev || !p->br)
+ goto out_unlock;
+
+ if (brport_attr->store_raw) {
+ char *buf_copy;
+
+ buf_copy = kstrndup(buf, count, GFP_KERNEL);
+ if (!buf_copy) {
+ ret = -ENOMEM;
+ goto out_unlock;
}
- rtnl_unlock();
+ spin_lock_bh(&p->br->lock);
+ ret = brport_attr->store_raw(p, buf_copy);
+ spin_unlock_bh(&p->br->lock);
+ kfree(buf_copy);
+ } else if (brport_attr->store) {
+ val = simple_strtoul(buf, &endp, 0);
+ if (endp == buf)
+ goto out_unlock;
+ spin_lock_bh(&p->br->lock);
+ ret = brport_attr->store(p, val);
+ spin_unlock_bh(&p->br->lock);
}
+
+ if (!ret) {
+ br_ifinfo_notify(RTM_NEWLINK, NULL, p);
+ ret = count;
+ }
+out_unlock:
+ rtnl_unlock();
+
return ret;
}
--
2.11.0
^ permalink raw reply related
* [PATCH net-next v3 0/2] net: bridge: add support for backup port
From: Nikolay Aleksandrov @ 2018-07-23 8:16 UTC (permalink / raw)
To: netdev
Cc: roopa, davem, anuradhak, wkok, bridge, stephen, makita.toshiaki,
Nikolay Aleksandrov
Hi,
This set introduces a new bridge port option that allows any port to have
any other port (in the same bridge of course) as its backup and traffic
will be forwarded to the backup port when the primary goes down. This is
mainly used in MLAG and EVPN setups where we have peerlink path which is
a backup of many (or even all) ports and is a participating bridge port
itself. There's more detailed information in patch 02. Patch 01 just
prepares the port sysfs code for options that take raw value. The main
issues that this set solves are scalability and fallback latency.
We have used similar code for over 6 months now to bring the fallback
latency of the backup peerlink down and avoid fdb notification storms.
Also due to the nature of master devices such setup is currently not
possible, and last but not least having tens of thousands of fdbs require
thousands of calls to switch.
I've also CCed our MLAG experts that have been using similar option.
Roopa also adds:
"Two switches acting in a MLAG pair are connected by the peerlink
interface which is a bridge port.
the config on one of the switches looks like the below. The other
switch also has a similar config.
eth0 is connected to one port on the server. And the server is
connected to both switches.
br0 -- team0---eth0
|
-- switch-peerlink
switch-peerlink becomes the failover/backport port when say team0 to
the server goes down.
Today, when team0 goes down, control plane has to withdraw all the fdb
entries pointing to team0
and re-install the fdb entries pointing to switch-peerlink...and
restore the fdb entries when team0 comes back up again.
and this is the problem we are trying to solve.
This also becomes necessary when multihoming is implemented by a
standard like E-VPN https://tools.ietf.org/html/rfc8365#section-8
where the 'switch-peerlink' is an overlay vxlan port (like nikolay
mentions in his patch commit). In these implementations, the fdb scale
can be much larger.
On why bond failover cannot be used here ?: the point that nikolay was
alluding to is, switch-peerlink in the above example is a bridge port
and is a failover/backport port for more than one or all ports in the
bridge br0. And you cannot enslave switch-peerlink into a second level
team
with other bridge ports. Hence a multi layered team device is not an
option (FWIW, switch-peerlink is also a teamed interface to the peer
switch)."
v3: Added Roopa's explanation and diagram
v2: In patch 01 use kstrdup/kfree to avoid casting the const buf. In order
to avoid using GFP_ATOMIC or always allocating I kept the spinlock inside
each branch.
Thanks,
Nik
Nikolay Aleksandrov (2):
net: bridge: add support for raw sysfs port options
net: bridge: add support for backup port
include/uapi/linux/if_link.h | 1 +
net/bridge/br_forward.c | 16 +++++++-
net/bridge/br_if.c | 53 ++++++++++++++++++++++++++
net/bridge/br_netlink.c | 30 ++++++++++++++-
net/bridge/br_private.h | 3 ++
net/bridge/br_sysfs_if.c | 89 +++++++++++++++++++++++++++++++++++++-------
6 files changed, 176 insertions(+), 16 deletions(-)
--
2.11.0
^ permalink raw reply
* [PATCH] net: axienet: Fix double deregister of mdio
From: Shubhrajyoti Datta @ 2018-07-23 8:38 UTC (permalink / raw)
To: netdev; +Cc: michal.simek, linux-kernel, Shubhrajyoti Datta
If the registration fails then mdio_unregister is called.
However at unbind the unregister ia attempted again resulting
in the below crash
[ 73.544038] kernel BUG at drivers/net/phy/mdio_bus.c:415!
[ 73.549362] Internal error: Oops - BUG: 0 [#1] SMP
[ 73.554127] Modules linked in:
[ 73.557168] CPU: 0 PID: 2249 Comm: sh Not tainted 4.14.0 #183
[ 73.562895] Hardware name: xlnx,zynqmp (DT)
[ 73.567062] task: ffffffc879e41180 task.stack: ffffff800cbe0000
[ 73.572973] PC is at mdiobus_unregister+0x84/0x88
[ 73.577656] LR is at axienet_mdio_teardown+0x18/0x30
[ 73.582601] pc : [<ffffff80085fa4cc>] lr : [<ffffff8008616858>]
pstate: 20000145
[ 73.589981] sp : ffffff800cbe3c30
[ 73.593277] x29: ffffff800cbe3c30 x28: ffffffc879e41180
[ 73.598573] x27: ffffff8008a21000 x26: 0000000000000040
[ 73.603868] x25: 0000000000000124 x24: ffffffc879efe920
[ 73.609164] x23: 0000000000000060 x22: ffffffc879e02000
[ 73.614459] x21: ffffffc879e02800 x20: ffffffc87b0b8870
[ 73.619754] x19: ffffffc879e02800 x18: 000000000000025d
[ 73.625050] x17: 0000007f9a719ad0 x16: ffffff8008195bd8
[ 73.630345] x15: 0000007f9a6b3d00 x14: 0000000000000010
[ 73.635640] x13: 74656e7265687465 x12: 0000000000000030
[ 73.640935] x11: 0000000000000030 x10: 0101010101010101
[ 73.646231] x9 : 241f394f42533300 x8 : ffffffc8799f6e98
[ 73.651526] x7 : ffffffc8799f6f18 x6 : ffffffc87b0ba318
[ 73.656822] x5 : ffffffc87b0ba498 x4 : 0000000000000000
[ 73.662117] x3 : 0000000000000000 x2 : 0000000000000008
[ 73.667412] x1 : 0000000000000004 x0 : ffffffc8799f4000
[ 73.672708] Process sh (pid: 2249, stack limit = 0xffffff800cbe0000)
Fix the same by making the bus NULL on unregister.
Signed-off-by: Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com>
---
drivers/net/ethernet/xilinx/xilinx_axienet_mdio.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/net/ethernet/xilinx/xilinx_axienet_mdio.c b/drivers/net/ethernet/xilinx/xilinx_axienet_mdio.c
index 16c3bfb..757a3b3 100644
--- a/drivers/net/ethernet/xilinx/xilinx_axienet_mdio.c
+++ b/drivers/net/ethernet/xilinx/xilinx_axienet_mdio.c
@@ -218,6 +218,7 @@ int axienet_mdio_setup(struct axienet_local *lp, struct device_node *np)
ret = of_mdiobus_register(bus, np1);
if (ret) {
mdiobus_free(bus);
+ lp->mii_bus = NULL;
return ret;
}
return 0;
--
2.7.4
This email and any attachments are intended for the sole use of the named recipient(s) and contain(s) confidential information that may be proprietary, privileged or copyrighted under applicable law. If you are not the intended recipient, do not read, copy, or forward this email message or any attachments. Delete this email message and any attachments immediately.
^ permalink raw reply related
* [PATCH net-next] ipv6: sr: Use kmemdup instead of duplicating it in parse_nla_srh
From: YueHaibing @ 2018-07-23 8:33 UTC (permalink / raw)
To: davem, kuznet, yoshfuji; +Cc: linux-kernel, netdev, YueHaibing
Replace calls to kmalloc followed by a memcpy with a direct call to
kmemdup.
Signed-off-by: YueHaibing <yuehaibing@huawei.com>
---
net/ipv6/seg6_local.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/net/ipv6/seg6_local.c b/net/ipv6/seg6_local.c
index cd6e4ca..e1025b4 100644
--- a/net/ipv6/seg6_local.c
+++ b/net/ipv6/seg6_local.c
@@ -637,12 +637,10 @@ static int parse_nla_srh(struct nlattr **attrs, struct seg6_local_lwt *slwt)
if (!seg6_validate_srh(srh, len))
return -EINVAL;
- slwt->srh = kmalloc(len, GFP_KERNEL);
+ slwt->srh = kmemdup(srh, len, GFP_KERNEL);
if (!slwt->srh)
return -ENOMEM;
- memcpy(slwt->srh, srh, len);
-
slwt->headroom += len;
return 0;
--
2.7.0
^ permalink raw reply related
* [patch iproute2/net-next v4] tc: introduce support for chain templates
From: Jiri Pirko @ 2018-07-23 7:24 UTC (permalink / raw)
To: netdev
Cc: davem, jhs, xiyou.wangcong, jakub.kicinski, simon.horman,
john.hurley, dsahern, mlxsw, sridhar.samudrala
In-Reply-To: <20180723072312.4153-1-jiri@resnulli.us>
From: Jiri Pirko <jiri@mellanox.com>
Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
v3->v4:
- reworked to chain object
v1->v2:
- moved the template handling
from "tc filter template" to "tc chaintemplate"
---
include/uapi/linux/rtnetlink.h | 7 +++
man/man8/tc.8 | 26 ++++++++
tc/tc.c | 5 +-
tc/tc_common.h | 1 +
tc/tc_filter.c | 131 +++++++++++++++++++++++++++++------------
tc/tc_monitor.c | 5 +-
6 files changed, 135 insertions(+), 40 deletions(-)
diff --git a/include/uapi/linux/rtnetlink.h b/include/uapi/linux/rtnetlink.h
index c3a7d8ecc7b9..8c1d600bfa33 100644
--- a/include/uapi/linux/rtnetlink.h
+++ b/include/uapi/linux/rtnetlink.h
@@ -150,6 +150,13 @@ enum {
RTM_NEWCACHEREPORT = 96,
#define RTM_NEWCACHEREPORT RTM_NEWCACHEREPORT
+ RTM_NEWCHAIN = 100,
+#define RTM_NEWCHAIN RTM_NEWCHAIN
+ RTM_DELCHAIN,
+#define RTM_DELCHAIN RTM_DELCHAIN
+ RTM_GETCHAIN,
+#define RTM_GETCHAIN RTM_GETCHAIN
+
__RTM_MAX,
#define RTM_MAX (((__RTM_MAX + 3) & ~3) - 1)
};
diff --git a/man/man8/tc.8 b/man/man8/tc.8
index 716dfec5f3a8..fd33f9b2f180 100644
--- a/man/man8/tc.8
+++ b/man/man8/tc.8
@@ -58,6 +58,22 @@ tc \- show / manipulate traffic control settings
.B flowid
\fIflow-id\fR
+.B tc
+.RI "[ " OPTIONS " ]"
+.B chain [ add | delete | get ] dev
+\fIDEV\fR
+.B [ parent
+\fIqdisc-id\fR
+.B | root ]\fR filtertype
+[ filtertype specific parameters ]
+
+.B tc
+.RI "[ " OPTIONS " ]"
+.B chain [ add | delete | get ] block
+\fIBLOCK_INDEX\fR filtertype
+[ filtertype specific parameters ]
+
+
.B tc
.RI "[ " OPTIONS " ]"
.RI "[ " FORMAT " ]"
@@ -80,6 +96,16 @@ tc \- show / manipulate traffic control settings
.RI "[ " OPTIONS " ]"
.B filter show block
\fIBLOCK_INDEX\fR
+.P
+.B tc
+.RI "[ " OPTIONS " ]"
+.B chain show dev
+\fIDEV\fR
+.P
+.B tc
+.RI "[ " OPTIONS " ]"
+.B chain show block
+\fIBLOCK_INDEX\fR
.P
.B tc
diff --git a/tc/tc.c b/tc/tc.c
index 0d223281ba25..2af67963c1e1 100644
--- a/tc/tc.c
+++ b/tc/tc.c
@@ -197,7 +197,8 @@ static void usage(void)
fprintf(stderr,
"Usage: tc [ OPTIONS ] OBJECT { COMMAND | help }\n"
" tc [-force] -batch filename\n"
- "where OBJECT := { qdisc | class | filter | action | monitor | exec }\n"
+ "where OBJECT := { qdisc | class | filter | chain |\n"
+ " action | monitor | exec }\n"
" OPTIONS := { -V[ersion] | -s[tatistics] | -d[etails] | -r[aw] |\n"
" -o[neline] | -j[son] | -p[retty] | -c[olor]\n"
" -b[atch] [filename] | -n[etns] name |\n"
@@ -212,6 +213,8 @@ static int do_cmd(int argc, char **argv, void *buf, size_t buflen)
return do_class(argc-1, argv+1);
if (matches(*argv, "filter") == 0)
return do_filter(argc-1, argv+1, buf, buflen);
+ if (matches(*argv, "chain") == 0)
+ return do_chain(argc-1, argv+1, buf, buflen);
if (matches(*argv, "actions") == 0)
return do_action(argc-1, argv+1, buf, buflen);
if (matches(*argv, "monitor") == 0)
diff --git a/tc/tc_common.h b/tc/tc_common.h
index 49c24616c2c3..272d1727027d 100644
--- a/tc/tc_common.h
+++ b/tc/tc_common.h
@@ -8,6 +8,7 @@ extern struct rtnl_handle rth;
extern int do_qdisc(int argc, char **argv);
extern int do_class(int argc, char **argv);
extern int do_filter(int argc, char **argv, void *buf, size_t buflen);
+extern int do_chain(int argc, char **argv, void *buf, size_t buflen);
extern int do_action(int argc, char **argv, void *buf, size_t buflen);
extern int do_tcmonitor(int argc, char **argv);
extern int do_exec(int argc, char **argv);
diff --git a/tc/tc_filter.c b/tc/tc_filter.c
index c5bb0bffe19b..15044b4bc6ed 100644
--- a/tc/tc_filter.c
+++ b/tc/tc_filter.c
@@ -45,6 +45,13 @@ static void usage(void)
"OPTIONS := ... try tc filter add <desired FILTER_KIND> help\n");
}
+static void chain_usage(void)
+{
+ fprintf(stderr,
+ "Usage: tc chain [ add | del | get | show ] [ dev STRING ]\n"
+ " tc chain [ add | del | get | show ] [ block BLOCK_INDEX ] ]\n");
+}
+
struct tc_filter_req {
struct nlmsghdr n;
struct tcmsg t;
@@ -85,7 +92,8 @@ static int tc_filter_modify(int cmd, unsigned int flags, int argc, char **argv,
req->n.nlmsg_type = cmd;
req->t.tcm_family = AF_UNSPEC;
- if (cmd == RTM_NEWTFILTER && flags & NLM_F_CREATE)
+ if ((cmd == RTM_NEWTFILTER || cmd == RTM_NEWCHAIN) &&
+ flags & NLM_F_CREATE)
protocol = htons(ETH_P_ALL);
while (argc > 0) {
@@ -261,7 +269,10 @@ int print_filter(const struct sockaddr_nl *who, struct nlmsghdr *n, void *arg)
if (n->nlmsg_type != RTM_NEWTFILTER &&
n->nlmsg_type != RTM_GETTFILTER &&
- n->nlmsg_type != RTM_DELTFILTER) {
+ n->nlmsg_type != RTM_DELTFILTER &&
+ n->nlmsg_type != RTM_NEWCHAIN &&
+ n->nlmsg_type != RTM_GETCHAIN &&
+ n->nlmsg_type != RTM_DELCHAIN) {
fprintf(stderr, "Not a filter(cmd %d)\n", n->nlmsg_type);
return 0;
}
@@ -273,27 +284,36 @@ int print_filter(const struct sockaddr_nl *who, struct nlmsghdr *n, void *arg)
parse_rtattr(tb, TCA_MAX, TCA_RTA(t), len);
- if (tb[TCA_KIND] == NULL) {
+ if (tb[TCA_KIND] == NULL && (n->nlmsg_type == RTM_NEWTFILTER ||
+ n->nlmsg_type == RTM_GETTFILTER ||
+ n->nlmsg_type == RTM_DELTFILTER)) {
fprintf(stderr, "print_filter: NULL kind\n");
return -1;
}
open_json_object(NULL);
- if (n->nlmsg_type == RTM_DELTFILTER)
+ if (n->nlmsg_type == RTM_DELTFILTER || n->nlmsg_type == RTM_DELCHAIN)
print_bool(PRINT_ANY, "deleted", "deleted ", true);
- if (n->nlmsg_type == RTM_NEWTFILTER &&
+ if ((n->nlmsg_type == RTM_NEWTFILTER ||
+ n->nlmsg_type == RTM_NEWCHAIN) &&
(n->nlmsg_flags & NLM_F_CREATE) &&
!(n->nlmsg_flags & NLM_F_EXCL))
print_bool(PRINT_ANY, "replaced", "replaced ", true);
- if (n->nlmsg_type == RTM_NEWTFILTER &&
+ if ((n->nlmsg_type == RTM_NEWTFILTER ||
+ n->nlmsg_type == RTM_NEWCHAIN) &&
(n->nlmsg_flags & NLM_F_CREATE) &&
(n->nlmsg_flags & NLM_F_EXCL))
print_bool(PRINT_ANY, "added", "added ", true);
- print_string(PRINT_FP, NULL, "filter ", NULL);
+ if (n->nlmsg_type == RTM_NEWTFILTER ||
+ n->nlmsg_type == RTM_GETTFILTER ||
+ n->nlmsg_type == RTM_DELTFILTER)
+ print_string(PRINT_FP, NULL, "filter ", NULL);
+ else
+ print_string(PRINT_FP, NULL, "chain ", NULL);
if (t->tcm_ifindex == TCM_IFINDEX_MAGIC_BLOCK) {
if (!filter_block_index ||
filter_block_index != t->tcm_block_index)
@@ -317,7 +337,9 @@ int print_filter(const struct sockaddr_nl *who, struct nlmsghdr *n, void *arg)
}
}
- if (t->tcm_info) {
+ if (t->tcm_info && (n->nlmsg_type == RTM_NEWTFILTER ||
+ n->nlmsg_type == RTM_DELTFILTER ||
+ n->nlmsg_type == RTM_GETTFILTER)) {
f_proto = TC_H_MIN(t->tcm_info);
__u32 prio = TC_H_MAJ(t->tcm_info)>>16;
@@ -334,7 +356,8 @@ int print_filter(const struct sockaddr_nl *who, struct nlmsghdr *n, void *arg)
print_uint(PRINT_ANY, "pref", "pref %u ", prio);
}
}
- print_string(PRINT_ANY, "kind", "%s ", rta_getattr_str(tb[TCA_KIND]));
+ if (tb[TCA_KIND])
+ print_string(PRINT_ANY, "kind", "%s ", rta_getattr_str(tb[TCA_KIND]));
if (tb[TCA_CHAIN]) {
__u32 chain_index = rta_getattr_u32(tb[TCA_CHAIN]);
@@ -345,15 +368,17 @@ int print_filter(const struct sockaddr_nl *who, struct nlmsghdr *n, void *arg)
chain_index);
}
- q = get_filter_kind(RTA_DATA(tb[TCA_KIND]));
- if (tb[TCA_OPTIONS]) {
- open_json_object("options");
- if (q)
- q->print_fopt(q, fp, tb[TCA_OPTIONS], t->tcm_handle);
- else
- print_string(PRINT_FP, NULL,
- "[cannot parse parameters]", NULL);
- close_json_object();
+ if (tb[TCA_KIND]) {
+ q = get_filter_kind(RTA_DATA(tb[TCA_KIND]));
+ if (tb[TCA_OPTIONS]) {
+ open_json_object("options");
+ if (q)
+ q->print_fopt(q, fp, tb[TCA_OPTIONS], t->tcm_handle);
+ else
+ print_string(PRINT_FP, NULL,
+ "[cannot parse parameters]", NULL);
+ close_json_object();
+ }
}
print_string(PRINT_FP, NULL, "\n", NULL);
@@ -496,17 +521,19 @@ static int tc_filter_get(int cmd, unsigned int flags, int argc, char **argv)
argc--; argv++;
}
- if (!protocol_set) {
- fprintf(stderr, "Must specify filter protocol\n");
- return -1;
- }
+ if (cmd == RTM_GETTFILTER) {
+ if (!protocol_set) {
+ fprintf(stderr, "Must specify filter protocol\n");
+ return -1;
+ }
- if (!prio) {
- fprintf(stderr, "Must specify filter priority\n");
- return -1;
- }
+ if (!prio) {
+ fprintf(stderr, "Must specify filter priority\n");
+ return -1;
+ }
- req.t.tcm_info = TC_H_MAKE(prio<<16, protocol);
+ req.t.tcm_info = TC_H_MAKE(prio<<16, protocol);
+ }
if (chain_index_set)
addattr32(&req.n, sizeof(req), TCA_CHAIN, chain_index);
@@ -516,11 +543,13 @@ static int tc_filter_get(int cmd, unsigned int flags, int argc, char **argv)
return -1;
}
- if (k[0])
- addattr_l(&req.n, sizeof(req), TCA_KIND, k, strlen(k)+1);
- else {
- fprintf(stderr, "Must specify filter type\n");
- return -1;
+ if (cmd == RTM_GETTFILTER) {
+ if (k[0])
+ addattr_l(&req.n, sizeof(req), TCA_KIND, k, strlen(k)+1);
+ else {
+ fprintf(stderr, "Must specify filter type\n");
+ return -1;
+ }
}
if (d[0]) {
@@ -539,10 +568,11 @@ static int tc_filter_get(int cmd, unsigned int flags, int argc, char **argv)
return -1;
}
- if (q->parse_fopt(q, fhandle, argc, argv, &req.n))
+ if (cmd == RTM_GETTFILTER &&
+ q->parse_fopt(q, fhandle, argc, argv, &req.n))
return 1;
- if (!fhandle) {
+ if (!fhandle && cmd == RTM_GETTFILTER) {
fprintf(stderr, "Must specify filter \"handle\"\n");
return -1;
}
@@ -569,7 +599,7 @@ static int tc_filter_get(int cmd, unsigned int flags, int argc, char **argv)
return 0;
}
-static int tc_filter_list(int argc, char **argv)
+static int tc_filter_list(int cmd, int argc, char **argv)
{
struct {
struct nlmsghdr n;
@@ -577,7 +607,7 @@ static int tc_filter_list(int argc, char **argv)
char buf[MAX_MSG];
} req = {
.n.nlmsg_len = NLMSG_LENGTH(sizeof(struct tcmsg)),
- .n.nlmsg_type = RTM_GETTFILTER,
+ .n.nlmsg_type = cmd,
.t.tcm_parent = TC_H_UNSPEC,
.t.tcm_family = AF_UNSPEC,
};
@@ -725,7 +755,7 @@ static int tc_filter_list(int argc, char **argv)
int do_filter(int argc, char **argv, void *buf, size_t buflen)
{
if (argc < 1)
- return tc_filter_list(0, NULL);
+ return tc_filter_list(RTM_GETTFILTER, 0, NULL);
if (matches(*argv, "add") == 0)
return tc_filter_modify(RTM_NEWTFILTER, NLM_F_EXCL|NLM_F_CREATE,
argc-1, argv+1, buf, buflen);
@@ -742,7 +772,7 @@ int do_filter(int argc, char **argv, void *buf, size_t buflen)
return tc_filter_get(RTM_GETTFILTER, 0, argc-1, argv+1);
if (matches(*argv, "list") == 0 || matches(*argv, "show") == 0
|| matches(*argv, "lst") == 0)
- return tc_filter_list(argc-1, argv+1);
+ return tc_filter_list(RTM_GETTFILTER, argc-1, argv+1);
if (matches(*argv, "help") == 0) {
usage();
return 0;
@@ -751,3 +781,28 @@ int do_filter(int argc, char **argv, void *buf, size_t buflen)
*argv);
return -1;
}
+
+int do_chain(int argc, char **argv, void *buf, size_t buflen)
+{
+ if (argc < 1)
+ return tc_filter_list(RTM_GETCHAIN, 0, NULL);
+ if (matches(*argv, "add") == 0) {
+ return tc_filter_modify(RTM_NEWCHAIN, NLM_F_EXCL | NLM_F_CREATE,
+ argc - 1, argv + 1, buf, buflen);
+ } else if (matches(*argv, "delete") == 0) {
+ return tc_filter_modify(RTM_DELCHAIN, 0,
+ argc - 1, argv + 1, buf, buflen);
+ } else if (matches(*argv, "get") == 0) {
+ return tc_filter_get(RTM_GETCHAIN, 0,
+ argc - 1, argv + 1);
+ } else if (matches(*argv, "list") == 0 || matches(*argv, "show") == 0 ||
+ matches(*argv, "lst") == 0) {
+ return tc_filter_list(RTM_GETCHAIN, argc - 1, argv + 1);
+ } else if (matches(*argv, "help") == 0) {
+ chain_usage();
+ return 0;
+ }
+ fprintf(stderr, "Command \"%s\" is unknown, try \"tc chain help\".\n",
+ *argv);
+ return -1;
+}
diff --git a/tc/tc_monitor.c b/tc/tc_monitor.c
index 077b138d1ec5..1f1ee08fb9cf 100644
--- a/tc/tc_monitor.c
+++ b/tc/tc_monitor.c
@@ -43,7 +43,10 @@ static int accept_tcmsg(const struct sockaddr_nl *who,
if (timestamp)
print_timestamp(fp);
- if (n->nlmsg_type == RTM_NEWTFILTER || n->nlmsg_type == RTM_DELTFILTER) {
+ if (n->nlmsg_type == RTM_NEWTFILTER ||
+ n->nlmsg_type == RTM_DELTFILTER ||
+ n->nlmsg_type == RTM_NEWCHAIN ||
+ n->nlmsg_type == RTM_DELCHAIN) {
print_filter(who, n, arg);
return 0;
}
--
2.14.4
^ permalink raw reply related
* [patch net-next v4 12/12] selftests: forwarding: add tests for TC chain templates
From: Jiri Pirko @ 2018-07-23 7:24 UTC (permalink / raw)
To: netdev
Cc: davem, jhs, xiyou.wangcong, jakub.kicinski, simon.horman,
john.hurley, dsahern, mlxsw, sridhar.samudrala
In-Reply-To: <20180723072312.4153-1-jiri@resnulli.us>
From: Jiri Pirko <jiri@mellanox.com>
Add basic sanity tests for TC chain templates.
Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
v3->v4:
- split from the originally single patch
---
.../testing/selftests/net/forwarding/tc_chains.sh | 44 +++++++++++++++++++++-
1 file changed, 43 insertions(+), 1 deletion(-)
diff --git a/tools/testing/selftests/net/forwarding/tc_chains.sh b/tools/testing/selftests/net/forwarding/tc_chains.sh
index 2730887a3b3a..031e322e28b3 100755
--- a/tools/testing/selftests/net/forwarding/tc_chains.sh
+++ b/tools/testing/selftests/net/forwarding/tc_chains.sh
@@ -1,7 +1,8 @@
#!/bin/bash
# SPDX-License-Identifier: GPL-2.0
-ALL_TESTS="unreachable_chain_test gact_goto_chain_test create_destroy_chain"
+ALL_TESTS="unreachable_chain_test gact_goto_chain_test create_destroy_chain \
+ template_filter_fits"
NUM_NETIFS=2
source tc_common.sh
source lib.sh
@@ -99,6 +100,47 @@ create_destroy_chain()
log_test "create destroy chain"
}
+template_filter_fits()
+{
+ RET=0
+
+ tc chain add dev $h2 ingress protocol ip \
+ flower dst_mac 00:00:00:00:00:00/FF:FF:FF:FF:FF:FF &> /dev/null
+ tc chain add dev $h2 ingress chain 1 protocol ip \
+ flower src_mac 00:00:00:00:00:00/FF:FF:FF:FF:FF:FF &> /dev/null
+
+ tc filter add dev $h2 ingress protocol ip pref 1 handle 1101 \
+ flower dst_mac $h2mac action drop
+ check_err $? "Failed to insert filter which fits template"
+
+ tc filter add dev $h2 ingress protocol ip pref 1 handle 1102 \
+ flower src_mac $h2mac action drop &> /dev/null
+ check_fail $? "Incorrectly succeded to insert filter which does not template"
+
+ tc filter add dev $h2 ingress chain 1 protocol ip pref 1 handle 1101 \
+ flower src_mac $h2mac action drop
+ check_err $? "Failed to insert filter which fits template"
+
+ tc filter add dev $h2 ingress chain 1 protocol ip pref 1 handle 1102 \
+ flower dst_mac $h2mac action drop &> /dev/null
+ check_fail $? "Incorrectly succeded to insert filter which does not template"
+
+ tc filter del dev $h2 ingress chain 1 protocol ip pref 1 handle 1102 \
+ flower &> /dev/null
+ tc filter del dev $h2 ingress chain 1 protocol ip pref 1 handle 1101 \
+ flower &> /dev/null
+
+ tc filter del dev $h2 ingress protocol ip pref 1 handle 1102 \
+ flower &> /dev/null
+ tc filter del dev $h2 ingress protocol ip pref 1 handle 1101 \
+ flower &> /dev/null
+
+ tc chain del dev $h2 ingress chain 1
+ tc chain del dev $h2 ingress
+
+ log_test "template filter fits"
+}
+
setup_prepare()
{
h1=${NETIFS[p1]}
--
2.14.4
^ permalink raw reply related
* [patch net-next v4 11/12] selftests: forwarding: add tests for TC chains creation adn destruction
From: Jiri Pirko @ 2018-07-23 7:24 UTC (permalink / raw)
To: netdev
Cc: davem, jhs, xiyou.wangcong, jakub.kicinski, simon.horman,
john.hurley, dsahern, mlxsw, sridhar.samudrala
In-Reply-To: <20180723072312.4153-1-jiri@resnulli.us>
From: Jiri Pirko <jiri@mellanox.com>
Add basic sanity tests for TC chains.
Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
v3->v4:
- split from the originally single patch
---
tools/testing/selftests/net/forwarding/lib.sh | 9 +++++++++
.../testing/selftests/net/forwarding/tc_chains.sh | 23 +++++++++++++++++++++-
2 files changed, 31 insertions(+), 1 deletion(-)
diff --git a/tools/testing/selftests/net/forwarding/lib.sh b/tools/testing/selftests/net/forwarding/lib.sh
index ec94395a54a7..158d59ffee40 100644
--- a/tools/testing/selftests/net/forwarding/lib.sh
+++ b/tools/testing/selftests/net/forwarding/lib.sh
@@ -44,6 +44,15 @@ check_tc_shblock_support()
fi
}
+check_tc_chain_support()
+{
+ tc help 2>&1|grep chain &> /dev/null
+ if [[ $? -ne 0 ]]; then
+ echo "SKIP: iproute2 too old; tc is missing chain support"
+ exit 1
+ fi
+}
+
if [[ "$(id -u)" -ne 0 ]]; then
echo "SKIP: need root privileges"
exit 0
diff --git a/tools/testing/selftests/net/forwarding/tc_chains.sh b/tools/testing/selftests/net/forwarding/tc_chains.sh
index d2c783e94df3..2730887a3b3a 100755
--- a/tools/testing/selftests/net/forwarding/tc_chains.sh
+++ b/tools/testing/selftests/net/forwarding/tc_chains.sh
@@ -1,7 +1,7 @@
#!/bin/bash
# SPDX-License-Identifier: GPL-2.0
-ALL_TESTS="unreachable_chain_test gact_goto_chain_test"
+ALL_TESTS="unreachable_chain_test gact_goto_chain_test create_destroy_chain"
NUM_NETIFS=2
source tc_common.sh
source lib.sh
@@ -80,6 +80,25 @@ gact_goto_chain_test()
log_test "gact goto chain ($tcflags)"
}
+create_destroy_chain()
+{
+ RET=0
+
+ tc chain add dev $h2 ingress
+ check_err $? "Failed to create default chain"
+
+ tc chain add dev $h2 ingress chain 1
+ check_err $? "Failed to create chain 1"
+
+ tc chain del dev $h2 ingress
+ check_err $? "Failed to destroy default chain"
+
+ tc chain del dev $h2 ingress chain 1
+ check_err $? "Failed to destroy chain 1"
+
+ log_test "create destroy chain"
+}
+
setup_prepare()
{
h1=${NETIFS[p1]}
@@ -103,6 +122,8 @@ cleanup()
vrf_cleanup
}
+check_tc_chain_support
+
trap cleanup EXIT
setup_prepare
--
2.14.4
^ permalink raw reply related
* [patch net-next v4 10/12] selftests: forwarding: move shblock tc support check to a separate helper
From: Jiri Pirko @ 2018-07-23 7:24 UTC (permalink / raw)
To: netdev
Cc: davem, jhs, xiyou.wangcong, jakub.kicinski, simon.horman,
john.hurley, dsahern, mlxsw, sridhar.samudrala
In-Reply-To: <20180723072312.4153-1-jiri@resnulli.us>
From: Jiri Pirko <jiri@mellanox.com>
The shared block support is only needed for tc_shblock.sh. No need to
require that for other test.
Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
tools/testing/selftests/net/forwarding/lib.sh | 3 +++
tools/testing/selftests/net/forwarding/tc_shblocks.sh | 2 ++
2 files changed, 5 insertions(+)
diff --git a/tools/testing/selftests/net/forwarding/lib.sh b/tools/testing/selftests/net/forwarding/lib.sh
index 2bb9cf303c53..ec94395a54a7 100644
--- a/tools/testing/selftests/net/forwarding/lib.sh
+++ b/tools/testing/selftests/net/forwarding/lib.sh
@@ -33,7 +33,10 @@ check_tc_version()
echo "SKIP: iproute2 too old; tc is missing JSON support"
exit 1
fi
+}
+check_tc_shblock_support()
+{
tc filter help 2>&1 | grep block &> /dev/null
if [[ $? -ne 0 ]]; then
echo "SKIP: iproute2 too old; tc is missing shared block support"
diff --git a/tools/testing/selftests/net/forwarding/tc_shblocks.sh b/tools/testing/selftests/net/forwarding/tc_shblocks.sh
index b5b917203815..9826a446e2c0 100755
--- a/tools/testing/selftests/net/forwarding/tc_shblocks.sh
+++ b/tools/testing/selftests/net/forwarding/tc_shblocks.sh
@@ -105,6 +105,8 @@ cleanup()
ip link set $swp2 address $swp2origmac
}
+check_tc_shblock_support
+
trap cleanup EXIT
setup_prepare
--
2.14.4
^ permalink raw reply related
* [patch net-next v4 09/12] mlxsw: spectrum: Implement chain template hinting
From: Jiri Pirko @ 2018-07-23 7:23 UTC (permalink / raw)
To: netdev
Cc: davem, jhs, xiyou.wangcong, jakub.kicinski, simon.horman,
john.hurley, dsahern, mlxsw, sridhar.samudrala
In-Reply-To: <20180723072312.4153-1-jiri@resnulli.us>
From: Jiri Pirko <jiri@mellanox.com>
Since cld_flower provides information about the filter template for
specific chain, use this information in order to prepare a region.
Use the template to find out what elements are going to be used
and pass that down to mlxsw_sp_acl_tcam_group_add(). Later on, when the
first filter is inserted, the mlxsw_sp_acl_tcam_group_use_patterns()
function would use this element usage information instead of looking
up a pattern.
Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
drivers/net/ethernet/mellanox/mlxsw/spectrum.c | 5 +++
drivers/net/ethernet/mellanox/mlxsw/spectrum.h | 9 ++++-
drivers/net/ethernet/mellanox/mlxsw/spectrum_acl.c | 12 ++++--
.../ethernet/mellanox/mlxsw/spectrum_acl_tcam.c | 25 ++++++++++--
.../ethernet/mellanox/mlxsw/spectrum_acl_tcam.h | 3 +-
.../net/ethernet/mellanox/mlxsw/spectrum_flower.c | 44 ++++++++++++++++++++--
6 files changed, 86 insertions(+), 12 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
index 317c92d6496e..039228525fb1 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
@@ -1455,6 +1455,11 @@ mlxsw_sp_setup_tc_cls_flower(struct mlxsw_sp_acl_block *acl_block,
return 0;
case TC_CLSFLOWER_STATS:
return mlxsw_sp_flower_stats(mlxsw_sp, acl_block, f);
+ case TC_CLSFLOWER_TMPLT_CREATE:
+ return mlxsw_sp_flower_tmplt_create(mlxsw_sp, acl_block, f);
+ case TC_CLSFLOWER_TMPLT_DESTROY:
+ mlxsw_sp_flower_tmplt_destroy(mlxsw_sp, acl_block, f);
+ return 0;
default:
return -EOPNOTSUPP;
}
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum.h b/drivers/net/ethernet/mellanox/mlxsw/spectrum.h
index 016058961542..3db386c2573f 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum.h
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum.h
@@ -543,7 +543,8 @@ mlxsw_sp_acl_ruleset_lookup(struct mlxsw_sp *mlxsw_sp,
struct mlxsw_sp_acl_ruleset *
mlxsw_sp_acl_ruleset_get(struct mlxsw_sp *mlxsw_sp,
struct mlxsw_sp_acl_block *block, u32 chain_index,
- enum mlxsw_sp_acl_profile profile);
+ enum mlxsw_sp_acl_profile profile,
+ struct mlxsw_afk_element_usage *tmplt_elusage);
void mlxsw_sp_acl_ruleset_put(struct mlxsw_sp *mlxsw_sp,
struct mlxsw_sp_acl_ruleset *ruleset);
u16 mlxsw_sp_acl_ruleset_group_id(struct mlxsw_sp_acl_ruleset *ruleset);
@@ -667,6 +668,12 @@ void mlxsw_sp_flower_destroy(struct mlxsw_sp *mlxsw_sp,
int mlxsw_sp_flower_stats(struct mlxsw_sp *mlxsw_sp,
struct mlxsw_sp_acl_block *block,
struct tc_cls_flower_offload *f);
+int mlxsw_sp_flower_tmplt_create(struct mlxsw_sp *mlxsw_sp,
+ struct mlxsw_sp_acl_block *block,
+ struct tc_cls_flower_offload *f);
+void mlxsw_sp_flower_tmplt_destroy(struct mlxsw_sp *mlxsw_sp,
+ struct mlxsw_sp_acl_block *block,
+ struct tc_cls_flower_offload *f);
/* spectrum_qdisc.c */
int mlxsw_sp_tc_qdisc_init(struct mlxsw_sp_port *mlxsw_sp_port);
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_acl.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_acl.c
index 217621d79e26..4a4739139d11 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_acl.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_acl.c
@@ -317,7 +317,8 @@ int mlxsw_sp_acl_block_unbind(struct mlxsw_sp *mlxsw_sp,
static struct mlxsw_sp_acl_ruleset *
mlxsw_sp_acl_ruleset_create(struct mlxsw_sp *mlxsw_sp,
struct mlxsw_sp_acl_block *block, u32 chain_index,
- const struct mlxsw_sp_acl_profile_ops *ops)
+ const struct mlxsw_sp_acl_profile_ops *ops,
+ struct mlxsw_afk_element_usage *tmplt_elusage)
{
struct mlxsw_sp_acl *acl = mlxsw_sp->acl;
struct mlxsw_sp_acl_ruleset *ruleset;
@@ -337,7 +338,8 @@ mlxsw_sp_acl_ruleset_create(struct mlxsw_sp *mlxsw_sp,
if (err)
goto err_rhashtable_init;
- err = ops->ruleset_add(mlxsw_sp, &acl->tcam, ruleset->priv);
+ err = ops->ruleset_add(mlxsw_sp, &acl->tcam, ruleset->priv,
+ tmplt_elusage);
if (err)
goto err_ops_ruleset_add;
@@ -419,7 +421,8 @@ mlxsw_sp_acl_ruleset_lookup(struct mlxsw_sp *mlxsw_sp,
struct mlxsw_sp_acl_ruleset *
mlxsw_sp_acl_ruleset_get(struct mlxsw_sp *mlxsw_sp,
struct mlxsw_sp_acl_block *block, u32 chain_index,
- enum mlxsw_sp_acl_profile profile)
+ enum mlxsw_sp_acl_profile profile,
+ struct mlxsw_afk_element_usage *tmplt_elusage)
{
const struct mlxsw_sp_acl_profile_ops *ops;
struct mlxsw_sp_acl *acl = mlxsw_sp->acl;
@@ -434,7 +437,8 @@ mlxsw_sp_acl_ruleset_get(struct mlxsw_sp *mlxsw_sp,
mlxsw_sp_acl_ruleset_ref_inc(ruleset);
return ruleset;
}
- return mlxsw_sp_acl_ruleset_create(mlxsw_sp, block, chain_index, ops);
+ return mlxsw_sp_acl_ruleset_create(mlxsw_sp, block, chain_index, ops,
+ tmplt_elusage);
}
void mlxsw_sp_acl_ruleset_put(struct mlxsw_sp *mlxsw_sp,
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_acl_tcam.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_acl_tcam.c
index e06d7d9e5b7f..310fd87895b8 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_acl_tcam.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_acl_tcam.c
@@ -189,6 +189,8 @@ struct mlxsw_sp_acl_tcam_group {
struct mlxsw_sp_acl_tcam_group_ops *ops;
const struct mlxsw_sp_acl_tcam_pattern *patterns;
unsigned int patterns_count;
+ bool tmplt_elusage_set;
+ struct mlxsw_afk_element_usage tmplt_elusage;
};
struct mlxsw_sp_acl_tcam_chunk {
@@ -234,13 +236,19 @@ mlxsw_sp_acl_tcam_group_add(struct mlxsw_sp *mlxsw_sp,
struct mlxsw_sp_acl_tcam *tcam,
struct mlxsw_sp_acl_tcam_group *group,
const struct mlxsw_sp_acl_tcam_pattern *patterns,
- unsigned int patterns_count)
+ unsigned int patterns_count,
+ struct mlxsw_afk_element_usage *tmplt_elusage)
{
int err;
group->tcam = tcam;
group->patterns = patterns;
group->patterns_count = patterns_count;
+ if (tmplt_elusage) {
+ group->tmplt_elusage_set = true;
+ memcpy(&group->tmplt_elusage, tmplt_elusage,
+ sizeof(group->tmplt_elusage));
+ }
INIT_LIST_HEAD(&group->region_list);
err = mlxsw_sp_acl_tcam_group_id_get(tcam, &group->id);
if (err)
@@ -449,6 +457,15 @@ mlxsw_sp_acl_tcam_group_use_patterns(struct mlxsw_sp_acl_tcam_group *group,
const struct mlxsw_sp_acl_tcam_pattern *pattern;
int i;
+ /* In case the template is set, we don't have to look up the pattern
+ * and just use the template.
+ */
+ if (group->tmplt_elusage_set) {
+ memcpy(out, &group->tmplt_elusage, sizeof(*out));
+ WARN_ON(!mlxsw_afk_element_usage_subset(elusage, out));
+ return;
+ }
+
for (i = 0; i < group->patterns_count; i++) {
pattern = &group->patterns[i];
mlxsw_afk_element_usage_fill(out, pattern->elements,
@@ -865,13 +882,15 @@ struct mlxsw_sp_acl_tcam_flower_rule {
static int
mlxsw_sp_acl_tcam_flower_ruleset_add(struct mlxsw_sp *mlxsw_sp,
struct mlxsw_sp_acl_tcam *tcam,
- void *ruleset_priv)
+ void *ruleset_priv,
+ struct mlxsw_afk_element_usage *tmplt_elusage)
{
struct mlxsw_sp_acl_tcam_flower_ruleset *ruleset = ruleset_priv;
return mlxsw_sp_acl_tcam_group_add(mlxsw_sp, tcam, &ruleset->group,
mlxsw_sp_acl_tcam_patterns,
- MLXSW_SP_ACL_TCAM_PATTERNS_COUNT);
+ MLXSW_SP_ACL_TCAM_PATTERNS_COUNT,
+ tmplt_elusage);
}
static void
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_acl_tcam.h b/drivers/net/ethernet/mellanox/mlxsw/spectrum_acl_tcam.h
index 68551da2221d..6403ec4f5267 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_acl_tcam.h
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_acl_tcam.h
@@ -64,7 +64,8 @@ int mlxsw_sp_acl_tcam_priority_get(struct mlxsw_sp *mlxsw_sp,
struct mlxsw_sp_acl_profile_ops {
size_t ruleset_priv_size;
int (*ruleset_add)(struct mlxsw_sp *mlxsw_sp,
- struct mlxsw_sp_acl_tcam *tcam, void *ruleset_priv);
+ struct mlxsw_sp_acl_tcam *tcam, void *ruleset_priv,
+ struct mlxsw_afk_element_usage *tmplt_elusage);
void (*ruleset_del)(struct mlxsw_sp *mlxsw_sp, void *ruleset_priv);
int (*ruleset_bind)(struct mlxsw_sp *mlxsw_sp, void *ruleset_priv,
struct mlxsw_sp_port *mlxsw_sp_port,
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_flower.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_flower.c
index 201761a3539e..b3cb618775af 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_flower.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_flower.c
@@ -414,7 +414,7 @@ int mlxsw_sp_flower_replace(struct mlxsw_sp *mlxsw_sp,
ruleset = mlxsw_sp_acl_ruleset_get(mlxsw_sp, block,
f->common.chain_index,
- MLXSW_SP_ACL_PROFILE_FLOWER);
+ MLXSW_SP_ACL_PROFILE_FLOWER, NULL);
if (IS_ERR(ruleset))
return PTR_ERR(ruleset);
@@ -458,7 +458,7 @@ void mlxsw_sp_flower_destroy(struct mlxsw_sp *mlxsw_sp,
ruleset = mlxsw_sp_acl_ruleset_get(mlxsw_sp, block,
f->common.chain_index,
- MLXSW_SP_ACL_PROFILE_FLOWER);
+ MLXSW_SP_ACL_PROFILE_FLOWER, NULL);
if (IS_ERR(ruleset))
return;
@@ -484,7 +484,7 @@ int mlxsw_sp_flower_stats(struct mlxsw_sp *mlxsw_sp,
ruleset = mlxsw_sp_acl_ruleset_get(mlxsw_sp, block,
f->common.chain_index,
- MLXSW_SP_ACL_PROFILE_FLOWER);
+ MLXSW_SP_ACL_PROFILE_FLOWER, NULL);
if (WARN_ON(IS_ERR(ruleset)))
return -EINVAL;
@@ -506,3 +506,41 @@ int mlxsw_sp_flower_stats(struct mlxsw_sp *mlxsw_sp,
mlxsw_sp_acl_ruleset_put(mlxsw_sp, ruleset);
return err;
}
+
+int mlxsw_sp_flower_tmplt_create(struct mlxsw_sp *mlxsw_sp,
+ struct mlxsw_sp_acl_block *block,
+ struct tc_cls_flower_offload *f)
+{
+ struct mlxsw_sp_acl_ruleset *ruleset;
+ struct mlxsw_sp_acl_rule_info rulei;
+ int err;
+
+ memset(&rulei, 0, sizeof(rulei));
+ err = mlxsw_sp_flower_parse(mlxsw_sp, block, &rulei, f);
+ if (err)
+ return err;
+ ruleset = mlxsw_sp_acl_ruleset_get(mlxsw_sp, block,
+ f->common.chain_index,
+ MLXSW_SP_ACL_PROFILE_FLOWER,
+ &rulei.values.elusage);
+ if (IS_ERR(ruleset))
+ return PTR_ERR(ruleset);
+ /* keep the reference to the ruleset */
+ return 0;
+}
+
+void mlxsw_sp_flower_tmplt_destroy(struct mlxsw_sp *mlxsw_sp,
+ struct mlxsw_sp_acl_block *block,
+ struct tc_cls_flower_offload *f)
+{
+ struct mlxsw_sp_acl_ruleset *ruleset;
+
+ ruleset = mlxsw_sp_acl_ruleset_get(mlxsw_sp, block,
+ f->common.chain_index,
+ MLXSW_SP_ACL_PROFILE_FLOWER, NULL);
+ if (IS_ERR(ruleset))
+ return;
+ /* put the reference to the ruleset kept in create */
+ mlxsw_sp_acl_ruleset_put(mlxsw_sp, ruleset);
+ mlxsw_sp_acl_ruleset_put(mlxsw_sp, ruleset);
+}
--
2.14.4
^ permalink raw reply related
* [patch net-next v4 08/12] net: sched: cls_flower: propagate chain teplate creation and destruction to drivers
From: Jiri Pirko @ 2018-07-23 7:23 UTC (permalink / raw)
To: netdev
Cc: davem, jhs, xiyou.wangcong, jakub.kicinski, simon.horman,
john.hurley, dsahern, mlxsw, sridhar.samudrala
In-Reply-To: <20180723072312.4153-1-jiri@resnulli.us>
From: Jiri Pirko <jiri@mellanox.com>
Introduce a couple of flower offload commands in order to propagate
template creation/destruction events down to device drivers.
Drivers may use this information to prepare HW in an optimal way
for future filter insertions.
Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
v2->v3:
- rebase on top of the reoffload patchset
v1->v2:
- remove leftover extack arg in fl_hw_create_tmplt()
---
include/net/pkt_cls.h | 2 ++
net/sched/cls_flower.c | 39 +++++++++++++++++++++++++++++++++++++++
2 files changed, 41 insertions(+)
diff --git a/include/net/pkt_cls.h b/include/net/pkt_cls.h
index e4252a176eec..7673ee03f093 100644
--- a/include/net/pkt_cls.h
+++ b/include/net/pkt_cls.h
@@ -726,6 +726,8 @@ enum tc_fl_command {
TC_CLSFLOWER_REPLACE,
TC_CLSFLOWER_DESTROY,
TC_CLSFLOWER_STATS,
+ TC_CLSFLOWER_TMPLT_CREATE,
+ TC_CLSFLOWER_TMPLT_DESTROY,
};
struct tc_cls_flower_offload {
diff --git a/net/sched/cls_flower.c b/net/sched/cls_flower.c
index f0c80758a594..6ccf60364297 100644
--- a/net/sched/cls_flower.c
+++ b/net/sched/cls_flower.c
@@ -1194,6 +1194,42 @@ static int fl_reoffload(struct tcf_proto *tp, bool add, tc_setup_cb_t *cb,
return 0;
}
+static void fl_hw_create_tmplt(struct tcf_chain *chain,
+ struct fl_flow_tmplt *tmplt)
+{
+ struct tc_cls_flower_offload cls_flower = {};
+ struct tcf_block *block = chain->block;
+ struct tcf_exts dummy_exts = { 0, };
+
+ cls_flower.common.chain_index = chain->index;
+ cls_flower.command = TC_CLSFLOWER_TMPLT_CREATE;
+ cls_flower.cookie = (unsigned long) tmplt;
+ cls_flower.dissector = &tmplt->dissector;
+ cls_flower.mask = &tmplt->mask;
+ cls_flower.key = &tmplt->dummy_key;
+ cls_flower.exts = &dummy_exts;
+
+ /* We don't care if driver (any of them) fails to handle this
+ * call. It serves just as a hint for it.
+ */
+ tc_setup_cb_call(block, NULL, TC_SETUP_CLSFLOWER,
+ &cls_flower, false);
+}
+
+static void fl_hw_destroy_tmplt(struct tcf_chain *chain,
+ struct fl_flow_tmplt *tmplt)
+{
+ struct tc_cls_flower_offload cls_flower = {};
+ struct tcf_block *block = chain->block;
+
+ cls_flower.common.chain_index = chain->index;
+ cls_flower.command = TC_CLSFLOWER_TMPLT_DESTROY;
+ cls_flower.cookie = (unsigned long) tmplt;
+
+ tc_setup_cb_call(block, NULL, TC_SETUP_CLSFLOWER,
+ &cls_flower, false);
+}
+
static void *fl_tmplt_create(struct net *net, struct tcf_chain *chain,
struct nlattr **tca,
struct netlink_ext_ack *extack)
@@ -1224,6 +1260,8 @@ static void *fl_tmplt_create(struct net *net, struct tcf_chain *chain,
fl_init_dissector(&tmplt->dissector, &tmplt->mask);
+ fl_hw_create_tmplt(chain, tmplt);
+
return tmplt;
errout_tmplt:
@@ -1237,6 +1275,7 @@ static void fl_tmplt_destroy(void *tmplt_priv)
{
struct fl_flow_tmplt *tmplt = tmplt_priv;
+ fl_hw_destroy_tmplt(tmplt->chain, tmplt);
kfree(tmplt);
}
--
2.14.4
^ permalink raw reply related
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