[PATCH 0/3] PROTO_CMSG_DATA_ONLY for Datagram (UDP)

[PATCH 0/3] PROTO_CMSG_DATA_ONLY for Datagram (UDP)

[Victor Stewart]

RE our conversation on the "[RFC 0/1] whitelisting UDP GSO and GRO
cmsgs" thread...

https://lore.kernel.org/io-uring/CAM1kxwi5m6i8hrtkw7nZYoziPTD-Wp03+fcsUwh3CuSc=81kUQ@mail.gmail.com/

here are the patches we discussed.

Victor Stewart (3):
   net/socket.c: add PROTO_CMSG_DATA_ONLY to __sys_sendmsg_sock
   net/ipv4/af_inet.c: add PROTO_CMSG_DATA_ONLY to inet_dgram_ops
   net/ipv6/af_inet6.c: add PROTO_CMSG_DATA_ONLY to inet6_dgram_ops

   net/ipv4/af_inet.c
     |   1 +
   net/ipv6/af_inet6.c
    |   1 +
   net/socket.c
       |   8 +-
   3 files changed, 7 insertions(+), 3 deletions(-)

Re: [PATCH 0/3] PROTO_CMSG_DATA_ONLY for Datagram (UDP)

[Jens Axboe]

On 12/12/20 8:31 AM, Victor Stewart wrote:
> RE our conversation on the "[RFC 0/1] whitelisting UDP GSO and GRO
> cmsgs" thread...
> 
> https://lore.kernel.org/io-uring/CAM1kxwi5m6i8hrtkw7nZYoziPTD-Wp03+fcsUwh3CuSc=81kUQ@mail.gmail.com/
> 
> here are the patches we discussed.
> 
> Victor Stewart (3):
>    net/socket.c: add PROTO_CMSG_DATA_ONLY to __sys_sendmsg_sock
>    net/ipv4/af_inet.c: add PROTO_CMSG_DATA_ONLY to inet_dgram_ops
>    net/ipv6/af_inet6.c: add PROTO_CMSG_DATA_ONLY to inet6_dgram_ops
> 
>    net/ipv4/af_inet.c
>      |   1 +
>    net/ipv6/af_inet6.c
>     |   1 +
>    net/socket.c
>        |   8 +-
>    3 files changed, 7 insertions(+), 3 deletions(-)

Changes look fine to me, but a few comments:

- I'd order 1/3 as 3/3, that ordering makes more sense as at that point it
  could actually be used.

- For adding it to af_inet/af_inet6, you should write a better commit message
  on the reasoning for the change. Right now it just describes what the
  patch does (which is obvious from the change), not WHY it's done. Really
  goes for current 1/3 as well, commit messages need to be better in
  general.

I'd also CC Jann Horn on the series, he's the one that found an issue there
in the past and also acked the previous change on doing PROTO_CMSG_DATA_ONLY.

-- 
Jens Axboe

Re: [PATCH 0/3] PROTO_CMSG_DATA_ONLY for Datagram (UDP)

[Victor Stewart]

On Sat, Dec 12, 2020 at 5:07 PM Jens Axboe <axboe@kernel.dk> wrote:
>
> On 12/12/20 8:31 AM, Victor Stewart wrote:
> > RE our conversation on the "[RFC 0/1] whitelisting UDP GSO and GRO
> > cmsgs" thread...
> >
> > https://lore.kernel.org/io-uring/CAM1kxwi5m6i8hrtkw7nZYoziPTD-Wp03+fcsUwh3CuSc=81kUQ@mail.gmail.com/
> >
> > here are the patches we discussed.
> >
> > Victor Stewart (3):
> >    net/socket.c: add PROTO_CMSG_DATA_ONLY to __sys_sendmsg_sock
> >    net/ipv4/af_inet.c: add PROTO_CMSG_DATA_ONLY to inet_dgram_ops
> >    net/ipv6/af_inet6.c: add PROTO_CMSG_DATA_ONLY to inet6_dgram_ops
> >
> >    net/ipv4/af_inet.c
> >      |   1 +
> >    net/ipv6/af_inet6.c
> >     |   1 +
> >    net/socket.c
> >        |   8 +-
> >    3 files changed, 7 insertions(+), 3 deletions(-)
>
> Changes look fine to me, but a few comments:
>
> - I'd order 1/3 as 3/3, that ordering makes more sense as at that point it
>   could actually be used.

