On 07/14, Lorenzo Bianconi wrote: > > On 07/08, Vladimir Vdovin wrote: > > > From: Lorenzo Bianconi <[email protected]> > > > > > > Introduce XDP RX checksum capability to XDP metadata specs. XDP RX > > > checksum will be use by devices capable of exposing receive checksum > > > result via bpf_xdp_metadata_rx_checksum(). > > > Moreover, introduce xmo_rx_checksum netdev callback in order to allow > > > the eBPF program bound to the device to retrieve the RX checksum result > > > computed by the hw NIC and reported via DMA descriptors. > > > > > > Signed-off-by: Lorenzo Bianconi <[email protected]> > > > Signed-off-by: Vladimir Vdovin <[email protected]> > > > --- > > > Documentation/netlink/specs/netdev.yaml | 5 ++++ > > > include/net/xdp.h | 18 ++++++++++++++ > > > include/uapi/linux/netdev.h | 3 +++ > > > net/core/xdp.c | 32 +++++++++++++++++++++++++ > > > tools/include/uapi/linux/netdev.h | 3 +++ > > > 5 files changed, 61 insertions(+) > > > > > > diff --git a/Documentation/netlink/specs/netdev.yaml > > > b/Documentation/netlink/specs/netdev.yaml > > > index 5f143da7458c..6d0d90d3a614 100644 > > > --- a/Documentation/netlink/specs/netdev.yaml > > > +++ b/Documentation/netlink/specs/netdev.yaml > > > @@ -61,6 +61,11 @@ definitions: > > > doc: | > > > Device is capable of exposing receive packet VLAN tag via > > > bpf_xdp_metadata_rx_vlan_tag(). > > > + - > > > + name: checksum > > > + doc: | > > > + Device is capable of exposing receive checksum result via > > > + bpf_xdp_metadata_rx_checksum(). > > > - > > > type: flags > > > name: xsk-flags > > > diff --git a/include/net/xdp.h b/include/net/xdp.h > > > index aa742f413c35..e255ff786131 100644 > > > --- a/include/net/xdp.h > > > +++ b/include/net/xdp.h > > > @@ -586,6 +586,10 @@ void xdp_attachment_setup(struct xdp_attachment_info > > > *info, > > > NETDEV_XDP_RX_METADATA_VLAN_TAG, \ > > > bpf_xdp_metadata_rx_vlan_tag, \ > > > xmo_rx_vlan_tag) \ > > > + XDP_METADATA_KFUNC(XDP_METADATA_KFUNC_RX_CHECKSUM, \ > > > + NETDEV_XDP_RX_METADATA_CHECKSUM, \ > > > + bpf_xdp_metadata_rx_checksum, \ > > > + xmo_rx_checksum) > > > > > > enum xdp_rx_metadata { > > > #define XDP_METADATA_KFUNC(name, _, __, ___) name, > > > @@ -643,12 +647,26 @@ enum xdp_rss_hash_type { > > > XDP_RSS_TYPE_L4_IPV6_SCTP_EX = XDP_RSS_TYPE_L4_IPV6_SCTP | > > > XDP_RSS_L3_DYNHDR, > > > }; > > > > [..] > > > > > +/* Please note the driver is required to invalidate the checksum if the > > > NIC > > > + * reports CHECKSUM_UNNECESSARY or CHECKSUM_COMPLETE and the eBPF program > > > + * modifies the packet since it can change some fields validated by the > > > + * checksum. > > > + */ > > > > Sorry, a bit confused about this part. IIUC this is in response to this > > discussion https://lore.kernel.org/bpf/aaLYfWnuuf_ne72u@lore-desk/ ? > > > > Looking at a few drivers: > > - bnxt (bnxt_rx_pkt) does UNNECESSARY - ok > > - mlx5 (mlx5e_handle_csum) does UNNECESSARY and skips COMPLETE if there is > > bpf prog attached > > - fbnic (fbnic_rx_csum) - can do COMPLETE even with xdp attached? > > - gve (gve_rx) - can do COMPLETE even with xdp attached? > > > > So none of the drivers invalidate anything (and nether the bpf machinery), > > some > > choose to (correctly?) not report COMPLETE when there is XDP. So was the > > discussion about documenting this and giving some guidance? This probably > > belongs to that `DOC: skb checksums` section of include/linux/skbuff.h? > > Hi Stanislav, > > re-reading the previous upstream discussion, I guess we should just document > the > expected driver behaviour (invalidate the cksum for both COMPLETE and > UNNECESSARY cases when the XDP program running on the NIC returns XDP_PASS) > and > add a given kselftest for it. Agree?
Yes, let's have something a bit more detailed about how it works right now vs the expectation. By my point is: the comment in this patch also seems wrong? We do not seem to invalidate the csum, we just disable COMPLETE when there is a xdp program attached (or I'm missing something).
