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;
> + }
