From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jesper Dangaard Brouer Subject: Re: [bpf-next PATCH 2/2] samples/bpf: xdp_rxq_info action XDP_TX must adjust MAC-addrs Date: Wed, 27 Jun 2018 13:20:26 +0200 Message-ID: <20180627132026.79190a42@redhat.com> References: <152993682254.8835.8864318933370018087.stgit@firesoul> <152993686871.8835.4524876041721452950.stgit@firesoul> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8BIT Cc: Networking , Daniel Borkmann , Toke =?UTF-8?B?SMO4aWxhbmQtSsO4cmdlbnNlbg==?= , Alexei Starovoitov , brouer@redhat.com To: Song Liu Return-path: Received: from mx3-rdu2.redhat.com ([66.187.233.73]:56124 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1752144AbeF0LUb (ORCPT ); Wed, 27 Jun 2018 07:20:31 -0400 In-Reply-To: Sender: netdev-owner@vger.kernel.org List-ID: On Tue, 26 Jun 2018 17:09:01 -0700 Song Liu wrote: > On Mon, Jun 25, 2018 at 7:27 AM, Jesper Dangaard Brouer > wrote: > > XDP_TX requires also changing the MAC-addrs, else some hardware > > may drop the TX packet before reaching the wire. This was > > observed with driver mlx5. > > > > If xdp_rxq_info select --action XDP_TX the swapmac functionality > > is activated. It is also possible to manually enable via cmdline > > option --swapmac. This is practical if wanting to measure the > > overhead of writing/updating payload for other action types. > > > > Signed-off-by: Jesper Dangaard Brouer > > Signed-off-by: Toke Høiland-Jørgensen > > --- [...] > > > > diff --git a/samples/bpf/xdp_rxq_info_kern.c b/samples/bpf/xdp_rxq_info_kern.c > > index 61af6210df2f..222a83eed1cb 100644 > > --- a/samples/bpf/xdp_rxq_info_kern.c > > +++ b/samples/bpf/xdp_rxq_info_kern.c > > @@ -21,6 +21,7 @@ struct config { > > enum cfg_options_flags { > > NO_TOUCH = 0x0U, > > READ_MEM = 0x1U, > > + SWAP_MAC = 0x2U, > > }; [...] > > @@ -98,7 +116,7 @@ int xdp_prognum0(struct xdp_md *ctx) > > rxq_rec->issue++; > > > > /* Default: Don't touch packet data, only count packets */ > > - if (unlikely(config->options & READ_MEM)) { > > + if (unlikely(config->options & (READ_MEM|SWAP_MAC))) { > > struct ethhdr *eth = data; > > > > if (eth + 1 > data_end) [...] > > diff --git a/samples/bpf/xdp_rxq_info_user.c b/samples/bpf/xdp_rxq_info_user.c > > index 435485d4f49e..248a7eab9531 100644 [...] > > @@ -119,6 +121,8 @@ static char* options2str(enum cfg_options_flags flag) > > { > > if (flag == NO_TOUCH) > > return "no_touch"; > > + if (flag & SWAP_MAC) > > + return "swapmac"; > > if (flag & READ_MEM) > > return "read"; > > I guess SWAP_MAC also reads the memory, so it "includes" READ_MEM? True (see _kern side) > It is OK for now. We may need to refactor this part when adding other > flags in the future. Sure, do remember that this is only a 'sample' program. -- Best regards, Jesper Dangaard Brouer MSc.CS, Principal Kernel Engineer at Red Hat LinkedIn: http://www.linkedin.com/in/brouer