public inbox for netdev@vger.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox