On Mon, Jul 31, 2023 at 06:03:26PM -0700, Alexei Starovoitov wrote:
> On Mon, Jul 31, 2023 at 3:56 AM Larysa Zaremba <larysa.zaremba@xxxxxxxxx> wrote:
> >
> > On Sun, Jul 30, 2023 at 09:13:02AM -0400, Willem de Bruijn wrote:
> > > Alexei Starovoitov wrote:
> > > > On Sat, Jul 29, 2023 at 9:15 AM Willem de Bruijn
> > > > <willemdebruijn.kernel@xxxxxxxxx> wrote:
> > > > >
> > > > > Alexei Starovoitov wrote:
> > > > > > On Fri, Jul 28, 2023 at 07:39:14PM +0200, Larysa Zaremba wrote:
> > > > > > >
> > > > > > > +union xdp_csum_info {
> > > > > > > + /* Checksum referred to by ``csum_start + csum_offset`` is considered
> > > > > > > + * valid, but was never calculated, TX device has to do this,
> > > > > > > + * starting from csum_start packet byte.
> > > > > > > + * Any preceding checksums are also considered valid.
> > > > > > > + * Available, if ``status == XDP_CHECKSUM_PARTIAL``.
> > > > > > > + */
> > > > > > > + struct {
> > > > > > > + u16 csum_start;
> > > > > > > + u16 csum_offset;
> > > > > > > + };
> > > > > > > +
> > > > > >
> > > > > > CHECKSUM_PARTIAL makes sense on TX, but this RX. I don't see in the above.
> > > > >
> > > > > It can be observed on RX when packets are looped.
> > > > >
> > > > > This may be observed even in XDP on veth.
> > > >
> > > > veth and XDP is a broken combination. GSO packets coming out of containers
> > > > cannot be parsed properly by XDP.
> > > > It was added mainly for testing. Just like "generic XDP".
> > > > bpf progs at skb layer is much better fit for veth.
> > >
> > > Ok. Still, seems forward looking and little cost to define the
> > > constant?
> > >
> >
> > +1
> > CHECKSUM_PARTIAL is mostly for testing and removing/adding it doesn't change
> > anything from the perspective of the user that does not use it, so I think it is
> > worth having.
>
> "little cost to define the constant".
> Not really. A constant in UAPI is a heavy burden.
Sorry for the delayed response.
I still do not comprehend the problem fully for this particular case,
considering it shouldn't block any future changes to the API by itself.
But, I personally have no reason to push hard the veth-supporting changes
(aside from wanting the tests to look nicer).
Still, before removing this in v5, I would like to get some additional feedback
on this, preferably from Jesper (who, if I remember correctly, takes an interest
in XDP on veth) or Stanislav.
If instead of union xdp_csum_info we will have just checksum as a second
argument, there will be no going back for this particular kfunc, so I want to be
sure nobody will ever need such feature.
[...]