right that makes sense.

>
> - For adding it to af_inet/af_inet6, you should write a better commit message
>   on the reasoning for the change. Right now it just describes what the
>   patch does (which is obvious from the change), not WHY it's done. Really
>   goes for current 1/3 as well, commit messages need to be better in
>   general.
>

okay thanks Jens. i would have reiterated the intention but assumed it
were implicit given I linked the initial conversation about enabling
UDP_SEGMENT (GSO) and UDP_GRO through io_uring.

> I'd also CC Jann Horn on the series, he's the one that found an issue there
> in the past and also acked the previous change on doing PROTO_CMSG_DATA_ONLY.

I CCed him on this reply. Soheil at the end of the first exchange
thread said he audited the UDP paths and believed this to be safe.

how/should I resubmit the patch with a proper intention explanation in
the meta and reorder the patches? my first patch and all lol.

>
> --
> Jens Axboe
>

Re: [PATCH 0/3] PROTO_CMSG_DATA_ONLY for Datagram (UDP)

[Jens Axboe]

On 12/12/20 10:25 AM, Victor Stewart wrote:
> On Sat, Dec 12, 2020 at 5:07 PM Jens Axboe <axboe@kernel.dk> wrote:
>>
>> On 12/12/20 8:31 AM, Victor Stewart wrote:
>>> RE our conversation on the "[RFC 0/1] whitelisting UDP GSO and GRO
>>> cmsgs" thread...
>>>
>>> https://lore.kernel.org/io-uring/CAM1kxwi5m6i8hrtkw7nZYoziPTD-Wp03+fcsUwh3CuSc=81kUQ@mail.gmail.com/
>>>
>>> here are the patches we discussed.
>>>
>>> Victor Stewart (3):
>>>    net/socket.c: add PROTO_CMSG_DATA_ONLY to __sys_sendmsg_sock
>>>    net/ipv4/af_inet.c: add PROTO_CMSG_DATA_ONLY to inet_dgram_ops
>>>    net/ipv6/af_inet6.c: add PROTO_CMSG_DATA_ONLY to inet6_dgram_ops
>>>
>>>    net/ipv4/af_inet.c
>>>      |   1 +
>>>    net/ipv6/af_inet6.c
>>>     |   1 +
>>>    net/socket.c
>>>        |   8 +-
>>>    3 files changed, 7 insertions(+), 3 deletions(-)
>>
>> Changes look fine to me, but a few comments:
>>
>> - I'd order 1/3 as 3/3, that ordering makes more sense as at that point it
>>   could actually be used.
> 
> right that makes sense.
> 
>>
>> - For adding it to af_inet/af_inet6, you should write a better commit message
>>   on the reasoning for the change. Right now it just describes what the
>>   patch does (which is obvious from the change), not WHY it's done. Really
>>   goes for current 1/3 as well, commit messages need to be better in
>>   general.
>>
> 
> okay thanks Jens. i would have reiterated the intention but assumed it
> were implicit given I linked the initial conversation about enabling
> UDP_SEGMENT (GSO) and UDP_GRO through io_uring.
> 
>> I'd also CC Jann Horn on the series, he's the one that found an issue there
>> in the past and also acked the previous change on doing PROTO_CMSG_DATA_ONLY.
> 
> I CCed him on this reply. Soheil at the end of the first exchange
> thread said he audited the UDP paths and believed this to be safe.
> 
> how/should I resubmit the patch with a proper intention explanation in
> the meta and reorder the patches? my first patch and all lol.

