All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Michael S. Tsirkin" <mst@redhat.com>
To: chia-yu.chang@nokia-bell-labs.com
Cc: tariqt@nvidia.com, linux-rdma@vger.kernel.org,
	shaojijie@huawei.com, shenjian15@huawei.com,
	salil.mehta@huawei.com, mbloch@nvidia.com, saeedm@nvidia.com,
	leon@kernel.org, eperezma@redhat.com, brett.creeley@amd.com,
	jasowang@redhat.com, virtualization@lists.linux.dev,
	xuanzhuo@linux.alibaba.com, pabeni@redhat.com,
	edumazet@google.com, parav@nvidia.com, linux-doc@vger.kernel.org,
	corbet@lwn.net, horms@kernel.org, dsahern@kernel.org,
	kuniyu@google.com, bpf@vger.kernel.org, netdev@vger.kernel.org,
	dave.taht@gmail.com, jhs@mojatatu.com, kuba@kernel.org,
	stephen@networkplumber.org, xiyou.wangcong@gmail.com,
	jiri@resnulli.us, davem@davemloft.net, andrew+netdev@lunn.ch,
	donald.hunter@gmail.com, ast@fiberby.net, liuhangbin@gmail.com,
	shuah@kernel.org, linux-kselftest@vger.kernel.org, ij@kernel.org,
	ncardwell@google.com, koen.de_schepper@nokia-bell-labs.com,
	g.white@cablelabs.com, ingemar.s.johansson@ericsson.com,
	mirja.kuehlewind@ericsson.com, cheshire@apple.com,
	rs.ietf@gmx.at, Jason_Livingood@comcast.com,
	vidhi_goel@apple.com
Subject: Re: [PATCH v1 net-next 3/3] virtio_net: Accurate ECN flag in virtio_net_hdr
Date: Sun, 1 Feb 2026 04:17:35 -0500	[thread overview]
Message-ID: <20260201035912-mutt-send-email-mst@kernel.org> (raw)
In-Reply-To: <20260131225510.2946-4-chia-yu.chang@nokia-bell-labs.com>

Thanks for the patch! Yet something to improve:

On Sat, Jan 31, 2026 at 11:55:10PM +0100, chia-yu.chang@nokia-bell-labs.com wrote:
> From: Chia-Yu Chang <chia-yu.chang@nokia-bell-labs.com>
> 
> Unlike RFC 3168 ECN, accurate ECN uses the CWR flag as part of the ACE
> field to count new packets with CE mark; however, it will be corrupted
> by the RFC 3168 ECN-aware TSO. Therefore, fallback shall be applied by
> seting NETIF_F_GSO_ACCECN to ensure that the CWR flag should not be
> changed within a super-skb.
> 
> To apply the aforementieond new AccECN GSO for virtio, new featue bits
> for host and guest are added for feature negotiation between driver and
> device. And the translation of Accurate ECN GSO flag between
> virtio_net_hdr and skb header for NETIF_F_GSO_ACCECN is also added to
> avoid CWR flag corruption due to RFC3168 ECN TSO.
> 
> Signed-off-by: Chia-Yu Chang <chia-yu.chang@nokia-bell-labs.com>


To the best of my understanding, this is a new feature - support
for VIRTIO_NET_F_HOST_ACCECN, VIRTIO_NET_F_GUEST_ACCECN?
The commit log makes it sound like it fixes some behaviour for
existing hardware, but that is not the case.





> ---
> v2:
> - Replace VIRTIO_NET_HDR_GSO_ECN with VIRTIO_NET_HDR_GSO_ECN_FLAGS

but where is v2? this is v1...

