Re: [PATCHv11 bpf-next 2/5] xdp: add a new helper for dev map multicast support

On Thu, 10 Sep 2020 11:44:50 +0200
Toke Høiland-Jørgensen [off-list ref] wrote:

Alexei Starovoitov [off-list ref] writes:

quoted

On Wed, Sep 9, 2020 at 8:30 PM David Ahern [off-list ref] wrote:  

quoted

quoted

I think the packets modification (edit dst mac, add vlan tag, etc) should be
done on egress, which rely on David's XDP egress support.  

agreed. The DEVMAP used for redirect can have programs attached that
update the packet headers - assuming you want to update them.  

Then you folks have to submit them as one set.
As-is the programmer cannot achieve correct behavior.  

The ability to attach a program to devmaps is already there. See:

fbee97feed9b ("bpf: Add support to attach bpf program to a devmap entry")

But now that you mention it, it does appear that this series is skipping
the hook that will actually run such a program. Didn't realise that was
in the caller of bq_enqueue() and not inside bq_enqueue() itself...

In the first revisions of Ahern's patchset (before fully integrated in
devmap), this was the case, but it changed in some of the last
revisions. (This also lost the sort-n-bulk effect in the process, that
optimize I-cache).  In these earlier revisions it operated on
xdp_frame's.  It would have been a lot easier for Hangbin's patch if
the devmap-prog operated on these xdp_frame's.

Maybe we should change the devmap-prog approach, and run this on the
xdp_frame's (in bq_xmit_all() to be precise) .  Hangbin's patchset
clearly shows that we need this "layer" between running the xdp_prog and
the devmap-prog. 

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