Just post is as a v2 with the change noted in the cover letter. I'd also
ensure that it threads properly, right now it's just coming through as 4
separate emails at my end. If you're using git send-email, make sure you
add --thread to the arguments.

-- 
Jens Axboe

Re: [PATCH 0/3] PROTO_CMSG_DATA_ONLY for Datagram (UDP)

[Victor Stewart]

On Sat, Dec 12, 2020 at 5:40 PM Jens Axboe <axboe@kernel.dk> wrote:
>
> On 12/12/20 10:25 AM, Victor Stewart wrote:
> > On Sat, Dec 12, 2020 at 5:07 PM Jens Axboe <axboe@kernel.dk> wrote:
> >>
> >> On 12/12/20 8:31 AM, Victor Stewart wrote:
> >>> RE our conversation on the "[RFC 0/1] whitelisting UDP GSO and GRO
> >>> cmsgs" thread...
> >>>
> >>> https://lore.kernel.org/io-uring/CAM1kxwi5m6i8hrtkw7nZYoziPTD-Wp03+fcsUwh3CuSc=81kUQ@mail.gmail.com/
> >>>
> >>> here are the patches we discussed.
> >>>
> >>> Victor Stewart (3):
> >>>    net/socket.c: add PROTO_CMSG_DATA_ONLY to __sys_sendmsg_sock
> >>>    net/ipv4/af_inet.c: add PROTO_CMSG_DATA_ONLY to inet_dgram_ops
> >>>    net/ipv6/af_inet6.c: add PROTO_CMSG_DATA_ONLY to inet6_dgram_ops
> >>>
> >>>    net/ipv4/af_inet.c
> >>>      |   1 +
> >>>    net/ipv6/af_inet6.c
> >>>     |   1 +
> >>>    net/socket.c
> >>>        |   8 +-
> >>>    3 files changed, 7 insertions(+), 3 deletions(-)
> >>
> >> Changes look fine to me, but a few comments:
> >>
> >> - I'd order 1/3 as 3/3, that ordering makes more sense as at that point it
> >>   could actually be used.
> >
> > right that makes sense.
> >
> >>
> >> - For adding it to af_inet/af_inet6, you should write a better commit message
> >>   on the reasoning for the change. Right now it just describes what the
> >>   patch does (which is obvious from the change), not WHY it's done. Really
> >>   goes for current 1/3 as well, commit messages need to be better in
> >>   general.
> >>
> >
> > okay thanks Jens. i would have reiterated the intention but assumed it
> > were implicit given I linked the initial conversation about enabling
> > UDP_SEGMENT (GSO) and UDP_GRO through io_uring.
> >
> >> I'd also CC Jann Horn on the series, he's the one that found an issue there
> >> in the past and also acked the previous change on doing PROTO_CMSG_DATA_ONLY.
> >
> > I CCed him on this reply. Soheil at the end of the first exchange
> > thread said he audited the UDP paths and believed this to be safe.
> >
> > how/should I resubmit the patch with a proper intention explanation in
> > the meta and reorder the patches? my first patch and all lol.
>
> Just post is as a v2 with the change noted in the cover letter. I'd also
> ensure that it threads properly, right now it's just coming through as 4
> separate emails at my end. If you're using git send-email, make sure you
> add --thread to the arguments.

oh i didn't know about git send-email. i was manually constructing /
sending them lol. thanks!

>
> --
> Jens Axboe
>

Re: [PATCH 0/3] PROTO_CMSG_DATA_ONLY for Datagram (UDP)

[Jens Axboe]

