Re: [PATCHv2 bpf-next] samples/bpf: add xdp program on egress for xdp_redirect_map

From: Jesper Dangaard Brouer <brouer@redhat.com>
To: Hangbin Liu <liuhangbin@gmail.com>
Cc: "Yonghong Song" <yhs@fb.com>,
	bpf@vger.kernel.org, netdev@vger.kernel.org,
	"Daniel Borkmann" <daniel@iogearbox.net>,
	"John Fastabend" <john.fastabend@gmail.com>,
	"Toke Høiland-Jørgensen" <toke@redhat.com>,
	"Tariq Toukan" <tariqt@mellanox.com>,
	"Maciej Fijalkowski" <maciej.fijalkowski@intel.com>,
	brouer@redhat.com
Subject: Re: [PATCHv2 bpf-next] samples/bpf: add xdp program on egress for xdp_redirect_map
Date: Mon, 30 Nov 2020 16:12:49 +0100	[thread overview]
Message-ID: <20201130161249.18f7ca43@carbon> (raw)
In-Reply-To: <20201130131020.GC277949@localhost.localdomain>

On Mon, 30 Nov 2020 21:10:20 +0800
Hangbin Liu <liuhangbin@gmail.com> wrote:

> On Mon, Nov 30, 2020 at 10:32:08AM +0100, Jesper Dangaard Brouer wrote:
> > > I plan to write a example about vlan header modification based on egress
> > > index. I will post the patch later.  
> > 
> > I did notice the internal thread you had with Toke.  I still think it
> > will be more simple to modify the Ethernet mac addresses.  Adding a
> > VLAN id tag is more work, and will confuse benchmarks.  You are  
> 
> I plan to only modify the vlan id if there has. 

This sentence is not complete, but because of the internal thread I
know/assume that you mean, that you will only modify the vlan id if
there is already another VLAN tag in the packet. Let me express that
this is not good enough. This is not a feasible choice.

> If you prefer to modify the mac address, which way you'd like? Set
> src mac to egress interface's MAC?

Yes, that will be a good choice, to use the src mac from the egress
interface.  This would simulate part of what is needed for L3/routing.

Can I request that the dst mac is will be the incoming src mac?
Or if you are user-friendly add option that allows to set dst mac.

This is close to what swap-MAC (swap_src_dst_mac) is used for.  Let me
explain in more details, why this is practical.  It is practical
because then the Ethernet frame will be a valid frame that is received
by the sending interface.  Thus, if you redirect back same interface
(like XDP_TX, but testing xdp_do_redirect code) then you can check on
traffic generator if all frames were actually forwarded.  This is
exactly what the Red Hat performance team's Trex packet generator setup
does to validate and find the zero-loss generator rate.

> > As Alexei already pointed out, you assignment is to modify the packet
> > in the 2nd devmap XDP-prog.  Why: because you need to realize that this
> > will break your approach to multicast in your previous patchset.
> > (Yes, the offlist patch I gave you, that move running 2nd devmap
> > XDP-prog to a later stage, solved this packet-modify issue).  
> 
> BTW, it looks with your patch, the counter on egress would make more sense.
> Should I add the counter after your patch posted?

As I tried to explain.  Regardless, I want a counter that counts the
times the 2nd devmap attached XDP-program runs.  This is not a counter
that counts egress packets.  This is a counter that show that the 2nd
devmap attached XDP-program is running.  I don't know how to make this
more clear.

We do need ANOTHER counter that report how many packets are transmitted
on the egress device.  I'm thinking we can simply read:

 /sys/class/net/mlx5p1/statistics/tx_packets

-- 
Best regards,
  Jesper Dangaard Brouer
  MSc.CS, Principal Kernel Engineer at Red Hat
  LinkedIn: http://www.linkedin.com/in/brouer