Re: [PATCH v3 3/4] nvmet-tcp: support IPv6 traffic class

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The nvmet_tcp_sock_set_tclass function attempts to configure the IPv6 Traffic Class by passing a 1-byte length to do_sock_setsockopt. However, do_ipv6_setsockopt rigidly requires the option length to be at least 4 bytes for IPV6_TCLASS. As a result, the operation fails with -EINVAL, which is silently ignored, and the traffic class is never applied.
--

--- Patch [3]: [PATCH v3 3/4] nvmet-tcp: support IPv6 traffic class ---
commit 97f9471c35e4e1cd4f5b7c6f1f3a6b3f8e13683a
Author: Geliang Tang <[email protected]>

nvmet-tcp: support IPv6 traffic class

Currently, nvmet-tcp only applies the received IPv4 TOS value when setting
up a queue socket, but does not handle the IPv6 traffic class. This patch
extends the queue socket setup to handle AF_INET6 sockets.

> diff --git a/drivers/nvme/target/tcp.c b/drivers/nvme/target/tcp.c
> index e64592b792577..d34249cf37564 100644
> --- a/drivers/nvme/target/tcp.c
> +++ b/drivers/nvme/target/tcp.c
> @@ -1735,6 +1735,20 @@ static void nvmet_tcp_sock_set_tos(struct sock *sk)
>  				   KERNEL_SOCKPTR(&tos), sizeof(tos));
>  }
>  
> +static void nvmet_tcp_sock_set_tclass(struct sock *sk)
> +{
> +#if IS_ENABLED(CONFIG_IPV6)
> +	if (sk->sk_family == AF_INET6) {
> +		u8 tclass = ip6_tclass(inet6_sk(sk)->rcv_flowinfo);
> +
> +		if (tclass > 0)
> +			do_sock_setsockopt(sk->sk_socket, false, SOL_IPV6,
> +					   IPV6_TCLASS, KERNEL_SOCKPTR(&tclass),
> +					   sizeof(tclass));
[Severity: Medium]
Will this silently fail to set the traffic class?

Looking at do_ipv6_setsockopt() in net/ipv6/ipv6_sockglue.c, the IPV6_TCLASS
handler explicitly requires a minimum optlen of sizeof(int):

    case IPV6_TCLASS:
        if (optlen < sizeof(int))
            goto e_inval;

Since tclass is a u8, sizeof(tclass) is 1. This causes the internal
setsockopt implementation to reject the option with -EINVAL, leaving the
traffic class unset.

Would it be better to store the traffic class in an int variable before
passing it to do_sock_setsockopt()?

> +	}
> +#endif
> +}
> +

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=3