On 12/12/20 10:58 AM, Victor Stewart wrote:
> On Sat, Dec 12, 2020 at 5:40 PM Jens Axboe <axboe@kernel.dk> wrote:
>>
>> On 12/12/20 10:25 AM, Victor Stewart wrote:
>>> On Sat, Dec 12, 2020 at 5:07 PM Jens Axboe <axboe@kernel.dk> wrote:
>>>>
>>>> On 12/12/20 8:31 AM, Victor Stewart wrote:
>>>>> RE our conversation on the "[RFC 0/1] whitelisting UDP GSO and GRO
>>>>> cmsgs" thread...
>>>>>
>>>>> https://lore.kernel.org/io-uring/CAM1kxwi5m6i8hrtkw7nZYoziPTD-Wp03+fcsUwh3CuSc=81kUQ@mail.gmail.com/
>>>>>
>>>>> here are the patches we discussed.
>>>>>
>>>>> Victor Stewart (3):
>>>>>    net/socket.c: add PROTO_CMSG_DATA_ONLY to __sys_sendmsg_sock
>>>>>    net/ipv4/af_inet.c: add PROTO_CMSG_DATA_ONLY to inet_dgram_ops
>>>>>    net/ipv6/af_inet6.c: add PROTO_CMSG_DATA_ONLY to inet6_dgram_ops
>>>>>
>>>>>    net/ipv4/af_inet.c
>>>>>      |   1 +
>>>>>    net/ipv6/af_inet6.c
>>>>>     |   1 +
>>>>>    net/socket.c
>>>>>        |   8 +-
>>>>>    3 files changed, 7 insertions(+), 3 deletions(-)
>>>>
>>>> Changes look fine to me, but a few comments:
>>>>
>>>> - I'd order 1/3 as 3/3, that ordering makes more sense as at that point it
>>>>   could actually be used.
>>>
>>> right that makes sense.
>>>
>>>>
>>>> - For adding it to af_inet/af_inet6, you should write a better commit message
>>>>   on the reasoning for the change. Right now it just describes what the
>>>>   patch does (which is obvious from the change), not WHY it's done. Really
>>>>   goes for current 1/3 as well, commit messages need to be better in
>>>>   general.
>>>>
>>>
>>> okay thanks Jens. i would have reiterated the intention but assumed it
>>> were implicit given I linked the initial conversation about enabling
>>> UDP_SEGMENT (GSO) and UDP_GRO through io_uring.
>>>
>>>> I'd also CC Jann Horn on the series, he's the one that found an issue there
>>>> in the past and also acked the previous change on doing PROTO_CMSG_DATA_ONLY.
>>>
>>> I CCed him on this reply. Soheil at the end of the first exchange
>>> thread said he audited the UDP paths and believed this to be safe.
>>>
>>> how/should I resubmit the patch with a proper intention explanation in
>>> the meta and reorder the patches? my first patch and all lol.
>>
>> Just post is as a v2 with the change noted in the cover letter. I'd also
>> ensure that it threads properly, right now it's just coming through as 4
>> separate emails at my end. If you're using git send-email, make sure you
>> add --thread to the arguments.
> 
> oh i didn't know about git send-email. i was manually constructing /
> sending them lol. thanks!

I'd recommend it, makes sure your mailer doesn't mangle anything either. FWIW,
this is what I do:

git format-patch sha1..sha2
mv 00*.patch /tmp/x

git send-email --no-signed-off-by-cc --thread --compose  --to linux-fsdevel@vger.kernel.org --cc torvalds@linux-foundation.org --cc viro@zeniv.linux.org.uk /tmp/x

(from a series I just sent out). And then I have the following section in
~/.gitconfig:

[sendemail]
from = Jens Axboe <axboe@kernel.dk>
smtpserver = smtp.gmail.com
smtpuser = axboe@kernel.dk
smtpencryption = tls
smtppass = hunter2
smtpserverport = 587

for using gmail to send them out.

--compose will fire up your editor to construct the cover letter, and
when you're happy with it, save+exit and git send-email will ask whether
to proceed or abort.

That's about all there is to it, and provides a consistent way to send out
patch series.

-- 
Jens Axboe