> ---
>  drivers/net/virtio_net.c        | 14 +++++++++++---
>  drivers/vdpa/pds/debugfs.c      |  6 ++++++
>  include/linux/virtio_net.h      | 18 +++++++++++-------
>  include/uapi/linux/virtio_net.h |  5 +++++
>  4 files changed, 33 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index db88dcaefb20..103fb87c690e 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -75,6 +75,7 @@ static const unsigned long guest_offloads[] = {
>  	VIRTIO_NET_F_GUEST_TSO4,
>  	VIRTIO_NET_F_GUEST_TSO6,
>  	VIRTIO_NET_F_GUEST_ECN,
> +	VIRTIO_NET_F_GUEST_ACCECN,
>  	VIRTIO_NET_F_GUEST_UFO,
>  	VIRTIO_NET_F_GUEST_CSUM,
>  	VIRTIO_NET_F_GUEST_USO4,
> @@ -87,6 +88,7 @@ static const unsigned long guest_offloads[] = {
>  #define GUEST_OFFLOAD_GRO_HW_MASK ((1ULL << VIRTIO_NET_F_GUEST_TSO4) | \
>  			(1ULL << VIRTIO_NET_F_GUEST_TSO6) | \
>  			(1ULL << VIRTIO_NET_F_GUEST_ECN)  | \
> +			(1ULL << VIRTIO_NET_F_GUEST_ACCECN) | \
>  			(1ULL << VIRTIO_NET_F_GUEST_UFO)  | \
>  			(1ULL << VIRTIO_NET_F_GUEST_USO4) | \
>  			(1ULL << VIRTIO_NET_F_GUEST_USO6) | \
> @@ -5976,6 +5978,7 @@ static int virtnet_xdp_set(struct net_device *dev, struct bpf_prog *prog,
>  	    && (virtio_has_feature(vi->vdev, VIRTIO_NET_F_GUEST_TSO4) ||
>  	        virtio_has_feature(vi->vdev, VIRTIO_NET_F_GUEST_TSO6) ||
>  	        virtio_has_feature(vi->vdev, VIRTIO_NET_F_GUEST_ECN) ||
> +		virtio_has_feature(vi->vdev, VIRTIO_NET_F_GUEST_ACCECN) ||
>  		virtio_has_feature(vi->vdev, VIRTIO_NET_F_GUEST_UFO) ||
>  		virtio_has_feature(vi->vdev, VIRTIO_NET_F_GUEST_CSUM) ||
>  		virtio_has_feature(vi->vdev, VIRTIO_NET_F_GUEST_USO4) ||
> @@ -6635,6 +6638,7 @@ static bool virtnet_check_guest_gso(const struct virtnet_info *vi)
>  	return virtio_has_feature(vi->vdev, VIRTIO_NET_F_GUEST_TSO4) ||
>  		virtio_has_feature(vi->vdev, VIRTIO_NET_F_GUEST_TSO6) ||
>  		virtio_has_feature(vi->vdev, VIRTIO_NET_F_GUEST_ECN) ||
> +		virtio_has_feature(vi->vdev, VIRTIO_NET_F_GUEST_ACCECN) ||
>  		virtio_has_feature(vi->vdev, VIRTIO_NET_F_GUEST_UFO) ||
>  		(virtio_has_feature(vi->vdev, VIRTIO_NET_F_GUEST_USO4) &&
>  		virtio_has_feature(vi->vdev, VIRTIO_NET_F_GUEST_USO6));
> @@ -6749,6 +6753,8 @@ static int virtnet_probe(struct virtio_device *vdev)
>  			dev->hw_features |= NETIF_F_TSO6;
>  		if (virtio_has_feature(vdev, VIRTIO_NET_F_HOST_ECN))
>  			dev->hw_features |= NETIF_F_TSO_ECN;
> +		if (virtio_has_feature(vdev, VIRTIO_NET_F_HOST_ACCECN))
> +			dev->hw_features |= NETIF_F_GSO_ACCECN;
>  		if (virtio_has_feature(vdev, VIRTIO_NET_F_HOST_USO))
>  			dev->hw_features |= NETIF_F_GSO_UDP_L4;
>  
> @@ -7169,9 +7175,11 @@ static struct virtio_device_id id_table[] = {
>  	VIRTIO_NET_F_CSUM, VIRTIO_NET_F_GUEST_CSUM, \
>  	VIRTIO_NET_F_MAC, \
>  	VIRTIO_NET_F_HOST_TSO4, VIRTIO_NET_F_HOST_UFO, VIRTIO_NET_F_HOST_TSO6, \
> -	VIRTIO_NET_F_HOST_ECN, VIRTIO_NET_F_GUEST_TSO4, VIRTIO_NET_F_GUEST_TSO6, \
> -	VIRTIO_NET_F_GUEST_ECN, VIRTIO_NET_F_GUEST_UFO, \
> -	VIRTIO_NET_F_HOST_USO, VIRTIO_NET_F_GUEST_USO4, VIRTIO_NET_F_GUEST_USO6, \
> +	VIRTIO_NET_F_HOST_ECN, VIRTIO_NET_F_HOST_ACCECN, \
> +	VIRTIO_NET_F_GUEST_TSO4, VIRTIO_NET_F_GUEST_TSO6, \
> +	VIRTIO_NET_F_GUEST_ECN, VIRTIO_NET_F_GUEST_ACCECN, \
> +	VIRTIO_NET_F_GUEST_UFO, VIRTIO_NET_F_HOST_USO, \
> +	VIRTIO_NET_F_GUEST_USO4, VIRTIO_NET_F_GUEST_USO6, \
>  	VIRTIO_NET_F_MRG_RXBUF, VIRTIO_NET_F_STATUS, VIRTIO_NET_F_CTRL_VQ, \
>  	VIRTIO_NET_F_CTRL_RX, VIRTIO_NET_F_CTRL_VLAN, \
>  	VIRTIO_NET_F_GUEST_ANNOUNCE, VIRTIO_NET_F_MQ, \
> diff --git a/drivers/vdpa/pds/debugfs.c b/drivers/vdpa/pds/debugfs.c
> index c328e694f6e7..90bd95db0245 100644
> --- a/drivers/vdpa/pds/debugfs.c
> +++ b/drivers/vdpa/pds/debugfs.c
> @@ -78,6 +78,9 @@ static void print_feature_bits_all(struct seq_file *seq, u64 features)
>  		case BIT_ULL(VIRTIO_NET_F_GUEST_ECN):
>  			seq_puts(seq, " VIRTIO_NET_F_GUEST_ECN");
>  			break;
> +		case BIT_ULL(VIRTIO_NET_F_GUEST_ACCECN):
> +			seq_puts(seq, " VIRTIO_NET_F_GUEST_ACCECN");
> +			break;
>  		case BIT_ULL(VIRTIO_NET_F_GUEST_UFO):
>  			seq_puts(seq, " VIRTIO_NET_F_GUEST_UFO");
>  			break;
> @@ -90,6 +93,9 @@ static void print_feature_bits_all(struct seq_file *seq, u64 features)
>  		case BIT_ULL(VIRTIO_NET_F_HOST_ECN):
>  			seq_puts(seq, " VIRTIO_NET_F_HOST_ECN");
>  			break;
> +		case BIT_ULL(VIRTIO_NET_F_HOST_ACCECN):
> +			seq_puts(seq, " VIRTIO_NET_F_HOST_ACCECN");
> +			break;
>  		case BIT_ULL(VIRTIO_NET_F_HOST_UFO):
>  			seq_puts(seq, " VIRTIO_NET_F_HOST_UFO");
>  			break;
> diff --git a/include/linux/virtio_net.h b/include/linux/virtio_net.h
> index 75dabb763c65..0cf86b026828 100644
> --- a/include/linux/virtio_net.h
> +++ b/include/linux/virtio_net.h
> @@ -11,7 +11,7 @@
>  
>  static inline bool virtio_net_hdr_match_proto(__be16 protocol, __u8 gso_type)
>  {
> -	switch (gso_type & ~VIRTIO_NET_HDR_GSO_ECN) {
> +	switch (gso_type & ~VIRTIO_NET_HDR_GSO_ECN_FLAGS) {
>  	case VIRTIO_NET_HDR_GSO_TCPV4:
>  		return protocol == cpu_to_be16(ETH_P_IP);
>  	case VIRTIO_NET_HDR_GSO_TCPV6:
> @@ -31,7 +31,7 @@ static inline int virtio_net_hdr_set_proto(struct sk_buff *skb,
>  	if (skb->protocol)
>  		return 0;
>  
> -	switch (hdr->gso_type & ~VIRTIO_NET_HDR_GSO_ECN) {
> +	switch (hdr->gso_type & ~VIRTIO_NET_HDR_GSO_ECN_FLAGS) {
>  	case VIRTIO_NET_HDR_GSO_TCPV4:
>  	case VIRTIO_NET_HDR_GSO_UDP:
>  	case VIRTIO_NET_HDR_GSO_UDP_L4:
> @@ -58,7 +58,7 @@ static inline int __virtio_net_hdr_to_skb(struct sk_buff *skb,
>  	unsigned int ip_proto;
>  
>  	if (hdr_gso_type != VIRTIO_NET_HDR_GSO_NONE) {
> -		switch (hdr_gso_type & ~VIRTIO_NET_HDR_GSO_ECN) {
> +		switch (hdr_gso_type & ~VIRTIO_NET_HDR_GSO_ECN_FLAGS) {
>  		case VIRTIO_NET_HDR_GSO_TCPV4:
>  			gso_type = SKB_GSO_TCPV4;
>  			ip_proto = IPPROTO_TCP;
> @@ -84,7 +84,9 @@ static inline int __virtio_net_hdr_to_skb(struct sk_buff *skb,
>  			return -EINVAL;
>  		}
>  
> -		if (hdr_gso_type & VIRTIO_NET_HDR_GSO_ECN)
> +		if (hdr_gso_type & VIRTIO_NET_HDR_GSO_ACCECN)
> +			gso_type |= SKB_GSO_TCP_ACCECN;
> +		else if (hdr_gso_type & VIRTIO_NET_HDR_GSO_ECN)
>  			gso_type |= SKB_GSO_TCP_ECN;
>  
>  		if (hdr->gso_size == 0)
> @@ -159,7 +161,7 @@ static inline int __virtio_net_hdr_to_skb(struct sk_buff *skb,
>  		unsigned int nh_off = p_off;
>  		struct skb_shared_info *shinfo = skb_shinfo(skb);
>  
> -		switch (gso_type & ~SKB_GSO_TCP_ECN) {
> +		switch (gso_type & ~(SKB_GSO_TCP_ECN | SKB_GSO_TCP_ACCECN)) {
>  		case SKB_GSO_UDP:
>  			/* UFO may not include transport header in gso_size. */
>  			nh_off -= thlen;
> @@ -231,7 +233,9 @@ static inline int virtio_net_hdr_from_skb(const struct sk_buff *skb,
>  			hdr->gso_type = VIRTIO_NET_HDR_GSO_UDP_L4;
>  		else
>  			return -EINVAL;
> -		if (sinfo->gso_type & SKB_GSO_TCP_ECN)
> +		if (sinfo->gso_type & SKB_GSO_TCP_ACCECN)
> +			hdr->gso_type |= VIRTIO_NET_HDR_GSO_ACCECN;
> +		else if (sinfo->gso_type & SKB_GSO_TCP_ECN)
>  			hdr->gso_type |= VIRTIO_NET_HDR_GSO_ECN;
>  	} else
>  		hdr->gso_type = VIRTIO_NET_HDR_GSO_NONE;
> @@ -282,7 +286,7 @@ virtio_net_hdr_tnl_to_skb(struct sk_buff *skb,
>  		return -EINVAL;
>  
>  	/* The UDP tunnel must carry a GSO packet, but no UFO. */
> -	gso_inner_type = hdr->gso_type & ~(VIRTIO_NET_HDR_GSO_ECN |
> +	gso_inner_type = hdr->gso_type & ~(VIRTIO_NET_HDR_GSO_ECN_FLAGS |
>  					   VIRTIO_NET_HDR_GSO_UDP_TUNNEL);
>  	if (!gso_inner_type || gso_inner_type == VIRTIO_NET_HDR_GSO_UDP)
>  		return -EINVAL;
> diff --git a/include/uapi/linux/virtio_net.h b/include/uapi/linux/virtio_net.h
> index 1db45b01532b..af5bfe45aa1f 100644
> --- a/include/uapi/linux/virtio_net.h
> +++ b/include/uapi/linux/virtio_net.h
> @@ -56,6 +56,8 @@
>  #define VIRTIO_NET_F_MQ	22	/* Device supports Receive Flow
>  					 * Steering */
>  #define VIRTIO_NET_F_CTRL_MAC_ADDR 23	/* Set MAC address */
> +#define VIRTIO_NET_F_HOST_ACCECN 25	/* Host can handle GSO of AccECN */
> +#define VIRTIO_NET_F_GUEST_ACCECN 26	/* Guest can handle GSO of AccECN */
>  #define VIRTIO_NET_F_DEVICE_STATS 50	/* Device can provide device-level statistics. */
>  #define VIRTIO_NET_F_VQ_NOTF_COAL 52	/* Device supports virtqueue notification coalescing */
>  #define VIRTIO_NET_F_NOTF_COAL	53	/* Device supports notifications coalescing */
> @@ -165,6 +167,9 @@ struct virtio_net_hdr_v1 {
>  #define VIRTIO_NET_HDR_GSO_UDP_TUNNEL (VIRTIO_NET_HDR_GSO_UDP_TUNNEL_IPV4 | \
>  				       VIRTIO_NET_HDR_GSO_UDP_TUNNEL_IPV6)
>  #define VIRTIO_NET_HDR_GSO_ECN		0x80	/* TCP has ECN set */
> +#define VIRTIO_NET_HDR_GSO_ACCECN	0x10	/* TCP AccECN segmentation */
> +#define VIRTIO_NET_HDR_GSO_ECN_FLAGS	(VIRTIO_NET_HDR_GSO_ECN | \
> +					 VIRTIO_NET_HDR_GSO_ACCECN)
>  	__u8 gso_type;
>  	__virtio16 hdr_len;	/* Ethernet + IP + tcp/udp hdrs */
>  	__virtio16 gso_size;	/* Bytes to append to hdr_len per frame */


UAPI changes need to be added to the virtio spec.
Pls get this approved by the virtio TC.
Thanks!


> -- 
> 2.34.1


  reply	other threads:[~2026-02-01  9:17 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-01-31 22:55 [PATCH v1 net-next 0/3] ECN offload handling for AccECN series chia-yu.chang
2026-01-31 22:55 ` [PATCH v1 net-next 1/3] net: update commnets for SKB_GSO_TCP_ECN and SKB_GSO_TCP_ACCECN chia-yu.chang
2026-02-01  9:01   ` Michael S. Tsirkin
2026-01-31 22:55 ` [PATCH v1 net-next 2/3] net: hns3/mlx5e: avoid corrupting CWR flag when receiving GRO packet chia-yu.chang
2026-02-01  9:05   ` Michael S. Tsirkin
2026-02-02 16:44     ` Chia-Yu Chang (Nokia)
2026-01-31 22:55 ` [PATCH v1 net-next 3/3] virtio_net: Accurate ECN flag in virtio_net_hdr chia-yu.chang
2026-02-01  9:17   ` Michael S. Tsirkin [this message]
2026-02-02 16:56     ` Chia-Yu Chang (Nokia)
2026-02-02 17:19       ` Michael S. Tsirkin
2026-02-02 21:09         ` Chia-Yu Chang (Nokia)
2026-02-02 23:45           ` Michael S. Tsirkin
2026-02-01  0:45 ` [PATCH v1 net-next 0/3] ECN offload handling for AccECN series Jakub Kicinski
2026-02-02 14:10   ` Chia-Yu Chang (Nokia)
  -- strict thread matches above, loose matches on Subject: below --
2026-01-28 14:44 chia-yu.chang
2026-01-28 14:44 ` [PATCH v1 net-next 3/3] virtio_net: Accurate ECN flag in virtio_net_hdr chia-yu.chang

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260201035912-mutt-send-email-mst@kernel.org \
    --to=mst@redhat.com \
    --cc=Jason_Livingood@comcast.com \
    --cc=andrew+netdev@lunn.ch \
    --cc=ast@fiberby.net \
    --cc=bpf@vger.kernel.org \
    --cc=brett.creeley@amd.com \
    --cc=cheshire@apple.com \
    --cc=chia-yu.chang@nokia-bell-labs.com \
    --cc=corbet@lwn.net \
    --cc=dave.taht@gmail.com \
    --cc=davem@davemloft.net \
    --cc=donald.hunter@gmail.com \
    --cc=dsahern@kernel.org \
    --cc=edumazet@google.com \
    --cc=eperezma@redhat.com \
    --cc=g.white@cablelabs.com \
    --cc=horms@kernel.org \
    --cc=ij@kernel.org \
    --cc=ingemar.s.johansson@ericsson.com \
    --cc=jasowang@redhat.com \
    --cc=jhs@mojatatu.com \
    --cc=jiri@resnulli.us \
    --cc=koen.de_schepper@nokia-bell-labs.com \
    --cc=kuba@kernel.org \
    --cc=kuniyu@google.com \
    --cc=leon@kernel.org \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-kselftest@vger.kernel.org \
    --cc=linux-rdma@vger.kernel.org \
    --cc=liuhangbin@gmail.com \
    --cc=mbloch@nvidia.com \
    --cc=mirja.kuehlewind@ericsson.com \
    --cc=ncardwell@google.com \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=parav@nvidia.com \
    --cc=rs.ietf@gmx.at \
    --cc=saeedm@nvidia.com \
    --cc=salil.mehta@huawei.com \
    --cc=shaojijie@huawei.com \
    --cc=shenjian15@huawei.com \
    --cc=shuah@kernel.org \
    --cc=stephen@networkplumber.org \
    --cc=tariqt@nvidia.com \
    --cc=vidhi_goel@apple.com \
    --cc=virtualization@lists.linux.dev \
    --cc=xiyou.wangcong@gmail.com \
    --cc=xuanzhuo@linux.alibaba.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.