* [PATCH net] udp: Pass 2 bytes of data with UDP_GRO cmsg to user-space
@ 2023-01-31 17:46 Jakub Sitnicki
2023-01-31 17:57 ` Eric Dumazet
0 siblings, 1 reply; 3+ messages in thread
From: Jakub Sitnicki @ 2023-01-31 17:46 UTC (permalink / raw)
To: netdev
Cc: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
kernel-team
While UDP_GRO cmsg interface lacks documentation, the selftests added in
commit 3327a9c46352 ("selftests: add functionals test for UDP GRO") suggest
that the user-space should allocate CMSG_SPACE for an u16 value and
interpret the returned bytes as such:
static int recv_msg(int fd, char *buf, int len, int *gso_size)
{
char control[CMSG_SPACE(sizeof(uint16_t))] = {0};
...
if (cmsg->cmsg_level == SOL_UDP
&& cmsg->cmsg_type == UDP_GRO) {
gsosizeptr = (uint16_t *) CMSG_DATA(cmsg);
*gso_size = *gsosizeptr;
break;
}
...
}
Today user-space will receive 4 bytes of data with an UDP_GRO cmsg, because
the kernel packs an int into the cmsg data, as we can confirm with strace:
recvmsg(8, {msg_name=...,
msg_iov=[{iov_base="\0\0..."..., iov_len=96000}],
msg_iovlen=1,
msg_control=[{cmsg_len=20, <-- sizeof(cmsghdr) + 4
cmsg_level=SOL_UDP,
cmsg_type=0x68}], <-- UDP_GRO
msg_controllen=24,
msg_flags=0}, 0) = 11200
This means that either UDP_GRO selftests are broken on big endian, or this
is a programming error. Assume the latter and pass only the needed 2 bytes
of data with the cmsg.
Fixing it like that has an added advantage that the cmsg becomes compatible
with what is expected by UDP_SEGMENT cmsg. It becomes possible to reuse the
cmsg when GSO packets are received on one socket and sent out of another.
Fixes: bcd1665e3569 ("udp: add support for UDP_GRO cmsg")
Signed-off-by: Jakub Sitnicki <jakub@cloudflare.com>
---
include/linux/udp.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/linux/udp.h b/include/linux/udp.h
index a2892e151644..44bb8d699248 100644
--- a/include/linux/udp.h
+++ b/include/linux/udp.h
@@ -125,7 +125,7 @@ static inline bool udp_get_no_check6_rx(struct sock *sk)
static inline void udp_cmsg_recv(struct msghdr *msg, struct sock *sk,
struct sk_buff *skb)
{
- int gso_size;
+ __u16 gso_size;
if (skb_shinfo(skb)->gso_type & SKB_GSO_UDP_L4) {
gso_size = skb_shinfo(skb)->gso_size;
--
2.39.1
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH net] udp: Pass 2 bytes of data with UDP_GRO cmsg to user-space
2023-01-31 17:46 [PATCH net] udp: Pass 2 bytes of data with UDP_GRO cmsg to user-space Jakub Sitnicki
@ 2023-01-31 17:57 ` Eric Dumazet
2023-02-06 10:41 ` Jakub Sitnicki
0 siblings, 1 reply; 3+ messages in thread
From: Eric Dumazet @ 2023-01-31 17:57 UTC (permalink / raw)
To: Jakub Sitnicki
Cc: netdev, David S. Miller, Jakub Kicinski, Paolo Abeni, kernel-team
On Tue, Jan 31, 2023 at 6:46 PM Jakub Sitnicki <jakub@cloudflare.com> wrote:
>
> While UDP_GRO cmsg interface lacks documentation, the selftests added in
> commit 3327a9c46352 ("selftests: add functionals test for UDP GRO") suggest
> that the user-space should allocate CMSG_SPACE for an u16 value and
> interpret the returned bytes as such:
>
> static int recv_msg(int fd, char *buf, int len, int *gso_size)
> {
> char control[CMSG_SPACE(sizeof(uint16_t))] = {0};
> ...
> if (cmsg->cmsg_level == SOL_UDP
> && cmsg->cmsg_type == UDP_GRO) {
> gsosizeptr = (uint16_t *) CMSG_DATA(cmsg);
> *gso_size = *gsosizeptr;
> break;
> }
> ...
> }
>
> Today user-space will receive 4 bytes of data with an UDP_GRO cmsg, because
> the kernel packs an int into the cmsg data, as we can confirm with strace:
>
> recvmsg(8, {msg_name=...,
> msg_iov=[{iov_base="\0\0..."..., iov_len=96000}],
> msg_iovlen=1,
> msg_control=[{cmsg_len=20, <-- sizeof(cmsghdr) + 4
> cmsg_level=SOL_UDP,
> cmsg_type=0x68}], <-- UDP_GRO
> msg_controllen=24,
> msg_flags=0}, 0) = 11200
>
> This means that either UDP_GRO selftests are broken on big endian, or this
> is a programming error. Assume the latter and pass only the needed 2 bytes
> of data with the cmsg.
>
> Fixing it like that has an added advantage that the cmsg becomes compatible
> with what is expected by UDP_SEGMENT cmsg. It becomes possible to reuse the
> cmsg when GSO packets are received on one socket and sent out of another.
>
> Fixes: bcd1665e3569 ("udp: add support for UDP_GRO cmsg")
> Signed-off-by: Jakub Sitnicki <jakub@cloudflare.com>
> ---
> include/linux/udp.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/include/linux/udp.h b/include/linux/udp.h
> index a2892e151644..44bb8d699248 100644
> --- a/include/linux/udp.h
> +++ b/include/linux/udp.h
> @@ -125,7 +125,7 @@ static inline bool udp_get_no_check6_rx(struct sock *sk)
> static inline void udp_cmsg_recv(struct msghdr *msg, struct sock *sk,
> struct sk_buff *skb)
> {
> - int gso_size;
> + __u16 gso_size;
>
> if (skb_shinfo(skb)->gso_type & SKB_GSO_UDP_L4) {
> gso_size = skb_shinfo(skb)->gso_size;
> --
> 2.39.1
>
This would break some applications.
I think the test can be fixed instead, this seems less risky.
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH net] udp: Pass 2 bytes of data with UDP_GRO cmsg to user-space
2023-01-31 17:57 ` Eric Dumazet
@ 2023-02-06 10:41 ` Jakub Sitnicki
0 siblings, 0 replies; 3+ messages in thread
From: Jakub Sitnicki @ 2023-02-06 10:41 UTC (permalink / raw)
To: Eric Dumazet
Cc: netdev, David S. Miller, Jakub Kicinski, Paolo Abeni, kernel-team
On Tue, Jan 31, 2023 at 06:57 PM +01, Eric Dumazet wrote:
> On Tue, Jan 31, 2023 at 6:46 PM Jakub Sitnicki <jakub@cloudflare.com> wrote:
>>
>> While UDP_GRO cmsg interface lacks documentation, the selftests added in
>> commit 3327a9c46352 ("selftests: add functionals test for UDP GRO") suggest
>> that the user-space should allocate CMSG_SPACE for an u16 value and
>> interpret the returned bytes as such:
>>
>> static int recv_msg(int fd, char *buf, int len, int *gso_size)
>> {
>> char control[CMSG_SPACE(sizeof(uint16_t))] = {0};
>> ...
>> if (cmsg->cmsg_level == SOL_UDP
>> && cmsg->cmsg_type == UDP_GRO) {
>> gsosizeptr = (uint16_t *) CMSG_DATA(cmsg);
>> *gso_size = *gsosizeptr;
>> break;
>> }
>> ...
>> }
>>
>> Today user-space will receive 4 bytes of data with an UDP_GRO cmsg, because
>> the kernel packs an int into the cmsg data, as we can confirm with strace:
>>
>> recvmsg(8, {msg_name=...,
>> msg_iov=[{iov_base="\0\0..."..., iov_len=96000}],
>> msg_iovlen=1,
>> msg_control=[{cmsg_len=20, <-- sizeof(cmsghdr) + 4
>> cmsg_level=SOL_UDP,
>> cmsg_type=0x68}], <-- UDP_GRO
>> msg_controllen=24,
>> msg_flags=0}, 0) = 11200
>>
>> This means that either UDP_GRO selftests are broken on big endian, or this
>> is a programming error. Assume the latter and pass only the needed 2 bytes
>> of data with the cmsg.
>>
>> Fixing it like that has an added advantage that the cmsg becomes compatible
>> with what is expected by UDP_SEGMENT cmsg. It becomes possible to reuse the
>> cmsg when GSO packets are received on one socket and sent out of another.
>>
>> Fixes: bcd1665e3569 ("udp: add support for UDP_GRO cmsg")
>> Signed-off-by: Jakub Sitnicki <jakub@cloudflare.com>
>> ---
>> include/linux/udp.h | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/include/linux/udp.h b/include/linux/udp.h
>> index a2892e151644..44bb8d699248 100644
>> --- a/include/linux/udp.h
>> +++ b/include/linux/udp.h
>> @@ -125,7 +125,7 @@ static inline bool udp_get_no_check6_rx(struct sock *sk)
>> static inline void udp_cmsg_recv(struct msghdr *msg, struct sock *sk,
>> struct sk_buff *skb)
>> {
>> - int gso_size;
>> + __u16 gso_size;
>>
>> if (skb_shinfo(skb)->gso_type & SKB_GSO_UDP_L4) {
>> gso_size = skb_shinfo(skb)->gso_size;
>> --
>> 2.39.1
>>
>
> This would break some applications.
>
> I think the test can be fixed instead, this seems less risky.
Thanks for guidance. Will fix the test.
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2023-02-06 10:42 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-01-31 17:46 [PATCH net] udp: Pass 2 bytes of data with UDP_GRO cmsg to user-space Jakub Sitnicki
2023-01-31 17:57 ` Eric Dumazet
2023-02-06 10:41 ` Jakub Sitnicki
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).