Re: [PATCH 0/3] PROTO_CMSG_DATA_ONLY for Datagram (UDP)

[Victor Stewart]

On Sat, Dec 12, 2020 at 6:02 PM Jens Axboe <axboe@kernel.dk> wrote:
>
> On 12/12/20 10:58 AM, Victor Stewart wrote:
> > On Sat, Dec 12, 2020 at 5:40 PM Jens Axboe <axboe@kernel.dk> wrote:
> >>
> >> On 12/12/20 10:25 AM, Victor Stewart wrote:
> >>> On Sat, Dec 12, 2020 at 5:07 PM Jens Axboe <axboe@kernel.dk> wrote:
> >>>>
> >>>> On 12/12/20 8:31 AM, Victor Stewart wrote:
> >>>>> RE our conversation on the "[RFC 0/1] whitelisting UDP GSO and GRO
> >>>>> cmsgs" thread...
> >>>>>
> >>>>> https://lore.kernel.org/io-uring/CAM1kxwi5m6i8hrtkw7nZYoziPTD-Wp03+fcsUwh3CuSc=81kUQ@mail.gmail.com/
> >>>>>
> >>>>> here are the patches we discussed.
> >>>>>
> >>>>> Victor Stewart (3):
> >>>>>    net/socket.c: add PROTO_CMSG_DATA_ONLY to __sys_sendmsg_sock
> >>>>>    net/ipv4/af_inet.c: add PROTO_CMSG_DATA_ONLY to inet_dgram_ops
> >>>>>    net/ipv6/af_inet6.c: add PROTO_CMSG_DATA_ONLY to inet6_dgram_ops
> >>>>>
> >>>>>    net/ipv4/af_inet.c
> >>>>>      |   1 +
> >>>>>    net/ipv6/af_inet6.c
> >>>>>     |   1 +
> >>>>>    net/socket.c
> >>>>>        |   8 +-
> >>>>>    3 files changed, 7 insertions(+), 3 deletions(-)
> >>>>
> >>>> Changes look fine to me, but a few comments:
> >>>>
> >>>> - I'd order 1/3 as 3/3, that ordering makes more sense as at that point it
> >>>>   could actually be used.
> >>>
> >>> right that makes sense.
> >>>
> >>>>
> >>>> - For adding it to af_inet/af_inet6, you should write a better commit message
> >>>>   on the reasoning for the change. Right now it just describes what the
> >>>>   patch does (which is obvious from the change), not WHY it's done. Really
> >>>>   goes for current 1/3 as well, commit messages need to be better in
> >>>>   general.
> >>>>
> >>>
> >>> okay thanks Jens. i would have reiterated the intention but assumed it
> >>> were implicit given I linked the initial conversation about enabling
> >>> UDP_SEGMENT (GSO) and UDP_GRO through io_uring.
> >>>
> >>>> I'd also CC Jann Horn on the series, he's the one that found an issue there
> >>>> in the past and also acked the previous change on doing PROTO_CMSG_DATA_ONLY.
> >>>
> >>> I CCed him on this reply. Soheil at the end of the first exchange
> >>> thread said he audited the UDP paths and believed this to be safe.
> >>>
> >>> how/should I resubmit the patch with a proper intention explanation in
> >>> the meta and reorder the patches? my first patch and all lol.
> >>
> >> Just post is as a v2 with the change noted in the cover letter. I'd also
> >> ensure that it threads properly, right now it's just coming through as 4
> >> separate emails at my end. If you're using git send-email, make sure you
> >> add --thread to the arguments.
> >
> > oh i didn't know about git send-email. i was manually constructing /
> > sending them lol. thanks!
>
> I'd recommend it, makes sure your mailer doesn't mangle anything either. FWIW,
> this is what I do:
>
> git format-patch sha1..sha2
> mv 00*.patch /tmp/x
>
> git send-email --no-signed-off-by-cc --thread --compose  --to linux-fsdevel@vger.kernel.org --cc torvalds@linux-foundation.org --cc viro@zeniv.linux.org.uk /tmp/x
>
> (from a series I just sent out). And then I have the following section in
> ~/.gitconfig:
>
> [sendemail]
> from = Jens Axboe <axboe@kernel.dk>
> smtpserver = smtp.gmail.com
> smtpuser = axboe@kernel.dk
> smtpencryption = tls
> smtppass = hunter2
> smtpserverport = 587
>
> for using gmail to send them out.
>
> --compose will fire up your editor to construct the cover letter, and
> when you're happy with it, save+exit and git send-email will ask whether
> to proceed or abort.
>
> That's about all there is to it, and provides a consistent way to send out
> patch series.

