[{"id":1764745,"web_url":"http://patchwork.ozlabs.org/comment/1764745/","msgid":"<59B15334.4070103@iogearbox.net>","list_archive_url":null,"date":"2017-09-07T14:09:56","subject":"Re: [V2 PATCH net-next 1/2] xdp: implement xdp_redirect_map for\n\tgeneric XDP","submitter":{"id":65705,"url":"http://patchwork.ozlabs.org/api/people/65705/","name":"Daniel Borkmann","email":"daniel@iogearbox.net"},"content":"On 09/07/2017 02:33 PM, Jesper Dangaard Brouer wrote:\n> Using bpf_redirect_map is allowed for generic XDP programs, but the\n> appropriate map lookup was never performed in xdp_do_generic_redirect().\n>\n> Instead the map-index is directly used as the ifindex.  For the\n> xdp_redirect_map sample in SKB-mode '-S', this resulted in trying\n> sending on ifindex 0 which isn't valid, resulting in getting SKB\n> packets dropped.  Thus, the reported performance numbers are wrong in\n> commit 24251c264798 (\"samples/bpf: add option for native and skb mode\n> for redirect apps\") for the 'xdp_redirect_map -S' case.\n>\n> It might seem innocent this was lacking, but it can actually crash the\n> kernel.  The potential crash is caused by not consuming redirect_info->map.\n> The bpf_redirect_map helper will set this_cpu_ptr(&redirect_info)->map\n> pointer, which will survive even after unloading the xdp bpf_prog and\n> deallocating the devmap data-structure.  This leaves a dead map\n> pointer around.  The kernel will crash when loading the xdp_redirect\n> sample (in native XDP mode) as it doesn't reset map (via bpf_redirect)\n> and returns XDP_REDIRECT, which will cause it to dereference the map\n> pointer.\n>\n> Fixes: 6103aa96ec07 (\"net: implement XDP_REDIRECT for xdp generic\")\n> Fixes: 24251c264798 (\"samples/bpf: add option for native and skb mode for redirect apps\")\n> Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>\n> ---\n>   include/trace/events/xdp.h |    4 ++--\n>   net/core/filter.c          |   14 +++++++++++---\n>   2 files changed, 13 insertions(+), 5 deletions(-)\n>\n> diff --git a/include/trace/events/xdp.h b/include/trace/events/xdp.h\n> index 862575ac8da9..4e16c43fba10 100644\n> --- a/include/trace/events/xdp.h\n> +++ b/include/trace/events/xdp.h\n> @@ -138,11 +138,11 @@ DEFINE_EVENT_PRINT(xdp_redirect_template, xdp_redirect_map_err,\n>\n>   #define _trace_xdp_redirect_map(dev, xdp, fwd, map, idx)\t\t\\\n>   \t trace_xdp_redirect_map(dev, xdp, fwd ? fwd->ifindex : 0,\t\\\n> -\t\t\t\t0, map, idx);\n> +\t\t\t\t0, map, idx)\n>\n>   #define _trace_xdp_redirect_map_err(dev, xdp, fwd, map, idx, err)\t\\\n>   \t trace_xdp_redirect_map_err(dev, xdp, fwd ? fwd->ifindex : 0,\t\\\n> -\t\t\t\t    err, map, idx);\n> +\t\t\t\t    err, map, idx)\n>\n>   #endif /* _TRACE_XDP_H */\n>\n> diff --git a/net/core/filter.c b/net/core/filter.c\n> index 5912c738a7b2..3767470cab6c 100644\n> --- a/net/core/filter.c\n> +++ b/net/core/filter.c\n> @@ -2566,13 +2566,19 @@ int xdp_do_generic_redirect(struct net_device *dev, struct sk_buff *skb,\n>   \t\t\t    struct bpf_prog *xdp_prog)\n>   {\n>   \tstruct redirect_info *ri = this_cpu_ptr(&redirect_info);\n> +\tstruct bpf_map *map = ri->map;\n>   \tu32 index = ri->ifindex;\n>   \tstruct net_device *fwd;\n>   \tunsigned int len;\n>   \tint err = 0;\n>\n> -\tfwd = dev_get_by_index_rcu(dev_net(dev), index);\n>   \tri->ifindex = 0;\n> +\tri->map = NULL;\n> +\n> +\tif (map)\n> +\t\tfwd = __dev_map_lookup_elem(map, index);\n> +\telse\n> +\t\tfwd = dev_get_by_index_rcu(dev_net(dev), index);\n>   \tif (unlikely(!fwd)) {\n>   \t\terr = -EINVAL;\n>   \t\tgoto err;\n> @@ -2590,10 +2596,12 @@ int xdp_do_generic_redirect(struct net_device *dev, struct sk_buff *skb,\n>   \t}\n>\n>   \tskb->dev = fwd;\n\nLooks much better above, thanks!\n\n> -\t_trace_xdp_redirect(dev, xdp_prog, index);\n> +\tmap ? _trace_xdp_redirect_map(dev, xdp_prog, fwd, map, index)\n> +\t\t: _trace_xdp_redirect(dev, xdp_prog, index);\n\nCould we rather make this in a way such that when the two\ntracepoints are disabled and thus patched out, that we can\nalso omit the extra conditional which has no purpose then?\nPerhaps just a consolidated _trace_xdp_generic_redirect_map()\nwould be better to avoid this altogether given we have twice\nthe same anyway, here and in err path.\n\nThanks,\nDaniel\n\n>   \treturn 0;\n>   err:\n> -\t_trace_xdp_redirect_err(dev, xdp_prog, index, err);\n> +\tmap ? _trace_xdp_redirect_map_err(dev, xdp_prog, fwd, map, index, err)\n> +\t\t: _trace_xdp_redirect_err(dev, xdp_prog, index, err);\n>   \treturn err;\n>   }\n>   EXPORT_SYMBOL_GPL(xdp_do_generic_redirect);\n>","headers":{"Return-Path":"<netdev-owner@vger.kernel.org>","X-Original-To":"patchwork-incoming@ozlabs.org","Delivered-To":"patchwork-incoming@ozlabs.org","Authentication-Results":"ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=netdev-owner@vger.kernel.org;\n\treceiver=<UNKNOWN>)","Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3xp2Qd5v0Sz9s9Y\n\tfor <patchwork-incoming@ozlabs.org>;\n\tFri,  8 Sep 2017 00:10:01 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1755408AbdIGOJ7 (ORCPT <rfc822;patchwork-incoming@ozlabs.org>);\n\tThu, 7 Sep 2017 10:09:59 -0400","from www62.your-server.de ([213.133.104.62]:34239 \"EHLO\n\twww62.your-server.de\" rhost-flags-OK-OK-OK-OK) by vger.kernel.org\n\twith ESMTP id S1754005AbdIGOJ7 (ORCPT\n\t<rfc822;netdev@vger.kernel.org>); Thu, 7 Sep 2017 10:09:59 -0400","from [92.105.166.74] (helo=localhost.localdomain)\n\tby www62.your-server.de with esmtpsa (TLSv1.2:DHE-RSA-AES256-SHA:256)\n\t(Exim 4.85_2) (envelope-from <daniel@iogearbox.net>)\n\tid 1dpxVF-0000sn-4r; Thu, 07 Sep 2017 16:09:57 +0200"],"Message-ID":"<59B15334.4070103@iogearbox.net>","Date":"Thu, 07 Sep 2017 16:09:56 +0200","From":"Daniel Borkmann <daniel@iogearbox.net>","User-Agent":"Mozilla/5.0 (X11; Linux x86_64;\n\trv:31.0) Gecko/20100101 Thunderbird/31.7.0","MIME-Version":"1.0","To":"Jesper Dangaard Brouer <brouer@redhat.com>, netdev@vger.kernel.org,\n\t\"David S. Miller\" <davem@davemloft.net>","CC":"Daniel Borkmann <borkmann@iogearbox.net>,\n\tJohn Fastabend <john.fastabend@gmail.com>,\n\tAndy Gospodarek <andy@greyhouse.net>, alexei.starovoitov@gmail.com","Subject":"Re: [V2 PATCH net-next 1/2] xdp: implement xdp_redirect_map for\n\tgeneric XDP","References":"<150478756604.28665.6915020425359475729.stgit@firesoul>\n\t<150478759310.28665.17184783248584070473.stgit@firesoul>","In-Reply-To":"<150478759310.28665.17184783248584070473.stgit@firesoul>","Content-Type":"text/plain; charset=utf-8; format=flowed","Content-Transfer-Encoding":"7bit","X-Authenticated-Sender":"daniel@iogearbox.net","X-Virus-Scanned":"Clear (ClamAV 0.99.2/23788/Thu Sep  7 14:38:32 2017)","Sender":"netdev-owner@vger.kernel.org","Precedence":"bulk","List-ID":"<netdev.vger.kernel.org>","X-Mailing-List":"netdev@vger.kernel.org"}},{"id":1765151,"web_url":"http://patchwork.ozlabs.org/comment/1765151/","msgid":"<20170908103601.21cdecb2@redhat.com>","list_archive_url":null,"date":"2017-09-08T08:36:01","subject":"Re: [V2 PATCH net-next 1/2] xdp: implement xdp_redirect_map for\n\tgeneric XDP","submitter":{"id":13625,"url":"http://patchwork.ozlabs.org/api/people/13625/","name":"Jesper Dangaard Brouer","email":"brouer@redhat.com"},"content":"On Thu, 07 Sep 2017 16:09:56 +0200\nDaniel Borkmann <daniel@iogearbox.net> wrote:\n\n> On 09/07/2017 02:33 PM, Jesper Dangaard Brouer wrote:\n> > Using bpf_redirect_map is allowed for generic XDP programs, but the\n> > appropriate map lookup was never performed in xdp_do_generic_redirect().\n> >\n> > Instead the map-index is directly used as the ifindex.  For the\n> > xdp_redirect_map sample in SKB-mode '-S', this resulted in trying\n> > sending on ifindex 0 which isn't valid, resulting in getting SKB\n> > packets dropped.  Thus, the reported performance numbers are wrong in\n> > commit 24251c264798 (\"samples/bpf: add option for native and skb mode\n> > for redirect apps\") for the 'xdp_redirect_map -S' case.\n> >\n> > It might seem innocent this was lacking, but it can actually crash the\n> > kernel.  The potential crash is caused by not consuming redirect_info->map.\n> > The bpf_redirect_map helper will set this_cpu_ptr(&redirect_info)->map\n> > pointer, which will survive even after unloading the xdp bpf_prog and\n> > deallocating the devmap data-structure.  This leaves a dead map\n> > pointer around.  The kernel will crash when loading the xdp_redirect\n> > sample (in native XDP mode) as it doesn't reset map (via bpf_redirect)\n> > and returns XDP_REDIRECT, which will cause it to dereference the map\n> > pointer.\n> >\n> > Fixes: 6103aa96ec07 (\"net: implement XDP_REDIRECT for xdp generic\")\n> > Fixes: 24251c264798 (\"samples/bpf: add option for native and skb mode for redirect apps\")\n> > Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>\n> > ---\n> >   include/trace/events/xdp.h |    4 ++--\n> >   net/core/filter.c          |   14 +++++++++++---\n> >   2 files changed, 13 insertions(+), 5 deletions(-)\n> >\n> > diff --git a/include/trace/events/xdp.h b/include/trace/events/xdp.h\n> > index 862575ac8da9..4e16c43fba10 100644\n> > --- a/include/trace/events/xdp.h\n> > +++ b/include/trace/events/xdp.h\n> > @@ -138,11 +138,11 @@ DEFINE_EVENT_PRINT(xdp_redirect_template, xdp_redirect_map_err,\n> >\n> >   #define _trace_xdp_redirect_map(dev, xdp, fwd, map, idx)\t\t\\\n> >   \t trace_xdp_redirect_map(dev, xdp, fwd ? fwd->ifindex : 0,\t\\\n> > -\t\t\t\t0, map, idx);\n> > +\t\t\t\t0, map, idx)\n> >\n> >   #define _trace_xdp_redirect_map_err(dev, xdp, fwd, map, idx, err)\t\\\n> >   \t trace_xdp_redirect_map_err(dev, xdp, fwd ? fwd->ifindex : 0,\t\\\n> > -\t\t\t\t    err, map, idx);\n> > +\t\t\t\t    err, map, idx)\n> >\n> >   #endif /* _TRACE_XDP_H */\n> >\n> > diff --git a/net/core/filter.c b/net/core/filter.c\n> > index 5912c738a7b2..3767470cab6c 100644\n> > --- a/net/core/filter.c\n> > +++ b/net/core/filter.c\n> > @@ -2566,13 +2566,19 @@ int xdp_do_generic_redirect(struct net_device *dev, struct sk_buff *skb,\n> >   \t\t\t    struct bpf_prog *xdp_prog)\n> >   {\n> >   \tstruct redirect_info *ri = this_cpu_ptr(&redirect_info);\n> > +\tstruct bpf_map *map = ri->map;\n> >   \tu32 index = ri->ifindex;\n> >   \tstruct net_device *fwd;\n> >   \tunsigned int len;\n> >   \tint err = 0;\n> >\n> > -\tfwd = dev_get_by_index_rcu(dev_net(dev), index);\n> >   \tri->ifindex = 0;\n> > +\tri->map = NULL;\n> > +\n> > +\tif (map)\n> > +\t\tfwd = __dev_map_lookup_elem(map, index);\n> > +\telse\n> > +\t\tfwd = dev_get_by_index_rcu(dev_net(dev), index);\n> >   \tif (unlikely(!fwd)) {\n> >   \t\terr = -EINVAL;\n> >   \t\tgoto err;\n> > @@ -2590,10 +2596,12 @@ int xdp_do_generic_redirect(struct net_device *dev, struct sk_buff *skb,\n> >   \t}\n> >\n> >   \tskb->dev = fwd;  \n> \n> Looks much better above, thanks!\n> \n> > -\t_trace_xdp_redirect(dev, xdp_prog, index);\n> > +\tmap ? _trace_xdp_redirect_map(dev, xdp_prog, fwd, map, index)\n> > +\t\t: _trace_xdp_redirect(dev, xdp_prog, index);  \n> \n> Could we rather make this in a way such that when the two\n> tracepoints are disabled and thus patched out, that we can\n> also omit the extra conditional which has no purpose then?\n\nFirst of all I don't think it make much of a difference, I measured the\nimpact of the full patch to \"cost\" 1.62 nanosec (which is arguably\nbelow the accuracy level of the system under test)\n\nSecondly, I plan to optimize the map case for generic XDP later, where\nI would naturally split this into two functions (as V1, and as\nnative-XDP), thus this extra conditional would go away.  As I've shown\nofflist (to you, John and Andy) I demonstrated a 24% speedup via a\nxmit_more hack for generic XDP.\n\n\n> Perhaps just a consolidated _trace_xdp_generic_redirect_map()\n> would be better to avoid this altogether given we have twice\n> the same anyway, here and in err path.\n\nI do want separate tracepoints for xdp_redirect and xdp_redirect_map,\nas it makes it more clear for users of the tracepoint (and attached\nbpf_prog's can be faster, knowing the context).","headers":{"Return-Path":"<netdev-owner@vger.kernel.org>","X-Original-To":"patchwork-incoming@ozlabs.org","Delivered-To":"patchwork-incoming@ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=netdev-owner@vger.kernel.org;\n\treceiver=<UNKNOWN>)","ext-mx04.extmail.prod.ext.phx2.redhat.com;\n\tdmarc=none (p=none dis=none) header.from=redhat.com","ext-mx04.extmail.prod.ext.phx2.redhat.com;\n\tspf=fail smtp.mailfrom=brouer@redhat.com"],"Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3xpVz22B6Lz9sBW\n\tfor <patchwork-incoming@ozlabs.org>;\n\tFri,  8 Sep 2017 18:36:14 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1754774AbdIHIgL (ORCPT <rfc822;patchwork-incoming@ozlabs.org>);\n\tFri, 8 Sep 2017 04:36:11 -0400","from mx1.redhat.com ([209.132.183.28]:47448 \"EHLO mx1.redhat.com\"\n\trhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP\n\tid S1751718AbdIHIgI (ORCPT <rfc822;netdev@vger.kernel.org>);\n\tFri, 8 Sep 2017 04:36:08 -0400","from smtp.corp.redhat.com\n\t(int-mx03.intmail.prod.int.phx2.redhat.com [10.5.11.13])\n\t(using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby mx1.redhat.com (Postfix) with ESMTPS id B0606821C7;\n\tFri,  8 Sep 2017 08:36:07 +0000 (UTC)","from localhost (ovpn-200-27.brq.redhat.com [10.40.200.27])\n\tby smtp.corp.redhat.com (Postfix) with ESMTP id 0BF2960A9B;\n\tFri,  8 Sep 2017 08:36:02 +0000 (UTC)"],"DMARC-Filter":"OpenDMARC Filter v1.3.2 mx1.redhat.com B0606821C7","Date":"Fri, 8 Sep 2017 10:36:01 +0200","From":"Jesper Dangaard Brouer <brouer@redhat.com>","To":"Daniel Borkmann <daniel@iogearbox.net>","Cc":"netdev@vger.kernel.org, \"David S. Miller\" <davem@davemloft.net>,\n\tDaniel Borkmann <borkmann@iogearbox.net>,\n\tJohn Fastabend <john.fastabend@gmail.com>,\n\tAndy Gospodarek <andy@greyhouse.net>,\n\talexei.starovoitov@gmail.com, brouer@redhat.com","Subject":"Re: [V2 PATCH net-next 1/2] xdp: implement xdp_redirect_map for\n\tgeneric XDP","Message-ID":"<20170908103601.21cdecb2@redhat.com>","In-Reply-To":"<59B15334.4070103@iogearbox.net>","References":"<150478756604.28665.6915020425359475729.stgit@firesoul>\n\t<150478759310.28665.17184783248584070473.stgit@firesoul>\n\t<59B15334.4070103@iogearbox.net>","MIME-Version":"1.0","Content-Type":"text/plain; charset=US-ASCII","Content-Transfer-Encoding":"7bit","X-Scanned-By":"MIMEDefang 2.79 on 10.5.11.13","X-Greylist":"Sender IP whitelisted, not delayed by milter-greylist-4.5.16\n\t(mx1.redhat.com [10.5.110.28]);\n\tFri, 08 Sep 2017 08:36:08 +0000 (UTC)","Sender":"netdev-owner@vger.kernel.org","Precedence":"bulk","List-ID":"<netdev.vger.kernel.org>","X-Mailing-List":"netdev@vger.kernel.org"}},{"id":1765235,"web_url":"http://patchwork.ozlabs.org/comment/1765235/","msgid":"<59B273C1.9060806@iogearbox.net>","list_archive_url":null,"date":"2017-09-08T10:41:05","subject":"Re: [V2 PATCH net-next 1/2] xdp: implement xdp_redirect_map for\n\tgeneric XDP","submitter":{"id":65705,"url":"http://patchwork.ozlabs.org/api/people/65705/","name":"Daniel Borkmann","email":"daniel@iogearbox.net"},"content":"On 09/08/2017 10:36 AM, Jesper Dangaard Brouer wrote:\n> On Thu, 07 Sep 2017 16:09:56 +0200\n> Daniel Borkmann <daniel@iogearbox.net> wrote:\n>> On 09/07/2017 02:33 PM, Jesper Dangaard Brouer wrote:\n>>> Using bpf_redirect_map is allowed for generic XDP programs, but the\n>>> appropriate map lookup was never performed in xdp_do_generic_redirect().\n>>>\n>>> Instead the map-index is directly used as the ifindex.  For the\n>>> xdp_redirect_map sample in SKB-mode '-S', this resulted in trying\n>>> sending on ifindex 0 which isn't valid, resulting in getting SKB\n>>> packets dropped.  Thus, the reported performance numbers are wrong in\n>>> commit 24251c264798 (\"samples/bpf: add option for native and skb mode\n>>> for redirect apps\") for the 'xdp_redirect_map -S' case.\n>>>\n>>> It might seem innocent this was lacking, but it can actually crash the\n>>> kernel.  The potential crash is caused by not consuming redirect_info->map.\n>>> The bpf_redirect_map helper will set this_cpu_ptr(&redirect_info)->map\n>>> pointer, which will survive even after unloading the xdp bpf_prog and\n>>> deallocating the devmap data-structure.  This leaves a dead map\n>>> pointer around.  The kernel will crash when loading the xdp_redirect\n>>> sample (in native XDP mode) as it doesn't reset map (via bpf_redirect)\n>>> and returns XDP_REDIRECT, which will cause it to dereference the map\n>>> pointer.\n>>>\n>>> Fixes: 6103aa96ec07 (\"net: implement XDP_REDIRECT for xdp generic\")\n>>> Fixes: 24251c264798 (\"samples/bpf: add option for native and skb mode for redirect apps\")\n>>> Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>\n>>> ---\n>>>    include/trace/events/xdp.h |    4 ++--\n>>>    net/core/filter.c          |   14 +++++++++++---\n>>>    2 files changed, 13 insertions(+), 5 deletions(-)\n>>>\n>>> diff --git a/include/trace/events/xdp.h b/include/trace/events/xdp.h\n>>> index 862575ac8da9..4e16c43fba10 100644\n>>> --- a/include/trace/events/xdp.h\n>>> +++ b/include/trace/events/xdp.h\n>>> @@ -138,11 +138,11 @@ DEFINE_EVENT_PRINT(xdp_redirect_template, xdp_redirect_map_err,\n>>>\n>>>    #define _trace_xdp_redirect_map(dev, xdp, fwd, map, idx)\t\t\\\n>>>    \t trace_xdp_redirect_map(dev, xdp, fwd ? fwd->ifindex : 0,\t\\\n>>> -\t\t\t\t0, map, idx);\n>>> +\t\t\t\t0, map, idx)\n>>>\n>>>    #define _trace_xdp_redirect_map_err(dev, xdp, fwd, map, idx, err)\t\\\n>>>    \t trace_xdp_redirect_map_err(dev, xdp, fwd ? fwd->ifindex : 0,\t\\\n>>> -\t\t\t\t    err, map, idx);\n>>> +\t\t\t\t    err, map, idx)\n>>>\n>>>    #endif /* _TRACE_XDP_H */\n>>>\n>>> diff --git a/net/core/filter.c b/net/core/filter.c\n>>> index 5912c738a7b2..3767470cab6c 100644\n>>> --- a/net/core/filter.c\n>>> +++ b/net/core/filter.c\n>>> @@ -2566,13 +2566,19 @@ int xdp_do_generic_redirect(struct net_device *dev, struct sk_buff *skb,\n>>>    \t\t\t    struct bpf_prog *xdp_prog)\n>>>    {\n>>>    \tstruct redirect_info *ri = this_cpu_ptr(&redirect_info);\n>>> +\tstruct bpf_map *map = ri->map;\n>>>    \tu32 index = ri->ifindex;\n>>>    \tstruct net_device *fwd;\n>>>    \tunsigned int len;\n>>>    \tint err = 0;\n>>>\n>>> -\tfwd = dev_get_by_index_rcu(dev_net(dev), index);\n>>>    \tri->ifindex = 0;\n>>> +\tri->map = NULL;\n>>> +\n>>> +\tif (map)\n>>> +\t\tfwd = __dev_map_lookup_elem(map, index);\n>>> +\telse\n>>> +\t\tfwd = dev_get_by_index_rcu(dev_net(dev), index);\n>>>    \tif (unlikely(!fwd)) {\n>>>    \t\terr = -EINVAL;\n>>>    \t\tgoto err;\n>>> @@ -2590,10 +2596,12 @@ int xdp_do_generic_redirect(struct net_device *dev, struct sk_buff *skb,\n>>>    \t}\n>>>\n>>>    \tskb->dev = fwd;\n>>\n>> Looks much better above, thanks!\n>>\n>>> -\t_trace_xdp_redirect(dev, xdp_prog, index);\n>>> +\tmap ? _trace_xdp_redirect_map(dev, xdp_prog, fwd, map, index)\n>>> +\t\t: _trace_xdp_redirect(dev, xdp_prog, index);\n>>\n>> Could we rather make this in a way such that when the two\n>> tracepoints are disabled and thus patched out, that we can\n>> also omit the extra conditional which has no purpose then?\n>\n> First of all I don't think it make much of a difference, I measured the\n> impact of the full patch to \"cost\" 1.62 nanosec (which is arguably\n> below the accuracy level of the system under test)\n>\n> Secondly, I plan to optimize the map case for generic XDP later, where\n> I would naturally split this into two functions (as V1, and as\n> native-XDP), thus this extra conditional would go away.  As I've shown\n> offlist (to you, John and Andy) I demonstrated a 24% speedup via a\n> xmit_more hack for generic XDP.\n\nOkay, that would be nice indeed to have xmit_more support for\ngeneric XDP as well. If this is going to be split off anyway\nlater on as in xdp_do_redirect() case, then:\n\nAcked-by: Daniel Borkmann <daniel@iogearbox.net>","headers":{"Return-Path":"<netdev-owner@vger.kernel.org>","X-Original-To":"patchwork-incoming@ozlabs.org","Delivered-To":"patchwork-incoming@ozlabs.org","Authentication-Results":"ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=netdev-owner@vger.kernel.org;\n\treceiver=<UNKNOWN>)","Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3xpYlC5wMFz9sCZ\n\tfor <patchwork-incoming@ozlabs.org>;\n\tFri,  8 Sep 2017 20:41:11 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1754597AbdIHKlJ (ORCPT <rfc822;patchwork-incoming@ozlabs.org>);\n\tFri, 8 Sep 2017 06:41:09 -0400","from www62.your-server.de ([213.133.104.62]:52071 \"EHLO\n\twww62.your-server.de\" rhost-flags-OK-OK-OK-OK) by vger.kernel.org\n\twith ESMTP id S1753132AbdIHKlI (ORCPT\n\t<rfc822;netdev@vger.kernel.org>); Fri, 8 Sep 2017 06:41:08 -0400","from [85.7.161.218] (helo=localhost.localdomain)\n\tby www62.your-server.de with esmtpsa (TLSv1.2:DHE-RSA-AES256-SHA:256)\n\t(Exim 4.85_2) (envelope-from <daniel@iogearbox.net>)\n\tid 1dqGig-0001Ad-H7; Fri, 08 Sep 2017 12:41:06 +0200"],"Message-ID":"<59B273C1.9060806@iogearbox.net>","Date":"Fri, 08 Sep 2017 12:41:05 +0200","From":"Daniel Borkmann <daniel@iogearbox.net>","User-Agent":"Mozilla/5.0 (X11; Linux x86_64;\n\trv:31.0) Gecko/20100101 Thunderbird/31.7.0","MIME-Version":"1.0","To":"Jesper Dangaard Brouer <brouer@redhat.com>","CC":"netdev@vger.kernel.org, \"David S. Miller\" <davem@davemloft.net>,\n\tDaniel Borkmann <borkmann@iogearbox.net>,\n\tJohn Fastabend <john.fastabend@gmail.com>,\n\tAndy Gospodarek <andy@greyhouse.net>, alexei.starovoitov@gmail.com","Subject":"Re: [V2 PATCH net-next 1/2] xdp: implement xdp_redirect_map for\n\tgeneric XDP","References":"<150478756604.28665.6915020425359475729.stgit@firesoul>\n\t<150478759310.28665.17184783248584070473.stgit@firesoul>\n\t<59B15334.4070103@iogearbox.net>\n\t<20170908103601.21cdecb2@redhat.com>","In-Reply-To":"<20170908103601.21cdecb2@redhat.com>","Content-Type":"text/plain; charset=windows-1252; format=flowed","Content-Transfer-Encoding":"7bit","X-Authenticated-Sender":"daniel@iogearbox.net","X-Virus-Scanned":"Clear (ClamAV 0.99.2/23793/Fri Sep  8 10:39:01 2017)","Sender":"netdev-owner@vger.kernel.org","Precedence":"bulk","List-ID":"<netdev.vger.kernel.org>","X-Mailing-List":"netdev@vger.kernel.org"}}]