lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Message-ID: <20231020180411.2a9f573d@kernel.org> Date: Fri, 20 Oct 2023 18:04:11 -0700 From: Jakub Kicinski <kuba@...nel.org> To: Stanislav Fomichev <sdf@...gle.com> Cc: bpf@...r.kernel.org, ast@...nel.org, daniel@...earbox.net, andrii@...nel.org, martin.lau@...ux.dev, song@...nel.org, yhs@...com, john.fastabend@...il.com, kpsingh@...nel.org, haoluo@...gle.com, jolsa@...nel.org, toke@...nel.org, willemb@...gle.com, dsahern@...nel.org, magnus.karlsson@...el.com, bjorn@...nel.org, maciej.fijalkowski@...el.com, hawk@...nel.org, yoong.siang.song@...el.com, netdev@...r.kernel.org, xdp-hints@...-project.net Subject: Re: [PATCH bpf-next v4 02/11] xsk: Add TX timestamp and TX checksum offload support On Thu, 19 Oct 2023 10:49:35 -0700 Stanislav Fomichev wrote: > diff --git a/Documentation/netlink/specs/netdev.yaml b/Documentation/netlink/specs/netdev.yaml > index 14511b13f305..22d2649a34ee 100644 > --- a/Documentation/netlink/specs/netdev.yaml > +++ b/Documentation/netlink/specs/netdev.yaml > @@ -55,6 +55,19 @@ name: netdev > name: hash > doc: > Device is capable of exposing receive packet hash via bpf_xdp_metadata_rx_hash(). > + - > + type: flags > + name: xsk-flags > + render-max: true I don't think you're using the MAX, maybe don't render it. IDK what purpose it'd serve for feature flag enums. > +/* > + * This structure defines the AF_XDP TX metadata hooks for network devices. s/This structure defines the // > + * The following hooks can be defined; unless noted otherwise, they are > + * optional and can be filled with a null pointer. > + * > + * void (*tmo_request_timestamp)(void *priv) > + * This function is called when AF_XDP frame requested egress timestamp. s/This function is // in many places > + * u64 (*tmo_fill_timestamp)(void *priv) > + * This function is called when AF_XDP frame, that had requested > + * egress timestamp, received a completion. The hook needs to return > + * the actual HW timestamp. > + * > + * void (*tmo_request_checksum)(u16 csum_start, u16 csum_offset, void *priv) > + * This function is called when AF_XDP frame requested HW checksum > + * offload. csum_start indicates position where checksumming should start. > + * csum_offset indicates position where checksum should be stored. > + * > + */ > +struct xsk_tx_metadata_ops { > + void (*tmo_request_timestamp)(void *priv); > + u64 (*tmo_fill_timestamp)(void *priv); > + void (*tmo_request_checksum)(u16 csum_start, u16 csum_offset, void *priv); > +}; Could you move the definition of the struct to include/net/xdp_sock.h ? netdevice.h doesn't need it. > +/* Request transmit timestamp. Upon completion, put it into tx_timestamp > + * field of struct xsk_tx_metadata. > + */ > +#define XDP_TX_METADATA_TIMESTAMP (1 << 0) > + > +/* Request transmit checksum offload. Checksum start position and offset > + * are communicated via csum_start and csum_offset fields of struct > + * xsk_tx_metadata. > + */ > +#define XDP_TX_METADATA_CHECKSUM (1 << 1) Reuse of enum netdev_xsk_flags is not an option? > +/* Force checksum calculation in software. Can be used for testing or > + * working around potential HW issues. This option causes performance > + * degradation and only works in XDP_COPY mode. > + */ > +#define XDP_TX_METADATA_CHECKSUM_SW (1 << 2) Is there a need for this to be on packet-by-packet basis? HW issues should generally be fixed by the driver, is there any type of problem in particular you have in mind here? > diff --git a/net/core/netdev-genl.c b/net/core/netdev-genl.c > index fe61f85bcf33..5d889c2425fd 100644 > --- a/net/core/netdev-genl.c > +++ b/net/core/netdev-genl.c > @@ -14,6 +14,7 @@ netdev_nl_dev_fill(struct net_device *netdev, struct sk_buff *rsp, > const struct genl_info *info) > { > u64 xdp_rx_meta = 0; > + u64 xsk_features = 0; rev xmas tree? :) > + meta = buffer - xs->pool->tx_metadata_len; > + > + if (meta->flags & XDP_TX_METADATA_CHECKSUM) { Do we need to worry about reserved / unsupported meta->flags ? > + if (unlikely(meta->csum_start + meta->csum_offset + > + sizeof(__sum16) > len)) { > + err = -EINVAL; > + goto free_err; > + }
Powered by blists - more mailing lists