awesome thanks! i'll be using this workflow from now on.

P.S. hope thats not your real password LOL

>
> --
> Jens Axboe
>

Re: [PATCH 0/3] PROTO_CMSG_DATA_ONLY for Datagram (UDP)

[Jens Axboe]

On 12/12/20 2:42 PM, Victor Stewart wrote:
> On Sat, Dec 12, 2020 at 6:02 PM Jens Axboe <axboe@kernel.dk> wrote:
>>
>> On 12/12/20 10:58 AM, Victor Stewart wrote:
>>> On Sat, Dec 12, 2020 at 5:40 PM Jens Axboe <axboe@kernel.dk> wrote:
>>>>
>>>> On 12/12/20 10:25 AM, Victor Stewart wrote:
>>>>> On Sat, Dec 12, 2020 at 5:07 PM Jens Axboe <axboe@kernel.dk> wrote:
>>>>>>
>>>>>> On 12/12/20 8:31 AM, Victor Stewart wrote:
>>>>>>> RE our conversation on the "[RFC 0/1] whitelisting UDP GSO and GRO
>>>>>>> cmsgs" thread...
>>>>>>>
>>>>>>> https://lore.kernel.org/io-uring/CAM1kxwi5m6i8hrtkw7nZYoziPTD-Wp03+fcsUwh3CuSc=81kUQ@mail.gmail.com/
>>>>>>>
>>>>>>> here are the patches we discussed.
>>>>>>>
>>>>>>> Victor Stewart (3):
>>>>>>>    net/socket.c: add PROTO_CMSG_DATA_ONLY to __sys_sendmsg_sock
>>>>>>>    net/ipv4/af_inet.c: add PROTO_CMSG_DATA_ONLY to inet_dgram_ops
>>>>>>>    net/ipv6/af_inet6.c: add PROTO_CMSG_DATA_ONLY to inet6_dgram_ops
>>>>>>>
>>>>>>>    net/ipv4/af_inet.c
>>>>>>>      |   1 +
>>>>>>>    net/ipv6/af_inet6.c
>>>>>>>     |   1 +
>>>>>>>    net/socket.c
>>>>>>>        |   8 +-
>>>>>>>    3 files changed, 7 insertions(+), 3 deletions(-)
>>>>>>
>>>>>> Changes look fine to me, but a few comments:
>>>>>>
>>>>>> - I'd order 1/3 as 3/3, that ordering makes more sense as at that point it
>>>>>>   could actually be used.
>>>>>
>>>>> right that makes sense.
>>>>>
>>>>>>
>>>>>> - For adding it to af_inet/af_inet6, you should write a better commit message
>>>>>>   on the reasoning for the change. Right now it just describes what the
>>>>>>   patch does (which is obvious from the change), not WHY it's done. Really
>>>>>>   goes for current 1/3 as well, commit messages need to be better in
>>>>>>   general.
>>>>>>
>>>>>
>>>>> okay thanks Jens. i would have reiterated the intention but assumed it
>>>>> were implicit given I linked the initial conversation about enabling
>>>>> UDP_SEGMENT (GSO) and UDP_GRO through io_uring.
>>>>>
>>>>>> I'd also CC Jann Horn on the series, he's the one that found an issue there
>>>>>> in the past and also acked the previous change on doing PROTO_CMSG_DATA_ONLY.
>>>>>
>>>>> I CCed him on this reply. Soheil at the end of the first exchange
>>>>> thread said he audited the UDP paths and believed this to be safe.
>>>>>
>>>>> how/should I resubmit the patch with a proper intention explanation in
>>>>> the meta and reorder the patches? my first patch and all lol.
>>>>
>>>> Just post is as a v2 with the change noted in the cover letter. I'd also
>>>> ensure that it threads properly, right now it's just coming through as 4
>>>> separate emails at my end. If you're using git send-email, make sure you
>>>> add --thread to the arguments.
>>>
>>> oh i didn't know about git send-email. i was manually constructing /
>>> sending them lol. thanks!
>>
>> I'd recommend it, makes sure your mailer doesn't mangle anything either. FWIW,
>> this is what I do:
>>
>> git format-patch sha1..sha2
>> mv 00*.patch /tmp/x
>>
>> git send-email --no-signed-off-by-cc --thread --compose  --to linux-fsdevel@vger.kernel.org --cc torvalds@linux-foundation.org --cc viro@zeniv.linux.org.uk /tmp/x
>>
>> (from a series I just sent out). And then I have the following section in
>> ~/.gitconfig:
>>
>> [sendemail]
>> from = Jens Axboe <axboe@kernel.dk>
>> smtpserver = smtp.gmail.com
>> smtpuser = axboe@kernel.dk
>> smtpencryption = tls
>> smtppass = hunter2
>> smtpserverport = 587
>>
>> for using gmail to send them out.
>>
>> --compose will fire up your editor to construct the cover letter, and
>> when you're happy with it, save+exit and git send-email will ask whether
>> to proceed or abort.
>>
>> That's about all there is to it, and provides a consistent way to send out
>> patch series.
> 
> awesome thanks! i'll be using this workflow from now on.
> 
> P.S. hope thats not your real password LOL

Haha it's not, google hunter2 and password and you'll see :-)

-- 
Jens Axboe

Re: [PATCH 0/3] PROTO_CMSG_DATA_ONLY for Datagram (UDP)

[Victor Stewart]

FYI for anyone who happens upon this...

for gmail you have to first turn on 2-factor authentication then
generate a custom app password for this to work. then use that
password, all the rest the same.

On Sat, Dec 12, 2020 at 9:44 PM Jens Axboe <axboe@kernel.dk> wrote:
>
> On 12/12/20 2:42 PM, Victor Stewart wrote:
> > On Sat, Dec 12, 2020 at 6:02 PM Jens Axboe <axboe@kernel.dk> wrote:
> >>
> >> On 12/12/20 10:58 AM, Victor Stewart wrote:
> >>> On Sat, Dec 12, 2020 at 5:40 PM Jens Axboe <axboe@kernel.dk> wrote:
> >>>>
> >>>> On 12/12/20 10:25 AM, Victor Stewart wrote:
> >>>>> On Sat, Dec 12, 2020 at 5:07 PM Jens Axboe <axboe@kernel.dk> wrote:
> >>>>>>
> >>>>>> On 12/12/20 8:31 AM, Victor Stewart wrote:
> >>>>>>> RE our conversation on the "[RFC 0/1] whitelisting UDP GSO and GRO
> >>>>>>> cmsgs" thread...
> >>>>>>>
> >>>>>>> https://lore.kernel.org/io-uring/CAM1kxwi5m6i8hrtkw7nZYoziPTD-Wp03+fcsUwh3CuSc=81kUQ@mail.gmail.com/
> >>>>>>>
> >>>>>>> here are the patches we discussed.
> >>>>>>>
> >>>>>>> Victor Stewart (3):
> >>>>>>>    net/socket.c: add PROTO_CMSG_DATA_ONLY to __sys_sendmsg_sock
> >>>>>>>    net/ipv4/af_inet.c: add PROTO_CMSG_DATA_ONLY to inet_dgram_ops
> >>>>>>>    net/ipv6/af_inet6.c: add PROTO_CMSG_DATA_ONLY to inet6_dgram_ops
> >>>>>>>
> >>>>>>>    net/ipv4/af_inet.c
> >>>>>>>      |   1 +
> >>>>>>>    net/ipv6/af_inet6.c
> >>>>>>>     |   1 +
> >>>>>>>    net/socket.c
> >>>>>>>        |   8 +-
> >>>>>>>    3 files changed, 7 insertions(+), 3 deletions(-)
> >>>>>>
> >>>>>> Changes look fine to me, but a few comments:
> >>>>>>
> >>>>>> - I'd order 1/3 as 3/3, that ordering makes more sense as at that point it
> >>>>>>   could actually be used.
> >>>>>
> >>>>> right that makes sense.
> >>>>>
> >>>>>>
> >>>>>> - For adding it to af_inet/af_inet6, you should write a better commit message
> >>>>>>   on the reasoning for the change. Right now it just describes what the
> >>>>>>   patch does (which is obvious from the change), not WHY it's done. Really
> >>>>>>   goes for current 1/3 as well, commit messages need to be better in
> >>>>>>   general.
> >>>>>>
> >>>>>
> >>>>> okay thanks Jens. i would have reiterated the intention but assumed it
> >>>>> were implicit given I linked the initial conversation about enabling
> >>>>> UDP_SEGMENT (GSO) and UDP_GRO through io_uring.
> >>>>>
> >>>>>> I'd also CC Jann Horn on the series, he's the one that found an issue there
> >>>>>> in the past and also acked the previous change on doing PROTO_CMSG_DATA_ONLY.
> >>>>>
> >>>>> I CCed him on this reply. Soheil at the end of the first exchange
> >>>>> thread said he audited the UDP paths and believed this to be safe.
> >>>>>
> >>>>> how/should I resubmit the patch with a proper intention explanation in
> >>>>> the meta and reorder the patches? my first patch and all lol.
> >>>>
> >>>> Just post is as a v2 with the change noted in the cover letter. I'd also
> >>>> ensure that it threads properly, right now it's just coming through as 4
> >>>> separate emails at my end. If you're using git send-email, make sure you
> >>>> add --thread to the arguments.
> >>>
> >>> oh i didn't know about git send-email. i was manually constructing /
> >>> sending them lol. thanks!
> >>
> >> I'd recommend it, makes sure your mailer doesn't mangle anything either. FWIW,
> >> this is what I do:
> >>
> >> git format-patch sha1..sha2
> >> mv 00*.patch /tmp/x
> >>
> >> git send-email --no-signed-off-by-cc --thread --compose  --to linux-fsdevel@vger.kernel.org --cc torvalds@linux-foundation.org --cc viro@zeniv.linux.org.uk /tmp/x
> >>
> >> (from a series I just sent out). And then I have the following section in
> >> ~/.gitconfig:
> >>
> >> [sendemail]
> >> from = Jens Axboe <axboe@kernel.dk>
> >> smtpserver = smtp.gmail.com
> >> smtpuser = axboe@kernel.dk
> >> smtpencryption = tls
> >> smtppass = hunter2
> >> smtpserverport = 587
> >>
> >> for using gmail to send them out.
> >>
> >> --compose will fire up your editor to construct the cover letter, and
> >> when you're happy with it, save+exit and git send-email will ask whether
> >> to proceed or abort.
> >>
> >> That's about all there is to it, and provides a consistent way to send out
> >> patch series.
> >
> > awesome thanks! i'll be using this workflow from now on.
> >
> > P.S. hope thats not your real password LOL
>
> Haha it's not, google hunter2 and password and you'll see :-)
>
> --
> Jens Axboe
>