[PATCH bpf-next] docs/bpf: Document some special sdiv/smod operations

[PATCH bpf-next] docs/bpf: Document some special sdiv/smod operations

[Yonghong Song]

Patch [1] fixed possible kernel crash due to specific sdiv/smod operations
in bpf program. The following are related operations and the expected results
of those operations:
  - LLONG_MIN/-1 = LLONG_MIN
  - INT_MIN/-1 = INT_MIN
  - LLONG_MIN%-1 = 0
  - INT_MIN%-1 = 0

Those operations are replaced with codes which won't cause
kernel crash. This patch documents what operations may cause exception and
what replacement operations are.

  [1] https://lore.kernel.org/all/20240913150326.1187788-1-yonghong.song@linux.dev/

Signed-off-by: Yonghong Song <yonghong.song@linux.dev>
---
 .../bpf/standardization/instruction-set.rst   | 25 +++++++++++++++----
 1 file changed, 20 insertions(+), 5 deletions(-)

diff --git a/Documentation/bpf/standardization/instruction-set.rst b/Documentation/bpf/standardization/instruction-set.rst
index ab820d565052..d150c1d7ad3b 100644
--- a/Documentation/bpf/standardization/instruction-set.rst
+++ b/Documentation/bpf/standardization/instruction-set.rst
@@ -347,11 +347,26 @@ register.
   =====  =====  =======  ==========================================================
 
 Underflow and overflow are allowed during arithmetic operations, meaning
-the 64-bit or 32-bit value will wrap. If BPF program execution would
-result in division by zero, the destination register is instead set to zero.
-If execution would result in modulo by zero, for ``ALU64`` the value of
-the destination register is unchanged whereas for ``ALU`` the upper
-32 bits of the destination register are zeroed.
+the 64-bit or 32-bit value will wrap. There are also a few arithmetic operations
+which may cause exception for certain architectures. Since crashing the kernel
+is not an option, those operations are replaced with alternative operations.
+
+.. table:: Arithmetic operations with possible exceptions
+
+  =====  ==========  =============================  ==========================
+  name   class       original                       replacement
+  =====  ==========  =============================  ==========================
+  DIV    ALU64/ALU   dst /= 0                       dst = 0
+  SDIV   ALU64/ALU   dst s/= 0                      dst = 0
+  MOD    ALU64       dst %= 0                       dst = dst (no replacement)
+  MOD    ALU         dst %= 0                       dst = (u32)dst
+  SMOD   ALU64       dst s%= 0                      dst = dst (no replacement)
+  SMOD   ALU         dst s%= 0                      dst = (u32)dst
+  SDIV   ALU64       dst s/= -1 (dst = LLONG_MIN)   dst = LLONG_MIN
+  SDIV   ALU         dst s/= -1 (dst = INT_MIN)     dst = (u32)INT_MIN
+  SMOD   ALU64       dst s%= -1 (dst = LLONG_MIN)   dst = 0
+  SMOD   ALU         dst s%= -1 (dst = INT_MIN)     dst = 0
+  =====  ==========  =============================  ===========================
 
 ``{ADD, X, ALU}``, where 'code' = ``ADD``, 'source' = ``X``, and 'class' = ``ALU``, means::
 
-- 
2.43.5

Re: [PATCH bpf-next] docs/bpf: Document some special sdiv/smod operations

[Alexei Starovoitov]

On Thu, Sep 26, 2024 at 8:39 PM Yonghong Song <yonghong.song@linux.dev> wrote:
>
> Patch [1] fixed possible kernel crash due to specific sdiv/smod operations
> in bpf program. The following are related operations and the expected results
> of those operations:
>   - LLONG_MIN/-1 = LLONG_MIN
>   - INT_MIN/-1 = INT_MIN
>   - LLONG_MIN%-1 = 0
>   - INT_MIN%-1 = 0
>
> Those operations are replaced with codes which won't cause
> kernel crash. This patch documents what operations may cause exception and
> what replacement operations are.
>
>   [1] https://lore.kernel.org/all/20240913150326.1187788-1-yonghong.song@linux.dev/
>
> Signed-off-by: Yonghong Song <yonghong.song@linux.dev>
> ---
>  .../bpf/standardization/instruction-set.rst   | 25 +++++++++++++++----
>  1 file changed, 20 insertions(+), 5 deletions(-)
>
> diff --git a/Documentation/bpf/standardization/instruction-set.rst b/Documentation/bpf/standardization/instruction-set.rst
> index ab820d565052..d150c1d7ad3b 100644
> --- a/Documentation/bpf/standardization/instruction-set.rst
> +++ b/Documentation/bpf/standardization/instruction-set.rst
> @@ -347,11 +347,26 @@ register.
>    =====  =====  =======  ==========================================================
>
>  Underflow and overflow are allowed during arithmetic operations, meaning
> -the 64-bit or 32-bit value will wrap. If BPF program execution would
> -result in division by zero, the destination register is instead set to zero.
> -If execution would result in modulo by zero, for ``ALU64`` the value of
> -the destination register is unchanged whereas for ``ALU`` the upper
> -32 bits of the destination register are zeroed.
> +the 64-bit or 32-bit value will wrap. There are also a few arithmetic operations
> +which may cause exception for certain architectures. Since crashing the kernel
> +is not an option, those operations are replaced with alternative operations.
> +
> +.. table:: Arithmetic operations with possible exceptions
> +
> +  =====  ==========  =============================  ==========================
> +  name   class       original                       replacement
> +  =====  ==========  =============================  ==========================
> +  DIV    ALU64/ALU   dst /= 0                       dst = 0
> +  SDIV   ALU64/ALU   dst s/= 0                      dst = 0
> +  MOD    ALU64       dst %= 0                       dst = dst (no replacement)
> +  MOD    ALU         dst %= 0                       dst = (u32)dst
> +  SMOD   ALU64       dst s%= 0                      dst = dst (no replacement)
> +  SMOD   ALU         dst s%= 0                      dst = (u32)dst
> +  SDIV   ALU64       dst s/= -1 (dst = LLONG_MIN)   dst = LLONG_MIN
> +  SDIV   ALU         dst s/= -1 (dst = INT_MIN)     dst = (u32)INT_MIN
> +  SMOD   ALU64       dst s%= -1 (dst = LLONG_MIN)   dst = 0
> +  SMOD   ALU         dst s%= -1 (dst = INT_MIN)     dst = 0

This is a great addition to the doc, but this file is currently
being used as a base for IETF standard which is in its final "edit" stage
which may require few patches,
so we cannot land any changes to instruction-set.rst
not related to standardization until RFC number is issued and
it becomes immutable. After that the same instruction-set.rst
file can be reused for future revisions on the standard.
Hopefully the draft will clear the final hurdle in a couple weeks.
Until then:
pw-bot: cr

Re: [PATCH bpf-next] docs/bpf: Document some special sdiv/smod operations

[Yonghong Song]

On 9/30/24 6:50 PM, Alexei Starovoitov wrote:
> On Thu, Sep 26, 2024 at 8:39 PM Yonghong Song <yonghong.song@linux.dev> wrote:
>> Patch [1] fixed possible kernel crash due to specific sdiv/smod operations
>> in bpf program. The following are related operations and the expected results
>> of those operations:
>>    - LLONG_MIN/-1 = LLONG_MIN
>>    - INT_MIN/-1 = INT_MIN
>>    - LLONG_MIN%-1 = 0
>>    - INT_MIN%-1 = 0
>>
>> Those operations are replaced with codes which won't cause
>> kernel crash. This patch documents what operations may cause exception and
>> what replacement operations are.
>>
>>    [1] https://lore.kernel.org/all/20240913150326.1187788-1-yonghong.song@linux.dev/
>>
>> Signed-off-by: Yonghong Song <yonghong.song@linux.dev>
>> ---
>>   .../bpf/standardization/instruction-set.rst   | 25 +++++++++++++++----
>>   1 file changed, 20 insertions(+), 5 deletions(-)
>>
>> diff --git a/Documentation/bpf/standardization/instruction-set.rst b/Documentation/bpf/standardization/instruction-set.rst
>> index ab820d565052..d150c1d7ad3b 100644
>> --- a/Documentation/bpf/standardization/instruction-set.rst
>> +++ b/Documentation/bpf/standardization/instruction-set.rst
>> @@ -347,11 +347,26 @@ register.
>>     =====  =====  =======  ==========================================================
>>
>>   Underflow and overflow are allowed during arithmetic operations, meaning
>> -the 64-bit or 32-bit value will wrap. If BPF program execution would
>> -result in division by zero, the destination register is instead set to zero.
>> -If execution would result in modulo by zero, for ``ALU64`` the value of
>> -the destination register is unchanged whereas for ``ALU`` the upper
>> -32 bits of the destination register are zeroed.
>> +the 64-bit or 32-bit value will wrap. There are also a few arithmetic operations
>> +which may cause exception for certain architectures. Since crashing the kernel
>> +is not an option, those operations are replaced with alternative operations.
>> +
>> +.. table:: Arithmetic operations with possible exceptions
>> +
>> +  =====  ==========  =============================  ==========================
>> +  name   class       original                       replacement
>> +  =====  ==========  =============================  ==========================
>> +  DIV    ALU64/ALU   dst /= 0                       dst = 0
>> +  SDIV   ALU64/ALU   dst s/= 0                      dst = 0
>> +  MOD    ALU64       dst %= 0                       dst = dst (no replacement)
>> +  MOD    ALU         dst %= 0                       dst = (u32)dst
>> +  SMOD   ALU64       dst s%= 0                      dst = dst (no replacement)
>> +  SMOD   ALU         dst s%= 0                      dst = (u32)dst
>> +  SDIV   ALU64       dst s/= -1 (dst = LLONG_MIN)   dst = LLONG_MIN
>> +  SDIV   ALU         dst s/= -1 (dst = INT_MIN)     dst = (u32)INT_MIN
>> +  SMOD   ALU64       dst s%= -1 (dst = LLONG_MIN)   dst = 0
>> +  SMOD   ALU         dst s%= -1 (dst = INT_MIN)     dst = 0
> This is a great addition to the doc, but this file is currently
> being used as a base for IETF standard which is in its final "edit" stage
> which may require few patches,
> so we cannot land any changes to instruction-set.rst
> not related to standardization until RFC number is issued and
> it becomes immutable. After that the same instruction-set.rst
> file can be reused for future revisions on the standard.
> Hopefully the draft will clear the final hurdle in a couple weeks.
> Until then:
> pw-bot: cr

Sure. No problem. Will resubmit once the RFC number is issued.

RE: [PATCH bpf-next] docs/bpf: Document some special sdiv/smod operations

[Dave Thaler]

Yonghong Song <yonghong.song@linux.dev> wrote: 
> On 9/30/24 6:50 PM, Alexei Starovoitov wrote:
> > On Thu, Sep 26, 2024 at 8:39 PM Yonghong Song <yonghong.song@linux.dev>
> wrote:
> >> Patch [1] fixed possible kernel crash due to specific sdiv/smod
> >> operations in bpf program. The following are related operations and
> >> the expected results of those operations:
> >>    - LLONG_MIN/-1 = LLONG_MIN
> >>    - INT_MIN/-1 = INT_MIN
> >>    - LLONG_MIN%-1 = 0
> >>    - INT_MIN%-1 = 0
> >>
> >> Those operations are replaced with codes which won't cause kernel
> >> crash. This patch documents what operations may cause exception and
> >> what replacement operations are.
> >>
> >>    [1]
> >> https://lore.kernel.org/all/20240913150326.1187788-1-yonghong.song@li
> >> nux.dev/
> >>
> >> Signed-off-by: Yonghong Song <yonghong.song@linux.dev>
> >> ---
> >>   .../bpf/standardization/instruction-set.rst   | 25 +++++++++++++++----
> >>   1 file changed, 20 insertions(+), 5 deletions(-)
> >>
> >> diff --git a/Documentation/bpf/standardization/instruction-set.rst
> >> b/Documentation/bpf/standardization/instruction-set.rst
> >> index ab820d565052..d150c1d7ad3b 100644
> >> --- a/Documentation/bpf/standardization/instruction-set.rst
> >> +++ b/Documentation/bpf/standardization/instruction-set.rst
> >> @@ -347,11 +347,26 @@ register.
> >>     =====  =====  =======
> >> ==========================================================
> >>
> >>   Underflow and overflow are allowed during arithmetic operations,
> >> meaning -the 64-bit or 32-bit value will wrap. If BPF program
> >> execution would -result in division by zero, the destination register is instead set
> to zero.
> >> -If execution would result in modulo by zero, for ``ALU64`` the value of
> >> -the destination register is unchanged whereas for ``ALU`` the upper
> >> -32 bits of the destination register are zeroed.
> >> +the 64-bit or 32-bit value will wrap. There are also a few
> >> +arithmetic operations which may cause exception for certain
> >> +architectures. Since crashing the kernel is not an option, those operations are
> replaced with alternative operations.
> >> +
> >> +.. table:: Arithmetic operations with possible exceptions
> >> +
> >> +  =====  ==========  =============================
> ==========================
> >> +  name   class       original                       replacement
> >> +  =====  ==========  =============================
> ==========================
> >> +  DIV    ALU64/ALU   dst /= 0                       dst = 0
> >> +  SDIV   ALU64/ALU   dst s/= 0                      dst = 0
> >> +  MOD    ALU64       dst %= 0                       dst = dst (no replacement)
> >> +  MOD    ALU         dst %= 0                       dst = (u32)dst
> >> +  SMOD   ALU64       dst s%= 0                      dst = dst (no replacement)
> >> +  SMOD   ALU         dst s%= 0                      dst = (u32)dst

All of the above are already covered in existing Table 5 and in my opinion
don't need to be repeated.

That is, the "original" is not what Table 5 has, so just introduces confusion
in the document in my opinion.

> >> +  SDIV   ALU64       dst s/= -1 (dst = LLONG_MIN)   dst = LLONG_MIN
> >> +  SDIV   ALU         dst s/= -1 (dst = INT_MIN)     dst = (u32)INT_MIN
> >> +  SMOD   ALU64       dst s%= -1 (dst = LLONG_MIN)   dst = 0
> >> +  SMOD   ALU         dst s%= -1 (dst = INT_MIN)     dst = 0

The above four are the new ones and I'd prefer a solution that modifies
existing table 5.  E.g. table 5 has now for SMOD:

dst = (src != 0) ? (dst s% src) : dst

and could have something like this:

dst = (src == 0) ? dst : ((src == -1 && dst == INT_MIN) ? 0 : (dst s% src))

> > This is a great addition to the doc, but this file is currently being
> > used as a base for IETF standard which is in its final "edit" stage
> > which may require few patches, so we cannot land any changes to
> > instruction-set.rst not related to standardization until RFC number is
> > issued and it becomes immutable. After that the same
> > instruction-set.rst file can be reused for future revisions on the
> > standard.
> > Hopefully the draft will clear the final hurdle in a couple weeks.
> > Until then:
> > pw-bot: cr
> 
> Sure. No problem. Will resubmit once the RFC number is issued.

I'm adding bpf@ietf.org to the To line since all changes in the standardization
directory should include that mailing list.

The WG should discuss whether any changes should be done via a new RFC
that obsoletes the first one, or as RFCs that Update and just describe deltas
(additions, etc.).

There are precedents both ways and I don't have a strong preference, but I
have a weak preference for delta-based ones since they're shorter and are
less likely to re-open discussion on previously resolved issues, thus often
saving the WG time.

Also FYI to Linux kernel folks:
With WG and AD approval, it's also possible (but not ideal) to take changes
at AUTH48.  That'd be up to the chairs and AD to decide though, and normally
that's just for purely editorial clarifications, e.g., to confusion called out by the
RFC editor pass.

Dave

[Bpf] Re: [PATCH bpf-next] docs/bpf: Document some special sdiv/smod operations

[Dave Thaler]

Yonghong Song <yonghong.song@linux.dev> wrote: 
> On 9/30/24 6:50 PM, Alexei Starovoitov wrote:
> > On Thu, Sep 26, 2024 at 8:39 PM Yonghong Song <yonghong.song@linux.dev>
> wrote:
> >> Patch [1] fixed possible kernel crash due to specific sdiv/smod
> >> operations in bpf program. The following are related operations and
> >> the expected results of those operations:
> >>    - LLONG_MIN/-1 = LLONG_MIN
> >>    - INT_MIN/-1 = INT_MIN
> >>    - LLONG_MIN%-1 = 0
> >>    - INT_MIN%-1 = 0
> >>
> >> Those operations are replaced with codes which won't cause kernel
> >> crash. This patch documents what operations may cause exception and
> >> what replacement operations are.
> >>
> >>    [1]
> >> https://lore.kernel.org/all/20240913150326.1187788-1-yonghong.song@li
> >> nux.dev/
> >>
> >> Signed-off-by: Yonghong Song <yonghong.song@linux.dev>
> >> ---
> >>   .../bpf/standardization/instruction-set.rst   | 25 +++++++++++++++----
> >>   1 file changed, 20 insertions(+), 5 deletions(-)
> >>
> >> diff --git a/Documentation/bpf/standardization/instruction-set.rst
> >> b/Documentation/bpf/standardization/instruction-set.rst
> >> index ab820d565052..d150c1d7ad3b 100644
> >> --- a/Documentation/bpf/standardization/instruction-set.rst
> >> +++ b/Documentation/bpf/standardization/instruction-set.rst
> >> @@ -347,11 +347,26 @@ register.
> >>     =====  =====  =======
> >> ==========================================================
> >>
> >>   Underflow and overflow are allowed during arithmetic operations,
> >> meaning -the 64-bit or 32-bit value will wrap. If BPF program
> >> execution would -result in division by zero, the destination register is instead set
> to zero.
> >> -If execution would result in modulo by zero, for ``ALU64`` the value of
> >> -the destination register is unchanged whereas for ``ALU`` the upper
> >> -32 bits of the destination register are zeroed.
> >> +the 64-bit or 32-bit value will wrap. There are also a few
> >> +arithmetic operations which may cause exception for certain
> >> +architectures. Since crashing the kernel is not an option, those operations are
> replaced with alternative operations.
> >> +
> >> +.. table:: Arithmetic operations with possible exceptions
> >> +
> >> +  =====  ==========  =============================
> ==========================
> >> +  name   class       original                       replacement
> >> +  =====  ==========  =============================
> ==========================
> >> +  DIV    ALU64/ALU   dst /= 0                       dst = 0
> >> +  SDIV   ALU64/ALU   dst s/= 0                      dst = 0
> >> +  MOD    ALU64       dst %= 0                       dst = dst (no replacement)
> >> +  MOD    ALU         dst %= 0                       dst = (u32)dst
> >> +  SMOD   ALU64       dst s%= 0                      dst = dst (no replacement)
> >> +  SMOD   ALU         dst s%= 0                      dst = (u32)dst

All of the above are already covered in existing Table 5 and in my opinion
don't need to be repeated.

That is, the "original" is not what Table 5 has, so just introduces confusion
in the document in my opinion.

> >> +  SDIV   ALU64       dst s/= -1 (dst = LLONG_MIN)   dst = LLONG_MIN
> >> +  SDIV   ALU         dst s/= -1 (dst = INT_MIN)     dst = (u32)INT_MIN
> >> +  SMOD   ALU64       dst s%= -1 (dst = LLONG_MIN)   dst = 0
> >> +  SMOD   ALU         dst s%= -1 (dst = INT_MIN)     dst = 0

The above four are the new ones and I'd prefer a solution that modifies
existing table 5.  E.g. table 5 has now for SMOD:

dst = (src != 0) ? (dst s% src) : dst

and could have something like this:

dst = (src == 0) ? dst : ((src == -1 && dst == INT_MIN) ? 0 : (dst s% src))

> > This is a great addition to the doc, but this file is currently being
> > used as a base for IETF standard which is in its final "edit" stage
> > which may require few patches, so we cannot land any changes to
> > instruction-set.rst not related to standardization until RFC number is
> > issued and it becomes immutable. After that the same
> > instruction-set.rst file can be reused for future revisions on the
> > standard.
> > Hopefully the draft will clear the final hurdle in a couple weeks.
> > Until then:
> > pw-bot: cr
> 
> Sure. No problem. Will resubmit once the RFC number is issued.

I'm adding bpf@ietf.org to the To line since all changes in the standardization
directory should include that mailing list.

The WG should discuss whether any changes should be done via a new RFC
that obsoletes the first one, or as RFCs that Update and just describe deltas
(additions, etc.).

There are precedents both ways and I don't have a strong preference, but I
have a weak preference for delta-based ones since they're shorter and are
less likely to re-open discussion on previously resolved issues, thus often
saving the WG time.

Also FYI to Linux kernel folks:
With WG and AD approval, it's also possible (but not ideal) to take changes
at AUTH48.  That'd be up to the chairs and AD to decide though, and normally
that's just for purely editorial clarifications, e.g., to confusion called out by the
RFC editor pass.

Dave

-- 
Bpf mailing list -- bpf@ietf.org
To unsubscribe send an email to bpf-leave@ietf.org

Re: [PATCH bpf-next] docs/bpf: Document some special sdiv/smod operations

[Alexei Starovoitov]

On Tue, Oct 1, 2024 at 12:54 PM Dave Thaler <dthaler1968@googlemail.com> wrote:
>
> Yonghong Song <yonghong.song@linux.dev> wrote:
> > On 9/30/24 6:50 PM, Alexei Starovoitov wrote:
> > > On Thu, Sep 26, 2024 at 8:39 PM Yonghong Song <yonghong.song@linux.dev>
> > wrote:
> > >> Patch [1] fixed possible kernel crash due to specific sdiv/smod
> > >> operations in bpf program. The following are related operations and
> > >> the expected results of those operations:
> > >>    - LLONG_MIN/-1 = LLONG_MIN
> > >>    - INT_MIN/-1 = INT_MIN
> > >>    - LLONG_MIN%-1 = 0
> > >>    - INT_MIN%-1 = 0
> > >>
> > >> Those operations are replaced with codes which won't cause kernel
> > >> crash. This patch documents what operations may cause exception and
> > >> what replacement operations are.
> > >>
> > >>    [1]
> > >> https://lore.kernel.org/all/20240913150326.1187788-1-yonghong.song@li
> > >> nux.dev/
> > >>
> > >> Signed-off-by: Yonghong Song <yonghong.song@linux.dev>
> > >> ---
> > >>   .../bpf/standardization/instruction-set.rst   | 25 +++++++++++++++----
> > >>   1 file changed, 20 insertions(+), 5 deletions(-)
> > >>
> > >> diff --git a/Documentation/bpf/standardization/instruction-set.rst
> > >> b/Documentation/bpf/standardization/instruction-set.rst
> > >> index ab820d565052..d150c1d7ad3b 100644
> > >> --- a/Documentation/bpf/standardization/instruction-set.rst
> > >> +++ b/Documentation/bpf/standardization/instruction-set.rst
> > >> @@ -347,11 +347,26 @@ register.
> > >>     =====  =====  =======
> > >> ==========================================================
> > >>
> > >>   Underflow and overflow are allowed during arithmetic operations,
> > >> meaning -the 64-bit or 32-bit value will wrap. If BPF program
> > >> execution would -result in division by zero, the destination register is instead set
> > to zero.
> > >> -If execution would result in modulo by zero, for ``ALU64`` the value of
> > >> -the destination register is unchanged whereas for ``ALU`` the upper
> > >> -32 bits of the destination register are zeroed.
> > >> +the 64-bit or 32-bit value will wrap. There are also a few
> > >> +arithmetic operations which may cause exception for certain
> > >> +architectures. Since crashing the kernel is not an option, those operations are
> > replaced with alternative operations.
> > >> +
> > >> +.. table:: Arithmetic operations with possible exceptions
> > >> +
> > >> +  =====  ==========  =============================
> > ==========================
> > >> +  name   class       original                       replacement
> > >> +  =====  ==========  =============================
> > ==========================
> > >> +  DIV    ALU64/ALU   dst /= 0                       dst = 0
> > >> +  SDIV   ALU64/ALU   dst s/= 0                      dst = 0
> > >> +  MOD    ALU64       dst %= 0                       dst = dst (no replacement)
> > >> +  MOD    ALU         dst %= 0                       dst = (u32)dst
> > >> +  SMOD   ALU64       dst s%= 0                      dst = dst (no replacement)
> > >> +  SMOD   ALU         dst s%= 0                      dst = (u32)dst
>
> All of the above are already covered in existing Table 5 and in my opinion
> don't need to be repeated.
>
> That is, the "original" is not what Table 5 has, so just introduces confusion
> in the document in my opinion.
>
> > >> +  SDIV   ALU64       dst s/= -1 (dst = LLONG_MIN)   dst = LLONG_MIN
> > >> +  SDIV   ALU         dst s/= -1 (dst = INT_MIN)     dst = (u32)INT_MIN
> > >> +  SMOD   ALU64       dst s%= -1 (dst = LLONG_MIN)   dst = 0
> > >> +  SMOD   ALU         dst s%= -1 (dst = INT_MIN)     dst = 0
>
> The above four are the new ones and I'd prefer a solution that modifies
> existing table 5.  E.g. table 5 has now for SMOD:
>
> dst = (src != 0) ? (dst s% src) : dst
>
> and could have something like this:
>
> dst = (src == 0) ? dst : ((src == -1 && dst == INT_MIN) ? 0 : (dst s% src))
>
> > > This is a great addition to the doc, but this file is currently being
> > > used as a base for IETF standard which is in its final "edit" stage
> > > which may require few patches, so we cannot land any changes to
> > > instruction-set.rst not related to standardization until RFC number is
> > > issued and it becomes immutable. After that the same
> > > instruction-set.rst file can be reused for future revisions on the
> > > standard.
> > > Hopefully the draft will clear the final hurdle in a couple weeks.
> > > Until then:
> > > pw-bot: cr
> >
> > Sure. No problem. Will resubmit once the RFC number is issued.
>
> I'm adding bpf@ietf.org to the To line since all changes in the standardization
> directory should include that mailing list.
>
> The WG should discuss whether any changes should be done via a new RFC
> that obsoletes the first one, or as RFCs that Update and just describe deltas
> (additions, etc.).
>
> There are precedents both ways and I don't have a strong preference, but I
> have a weak preference for delta-based ones since they're shorter and are
> less likely to re-open discussion on previously resolved issues, thus often
> saving the WG time.

Delta-based additions make sense to me.

>
> Also FYI to Linux kernel folks:
> With WG and AD approval, it's also possible (but not ideal) to take changes
> at AUTH48.  That'd be up to the chairs and AD to decide though, and normally
> that's just for purely editorial clarifications, e.g., to confusion called out by the
> RFC editor pass.

Also agree. We should keep AUTH going its course as-is.
All ISA additions can be in the future delta RFC.

As far as file logistics... my preference is to keep
Documentation/bpf/standardization/instruction-set.rst
up to date.
Right now it's effectively frozen while awaiting changes (if any)
necessary for AUTH. After official RFC is issued
we can start landing patches into instruction-set.rst and
git diff 04efaebd72d1..whatever_future_sha instruction-set.rst
will automatically generate the future delta RFC.
Once RFC number is issued we can add a git tag for the particular
sha that was the base for RFC as a documentation step and to
simplify future 'git diff'.

[Bpf] Re: [PATCH bpf-next] docs/bpf: Document some special sdiv/smod operations

[Alexei Starovoitov]

On Tue, Oct 1, 2024 at 12:54 PM Dave Thaler <dthaler1968@googlemail.com> wrote:
>
> Yonghong Song <yonghong.song@linux.dev> wrote:
> > On 9/30/24 6:50 PM, Alexei Starovoitov wrote:
> > > On Thu, Sep 26, 2024 at 8:39 PM Yonghong Song <yonghong.song@linux.dev>
> > wrote:
> > >> Patch [1] fixed possible kernel crash due to specific sdiv/smod
> > >> operations in bpf program. The following are related operations and
> > >> the expected results of those operations:
> > >>    - LLONG_MIN/-1 = LLONG_MIN
> > >>    - INT_MIN/-1 = INT_MIN
> > >>    - LLONG_MIN%-1 = 0
> > >>    - INT_MIN%-1 = 0
> > >>
> > >> Those operations are replaced with codes which won't cause kernel
> > >> crash. This patch documents what operations may cause exception and
> > >> what replacement operations are.
> > >>
> > >>    [1]
> > >> https://lore.kernel.org/all/20240913150326.1187788-1-yonghong.song@li
> > >> nux.dev/
> > >>
> > >> Signed-off-by: Yonghong Song <yonghong.song@linux.dev>
> > >> ---
> > >>   .../bpf/standardization/instruction-set.rst   | 25 +++++++++++++++----
> > >>   1 file changed, 20 insertions(+), 5 deletions(-)
> > >>
> > >> diff --git a/Documentation/bpf/standardization/instruction-set.rst
> > >> b/Documentation/bpf/standardization/instruction-set.rst
> > >> index ab820d565052..d150c1d7ad3b 100644
> > >> --- a/Documentation/bpf/standardization/instruction-set.rst
> > >> +++ b/Documentation/bpf/standardization/instruction-set.rst
> > >> @@ -347,11 +347,26 @@ register.
> > >>     =====  =====  =======
> > >> ==========================================================
> > >>
> > >>   Underflow and overflow are allowed during arithmetic operations,
> > >> meaning -the 64-bit or 32-bit value will wrap. If BPF program
> > >> execution would -result in division by zero, the destination register is instead set
> > to zero.
> > >> -If execution would result in modulo by zero, for ``ALU64`` the value of
> > >> -the destination register is unchanged whereas for ``ALU`` the upper
> > >> -32 bits of the destination register are zeroed.
> > >> +the 64-bit or 32-bit value will wrap. There are also a few
> > >> +arithmetic operations which may cause exception for certain
> > >> +architectures. Since crashing the kernel is not an option, those operations are
> > replaced with alternative operations.
> > >> +
> > >> +.. table:: Arithmetic operations with possible exceptions
> > >> +
> > >> +  =====  ==========  =============================
> > ==========================
> > >> +  name   class       original                       replacement
> > >> +  =====  ==========  =============================
> > ==========================
> > >> +  DIV    ALU64/ALU   dst /= 0                       dst = 0
> > >> +  SDIV   ALU64/ALU   dst s/= 0                      dst = 0
> > >> +  MOD    ALU64       dst %= 0                       dst = dst (no replacement)
> > >> +  MOD    ALU         dst %= 0                       dst = (u32)dst
> > >> +  SMOD   ALU64       dst s%= 0                      dst = dst (no replacement)
> > >> +  SMOD   ALU         dst s%= 0                      dst = (u32)dst
>
> All of the above are already covered in existing Table 5 and in my opinion
> don't need to be repeated.
>
> That is, the "original" is not what Table 5 has, so just introduces confusion
> in the document in my opinion.
>
> > >> +  SDIV   ALU64       dst s/= -1 (dst = LLONG_MIN)   dst = LLONG_MIN
> > >> +  SDIV   ALU         dst s/= -1 (dst = INT_MIN)     dst = (u32)INT_MIN
> > >> +  SMOD   ALU64       dst s%= -1 (dst = LLONG_MIN)   dst = 0
> > >> +  SMOD   ALU         dst s%= -1 (dst = INT_MIN)     dst = 0
>
> The above four are the new ones and I'd prefer a solution that modifies
> existing table 5.  E.g. table 5 has now for SMOD:
>
> dst = (src != 0) ? (dst s% src) : dst
>
> and could have something like this:
>
> dst = (src == 0) ? dst : ((src == -1 && dst == INT_MIN) ? 0 : (dst s% src))
>
> > > This is a great addition to the doc, but this file is currently being
> > > used as a base for IETF standard which is in its final "edit" stage
> > > which may require few patches, so we cannot land any changes to
> > > instruction-set.rst not related to standardization until RFC number is
> > > issued and it becomes immutable. After that the same
> > > instruction-set.rst file can be reused for future revisions on the
> > > standard.
> > > Hopefully the draft will clear the final hurdle in a couple weeks.
> > > Until then:
> > > pw-bot: cr
> >
> > Sure. No problem. Will resubmit once the RFC number is issued.
>
> I'm adding bpf@ietf.org to the To line since all changes in the standardization
> directory should include that mailing list.
>
> The WG should discuss whether any changes should be done via a new RFC
> that obsoletes the first one, or as RFCs that Update and just describe deltas
> (additions, etc.).
>
> There are precedents both ways and I don't have a strong preference, but I
> have a weak preference for delta-based ones since they're shorter and are
> less likely to re-open discussion on previously resolved issues, thus often
> saving the WG time.

Delta-based additions make sense to me.

>
> Also FYI to Linux kernel folks:
> With WG and AD approval, it's also possible (but not ideal) to take changes
> at AUTH48.  That'd be up to the chairs and AD to decide though, and normally
> that's just for purely editorial clarifications, e.g., to confusion called out by the
> RFC editor pass.

Also agree. We should keep AUTH going its course as-is.
All ISA additions can be in the future delta RFC.

As far as file logistics... my preference is to keep
Documentation/bpf/standardization/instruction-set.rst
up to date.
Right now it's effectively frozen while awaiting changes (if any)
necessary for AUTH. After official RFC is issued
we can start landing patches into instruction-set.rst and
git diff 04efaebd72d1..whatever_future_sha instruction-set.rst
will automatically generate the future delta RFC.
Once RFC number is issued we can add a git tag for the particular
sha that was the base for RFC as a documentation step and to
simplify future 'git diff'.

-- 
Bpf mailing list -- bpf@ietf.org
To unsubscribe send an email to bpf-leave@ietf.org

RE: [PATCH bpf-next] docs/bpf: Document some special sdiv/smod operations

[Dave Thaler]

Alexei Starovoitov <alexei.starovoitov@gmail.com> wrote:
> On Tue, Oct 1, 2024 at 12:54 PM Dave Thaler <dthaler1968@googlemail.com>
> wrote:
[...]
> > I'm adding bpf@ietf.org to the To line since all changes in the
> > standardization directory should include that mailing list.
> >
> > The WG should discuss whether any changes should be done via a new RFC
> > that obsoletes the first one, or as RFCs that Update and just describe
> > deltas (additions, etc.).
> >
> > There are precedents both ways and I don't have a strong preference,
> > but I have a weak preference for delta-based ones since they're
> > shorter and are less likely to re-open discussion on previously
> > resolved issues, thus often saving the WG time.
> 
> Delta-based additions make sense to me.
> 
> > Also FYI to Linux kernel folks:
> > With WG and AD approval, it's also possible (but not ideal) to take
> > changes at AUTH48.  That'd be up to the chairs and AD to decide
> > though, and normally that's just for purely editorial clarifications,
> > e.g., to confusion called out by the RFC editor pass.
> 
> Also agree. We should keep AUTH going its course as-is.
> All ISA additions can be in the future delta RFC.
> 
> As far as file logistics... my preference is to keep
> Documentation/bpf/standardization/instruction-set.rst
> up to date.
> Right now it's effectively frozen while awaiting changes (if any) necessary for AUTH.
> After official RFC is issued we can start landing patches into instruction-set.rst and
> git diff 04efaebd72d1..whatever_future_sha instruction-set.rst will automatically
> generate the future delta RFC.
> Once RFC number is issued we can add a git tag for the particular sha that was the
> base for RFC as a documentation step and to simplify future 'git diff'.

My concern is that index.rst says:
> This directory contains documents that are being iterated on as part of the BPF
> standardization effort with the IETF. See the `IETF BPF Working Group`_ page
> for the working group charter, documents, and more.

So having a document that is NOT part of the IETF BPF Working Group would seem
out of place and, in my view, better located up a level (outside standardization).

Here’s some examples of delta-based RFCs which explain the gap and provide
the addition or clarification, and formally Update (not replace/obsolete) the original
RFC:
* https://www.rfc-editor.org/rfc/rfc6585.html: Additional HTTP Status Codes
* https://www.rfc-editor.org/rfc/rfc6840.html: Clarifications and Implementation Notes
   for DNS Security (DNSSEC)
* https://www.rfc-editor.org/rfc/rfc9295.html: Clarifications for Ed25519, Ed448,
   X25519, and X448 Algorithm Identifiers
* https://www.rfc-editor.org/rfc/rfc5756.html: Updates for RSAES-OAEP and 
   RSASSA-PSS Algorithm Parameters

Having a full document too is valuable but unless the IETF BPF WG
decides to take on a -bis document, I'd suggest keeping it out of the "standardization"
(say up 1 level) to avoid confusion, and just have one or more delta-based rst files
in the standardization directory.

Dave

[Bpf] Re: [PATCH bpf-next] docs/bpf: Document some special sdiv/smod operations

[Dave Thaler]

Alexei Starovoitov <alexei.starovoitov@gmail.com> wrote:
> On Tue, Oct 1, 2024 at 12:54 PM Dave Thaler <dthaler1968@googlemail.com>
> wrote:
[...]
> > I'm adding bpf@ietf.org to the To line since all changes in the
> > standardization directory should include that mailing list.
> >
> > The WG should discuss whether any changes should be done via a new RFC
> > that obsoletes the first one, or as RFCs that Update and just describe
> > deltas (additions, etc.).
> >
> > There are precedents both ways and I don't have a strong preference,
> > but I have a weak preference for delta-based ones since they're
> > shorter and are less likely to re-open discussion on previously
> > resolved issues, thus often saving the WG time.
> 
> Delta-based additions make sense to me.
> 
> > Also FYI to Linux kernel folks:
> > With WG and AD approval, it's also possible (but not ideal) to take
> > changes at AUTH48.  That'd be up to the chairs and AD to decide
> > though, and normally that's just for purely editorial clarifications,
> > e.g., to confusion called out by the RFC editor pass.
> 
> Also agree. We should keep AUTH going its course as-is.
> All ISA additions can be in the future delta RFC.
> 
> As far as file logistics... my preference is to keep
> Documentation/bpf/standardization/instruction-set.rst
> up to date.
> Right now it's effectively frozen while awaiting changes (if any) necessary for AUTH.
> After official RFC is issued we can start landing patches into instruction-set.rst and
> git diff 04efaebd72d1..whatever_future_sha instruction-set.rst will automatically
> generate the future delta RFC.
> Once RFC number is issued we can add a git tag for the particular sha that was the
> base for RFC as a documentation step and to simplify future 'git diff'.

My concern is that index.rst says:
> This directory contains documents that are being iterated on as part of the BPF
> standardization effort with the IETF. See the `IETF BPF Working Group`_ page
> for the working group charter, documents, and more.

So having a document that is NOT part of the IETF BPF Working Group would seem
out of place and, in my view, better located up a level (outside standardization).

Here’s some examples of delta-based RFCs which explain the gap and provide
the addition or clarification, and formally Update (not replace/obsolete) the original
RFC:
* https://www.rfc-editor.org/rfc/rfc6585.html: Additional HTTP Status Codes
* https://www.rfc-editor.org/rfc/rfc6840.html: Clarifications and Implementation Notes
   for DNS Security (DNSSEC)
* https://www.rfc-editor.org/rfc/rfc9295.html: Clarifications for Ed25519, Ed448,
   X25519, and X448 Algorithm Identifiers
* https://www.rfc-editor.org/rfc/rfc5756.html: Updates for RSAES-OAEP and 
   RSASSA-PSS Algorithm Parameters

Having a full document too is valuable but unless the IETF BPF WG
decides to take on a -bis document, I'd suggest keeping it out of the "standardization"
(say up 1 level) to avoid confusion, and just have one or more delta-based rst files
in the standardization directory.

Dave



-- 
Bpf mailing list -- bpf@ietf.org
To unsubscribe send an email to bpf-leave@ietf.org

Re: [PATCH bpf-next] docs/bpf: Document some special sdiv/smod operations

[Alexei Starovoitov]

On Thu, Nov 7, 2024 at 6:30 PM Dave Thaler <dthaler1968@googlemail.com> wrote:
>
>
> Alexei Starovoitov <alexei.starovoitov@gmail.com> wrote:
> > On Tue, Oct 1, 2024 at 12:54 PM Dave Thaler <dthaler1968@googlemail.com>
> > wrote:
> [...]
> > > I'm adding bpf@ietf.org to the To line since all changes in the
> > > standardization directory should include that mailing list.
> > >
> > > The WG should discuss whether any changes should be done via a new RFC
> > > that obsoletes the first one, or as RFCs that Update and just describe
> > > deltas (additions, etc.).
> > >
> > > There are precedents both ways and I don't have a strong preference,
> > > but I have a weak preference for delta-based ones since they're
> > > shorter and are less likely to re-open discussion on previously
> > > resolved issues, thus often saving the WG time.
> >
> > Delta-based additions make sense to me.
> >
> > > Also FYI to Linux kernel folks:
> > > With WG and AD approval, it's also possible (but not ideal) to take
> > > changes at AUTH48.  That'd be up to the chairs and AD to decide
> > > though, and normally that's just for purely editorial clarifications,
> > > e.g., to confusion called out by the RFC editor pass.
> >
> > Also agree. We should keep AUTH going its course as-is.
> > All ISA additions can be in the future delta RFC.
> >
> > As far as file logistics... my preference is to keep
> > Documentation/bpf/standardization/instruction-set.rst
> > up to date.
> > Right now it's effectively frozen while awaiting changes (if any) necessary for AUTH.
> > After official RFC is issued we can start landing patches into instruction-set.rst and
> > git diff 04efaebd72d1..whatever_future_sha instruction-set.rst will automatically
> > generate the future delta RFC.
> > Once RFC number is issued we can add a git tag for the particular sha that was the
> > base for RFC as a documentation step and to simplify future 'git diff'.
>
> My concern is that index.rst says:
> > This directory contains documents that are being iterated on as part of the BPF
> > standardization effort with the IETF. See the `IETF BPF Working Group`_ page
> > for the working group charter, documents, and more.
>
> So having a document that is NOT part of the IETF BPF Working Group would seem
> out of place and, in my view, better located up a level (outside standardization).

It's a part of bpf wg. It's not a new document.

> Here’s some examples of delta-based RFCs which explain the gap and provide
> the addition or clarification, and formally Update (not replace/obsolete) the original
> RFC:
> * https://www.rfc-editor.org/rfc/rfc6585.html: Additional HTTP Status Codes
> * https://www.rfc-editor.org/rfc/rfc6840.html: Clarifications and Implementation Notes
>    for DNS Security (DNSSEC)
> * https://www.rfc-editor.org/rfc/rfc9295.html: Clarifications for Ed25519, Ed448,
>    X25519, and X448 Algorithm Identifiers
> * https://www.rfc-editor.org/rfc/rfc5756.html: Updates for RSAES-OAEP and
>    RSASSA-PSS Algorithm Parameters
>
> Having a full document too is valuable but unless the IETF BPF WG
> decides to take on a -bis document, I'd suggest keeping it out of the "standardization"
> (say up 1 level) to avoid confusion, and just have one or more delta-based rst files
> in the standardization directory.

This patch is effectively a fix to the standard.
It's a standard git development process when fixes are applied
to the existing document.
Forking the whole doc into a different file just to apply fixes
makes no sense to me.

The formal delta-s for IETF can be created out of git.
We only need to tag the current version and then
git diff rfc9669_tag..HEAD
will give us that delta.
That will satisfy IETF process and won't mess up normal git style
kernel development.

btw do we still need to do any minor edit/fixes to instruction-set.rst
before tagging it as RFC9669 ?

[Bpf] Re: [PATCH bpf-next] docs/bpf: Document some special sdiv/smod operations

[Alexei Starovoitov]

On Thu, Nov 7, 2024 at 6:30 PM Dave Thaler <dthaler1968@googlemail.com> wrote:
>
>
> Alexei Starovoitov <alexei.starovoitov@gmail.com> wrote:
> > On Tue, Oct 1, 2024 at 12:54 PM Dave Thaler <dthaler1968@googlemail.com>
> > wrote:
> [...]
> > > I'm adding bpf@ietf.org to the To line since all changes in the
> > > standardization directory should include that mailing list.
> > >
> > > The WG should discuss whether any changes should be done via a new RFC
> > > that obsoletes the first one, or as RFCs that Update and just describe
> > > deltas (additions, etc.).
> > >
> > > There are precedents both ways and I don't have a strong preference,
> > > but I have a weak preference for delta-based ones since they're
> > > shorter and are less likely to re-open discussion on previously
> > > resolved issues, thus often saving the WG time.
> >
> > Delta-based additions make sense to me.
> >
> > > Also FYI to Linux kernel folks:
> > > With WG and AD approval, it's also possible (but not ideal) to take
> > > changes at AUTH48.  That'd be up to the chairs and AD to decide
> > > though, and normally that's just for purely editorial clarifications,
> > > e.g., to confusion called out by the RFC editor pass.
> >
> > Also agree. We should keep AUTH going its course as-is.
> > All ISA additions can be in the future delta RFC.
> >
> > As far as file logistics... my preference is to keep
> > Documentation/bpf/standardization/instruction-set.rst
> > up to date.
> > Right now it's effectively frozen while awaiting changes (if any) necessary for AUTH.
> > After official RFC is issued we can start landing patches into instruction-set.rst and
> > git diff 04efaebd72d1..whatever_future_sha instruction-set.rst will automatically
> > generate the future delta RFC.
> > Once RFC number is issued we can add a git tag for the particular sha that was the
> > base for RFC as a documentation step and to simplify future 'git diff'.
>
> My concern is that index.rst says:
> > This directory contains documents that are being iterated on as part of the BPF
> > standardization effort with the IETF. See the `IETF BPF Working Group`_ page
> > for the working group charter, documents, and more.
>
> So having a document that is NOT part of the IETF BPF Working Group would seem
> out of place and, in my view, better located up a level (outside standardization).

It's a part of bpf wg. It's not a new document.

> Here’s some examples of delta-based RFCs which explain the gap and provide
> the addition or clarification, and formally Update (not replace/obsolete) the original
> RFC:
> * https://www.rfc-editor.org/rfc/rfc6585.html: Additional HTTP Status Codes
> * https://www.rfc-editor.org/rfc/rfc6840.html: Clarifications and Implementation Notes
>    for DNS Security (DNSSEC)
> * https://www.rfc-editor.org/rfc/rfc9295.html: Clarifications for Ed25519, Ed448,
>    X25519, and X448 Algorithm Identifiers
> * https://www.rfc-editor.org/rfc/rfc5756.html: Updates for RSAES-OAEP and
>    RSASSA-PSS Algorithm Parameters
>
> Having a full document too is valuable but unless the IETF BPF WG
> decides to take on a -bis document, I'd suggest keeping it out of the "standardization"
> (say up 1 level) to avoid confusion, and just have one or more delta-based rst files
> in the standardization directory.

This patch is effectively a fix to the standard.
It's a standard git development process when fixes are applied
to the existing document.
Forking the whole doc into a different file just to apply fixes
makes no sense to me.

The formal delta-s for IETF can be created out of git.
We only need to tag the current version and then
git diff rfc9669_tag..HEAD
will give us that delta.
That will satisfy IETF process and won't mess up normal git style
kernel development.

btw do we still need to do any minor edit/fixes to instruction-set.rst
before tagging it as RFC9669 ?

-- 
Bpf mailing list -- bpf@ietf.org
To unsubscribe send an email to bpf-leave@ietf.org

RE: [PATCH bpf-next] docs/bpf: Document some special sdiv/smod operations

[Dave Thaler]

> -----Original Message-----
> From: Alexei Starovoitov <alexei.starovoitov@gmail.com>
> Sent: Friday, November 8, 2024 10:38 AM
> To: Dave Thaler <dthaler1968@googlemail.com>
> Cc: Yonghong Song <yonghong.song@linux.dev>; bpf@ietf.org; bpf
> <bpf@vger.kernel.org>; Alexei Starovoitov <ast@kernel.org>; Andrii Nakryiko
> <andrii@kernel.org>; Daniel Borkmann <daniel@iogearbox.net>; Martin KaFai Lau
> <martin.lau@kernel.org>
> Subject: Re: [PATCH bpf-next] docs/bpf: Document some special sdiv/smod
> operations
> 
> On Thu, Nov 7, 2024 at 6:30 PM Dave Thaler <dthaler1968@googlemail.com>
> wrote:
> >
> >
> > Alexei Starovoitov <alexei.starovoitov@gmail.com> wrote:
> > > On Tue, Oct 1, 2024 at 12:54 PM Dave Thaler
> > > <dthaler1968@googlemail.com>
> > > wrote:
> > [...]
> > > > I'm adding bpf@ietf.org to the To line since all changes in the
> > > > standardization directory should include that mailing list.
> > > >
> > > > The WG should discuss whether any changes should be done via a new
> > > > RFC that obsoletes the first one, or as RFCs that Update and just
> > > > describe deltas (additions, etc.).
> > > >
> > > > There are precedents both ways and I don't have a strong
> > > > preference, but I have a weak preference for delta-based ones
> > > > since they're shorter and are less likely to re-open discussion on
> > > > previously resolved issues, thus often saving the WG time.
> > >
> > > Delta-based additions make sense to me.
> > >
> > > > Also FYI to Linux kernel folks:
> > > > With WG and AD approval, it's also possible (but not ideal) to
> > > > take changes at AUTH48.  That'd be up to the chairs and AD to
> > > > decide though, and normally that's just for purely editorial
> > > > clarifications, e.g., to confusion called out by the RFC editor pass.
> > >
> > > Also agree. We should keep AUTH going its course as-is.
> > > All ISA additions can be in the future delta RFC.
> > >
> > > As far as file logistics... my preference is to keep
> > > Documentation/bpf/standardization/instruction-set.rst
> > > up to date.
> > > Right now it's effectively frozen while awaiting changes (if any) necessary for
> AUTH.
> > > After official RFC is issued we can start landing patches into
> > > instruction-set.rst and git diff 04efaebd72d1..whatever_future_sha
> > > instruction-set.rst will automatically generate the future delta RFC.
> > > Once RFC number is issued we can add a git tag for the particular
> > > sha that was the base for RFC as a documentation step and to simplify future 'git
> diff'.
> >
> > My concern is that index.rst says:
> > > This directory contains documents that are being iterated on as part
> > > of the BPF standardization effort with the IETF. See the `IETF BPF
> > > Working Group`_ page for the working group charter, documents, and more.
> >
> > So having a document that is NOT part of the IETF BPF Working Group
> > would seem out of place and, in my view, better located up a level (outside
> standardization).
> 
> It's a part of bpf wg. It's not a new document.

RFC 9669 is immutable.  Any additions require a new document, in
IETF terminology, since would result in a new RFC number.

> > Here’s some examples of delta-based RFCs which explain the gap and
> > provide the addition or clarification, and formally Update (not
> > replace/obsolete) the original
> > RFC:
> > * https://www.rfc-editor.org/rfc/rfc6585.html: Additional HTTP Status
> > Codes
> > * https://www.rfc-editor.org/rfc/rfc6840.html: Clarifications and Implementation
> Notes
> >    for DNS Security (DNSSEC)
> > * https://www.rfc-editor.org/rfc/rfc9295.html: Clarifications for Ed25519, Ed448,
> >    X25519, and X448 Algorithm Identifiers
> > * https://www.rfc-editor.org/rfc/rfc5756.html: Updates for RSAES-OAEP and
> >    RSASSA-PSS Algorithm Parameters
> >
> > Having a full document too is valuable but unless the IETF BPF WG
> > decides to take on a -bis document, I'd suggest keeping it out of the
> "standardization"
> > (say up 1 level) to avoid confusion, and just have one or more
> > delta-based rst files in the standardization directory.
> 
> This patch is effectively a fix to the standard.

Two of the examples I provided above fit into that category.
Two are examples of adding new codepoints.

> It's a standard git development process when fixes are applied to the existing
> document.
> Forking the whole doc into a different file just to apply fixes makes no sense to me.

Welcome to the IETF and immutable RFCs 😊

> The formal delta-s for IETF can be created out of git.

Not in the IETF per se, since a new document needs new boilerplate, with
a new abstract, introduction, etc.  At most, part of the document could be created
out of git, but I'm not convinced that git diffs alone (as opposed to some English
prose too for each, as in the examples I cited) make for good content in an IETF document.

> We only need to tag the current version and then git diff rfc9669_tag..HEAD will give
> us that delta.
> That will satisfy IETF process and won't mess up normal git style kernel
> development.

I am not convinced it is sufficient.  Can you point to any precedents in the IETF for
such an approach?  I can't offhand... See the RFC 5756 reference above for what
I mean by English prose for each diff.

> btw do we still need to do any minor edit/fixes to instruction-set.rst before tagging it
> as RFC9669 ?

Yes, we need to backport the formatting/nits from the RFC editor pass.

Dave

[Bpf] Re: [PATCH bpf-next] docs/bpf: Document some special sdiv/smod operations

[Dave Thaler]

> -----Original Message-----
> From: Alexei Starovoitov <alexei.starovoitov@gmail.com>
> Sent: Friday, November 8, 2024 10:38 AM
> To: Dave Thaler <dthaler1968@googlemail.com>
> Cc: Yonghong Song <yonghong.song@linux.dev>; bpf@ietf.org; bpf
> <bpf@vger.kernel.org>; Alexei Starovoitov <ast@kernel.org>; Andrii Nakryiko
> <andrii@kernel.org>; Daniel Borkmann <daniel@iogearbox.net>; Martin KaFai Lau
> <martin.lau@kernel.org>
> Subject: Re: [PATCH bpf-next] docs/bpf: Document some special sdiv/smod
> operations
> 
> On Thu, Nov 7, 2024 at 6:30 PM Dave Thaler <dthaler1968@googlemail.com>
> wrote:
> >
> >
> > Alexei Starovoitov <alexei.starovoitov@gmail.com> wrote:
> > > On Tue, Oct 1, 2024 at 12:54 PM Dave Thaler
> > > <dthaler1968@googlemail.com>
> > > wrote:
> > [...]
> > > > I'm adding bpf@ietf.org to the To line since all changes in the
> > > > standardization directory should include that mailing list.
> > > >
> > > > The WG should discuss whether any changes should be done via a new
> > > > RFC that obsoletes the first one, or as RFCs that Update and just
> > > > describe deltas (additions, etc.).
> > > >
> > > > There are precedents both ways and I don't have a strong
> > > > preference, but I have a weak preference for delta-based ones
> > > > since they're shorter and are less likely to re-open discussion on
> > > > previously resolved issues, thus often saving the WG time.
> > >
> > > Delta-based additions make sense to me.
> > >
> > > > Also FYI to Linux kernel folks:
> > > > With WG and AD approval, it's also possible (but not ideal) to
> > > > take changes at AUTH48.  That'd be up to the chairs and AD to
> > > > decide though, and normally that's just for purely editorial
> > > > clarifications, e.g., to confusion called out by the RFC editor pass.
> > >
> > > Also agree. We should keep AUTH going its course as-is.
> > > All ISA additions can be in the future delta RFC.
> > >
> > > As far as file logistics... my preference is to keep
> > > Documentation/bpf/standardization/instruction-set.rst
> > > up to date.
> > > Right now it's effectively frozen while awaiting changes (if any) necessary for
> AUTH.
> > > After official RFC is issued we can start landing patches into
> > > instruction-set.rst and git diff 04efaebd72d1..whatever_future_sha
> > > instruction-set.rst will automatically generate the future delta RFC.
> > > Once RFC number is issued we can add a git tag for the particular
> > > sha that was the base for RFC as a documentation step and to simplify future 'git
> diff'.
> >
> > My concern is that index.rst says:
> > > This directory contains documents that are being iterated on as part
> > > of the BPF standardization effort with the IETF. See the `IETF BPF
> > > Working Group`_ page for the working group charter, documents, and more.
> >
> > So having a document that is NOT part of the IETF BPF Working Group
> > would seem out of place and, in my view, better located up a level (outside
> standardization).
> 
> It's a part of bpf wg. It's not a new document.

RFC 9669 is immutable.  Any additions require a new document, in
IETF terminology, since would result in a new RFC number.

> > Here’s some examples of delta-based RFCs which explain the gap and
> > provide the addition or clarification, and formally Update (not
> > replace/obsolete) the original
> > RFC:
> > * https://www.rfc-editor.org/rfc/rfc6585.html: Additional HTTP Status
> > Codes
> > * https://www.rfc-editor.org/rfc/rfc6840.html: Clarifications and Implementation
> Notes
> >    for DNS Security (DNSSEC)
> > * https://www.rfc-editor.org/rfc/rfc9295.html: Clarifications for Ed25519, Ed448,
> >    X25519, and X448 Algorithm Identifiers
> > * https://www.rfc-editor.org/rfc/rfc5756.html: Updates for RSAES-OAEP and
> >    RSASSA-PSS Algorithm Parameters
> >
> > Having a full document too is valuable but unless the IETF BPF WG
> > decides to take on a -bis document, I'd suggest keeping it out of the
> "standardization"
> > (say up 1 level) to avoid confusion, and just have one or more
> > delta-based rst files in the standardization directory.
> 
> This patch is effectively a fix to the standard.

Two of the examples I provided above fit into that category.
Two are examples of adding new codepoints.

> It's a standard git development process when fixes are applied to the existing
> document.
> Forking the whole doc into a different file just to apply fixes makes no sense to me.

Welcome to the IETF and immutable RFCs 😊

> The formal delta-s for IETF can be created out of git.

Not in the IETF per se, since a new document needs new boilerplate, with
a new abstract, introduction, etc.  At most, part of the document could be created
out of git, but I'm not convinced that git diffs alone (as opposed to some English
prose too for each, as in the examples I cited) make for good content in an IETF document.

> We only need to tag the current version and then git diff rfc9669_tag..HEAD will give
> us that delta.
> That will satisfy IETF process and won't mess up normal git style kernel
> development.

I am not convinced it is sufficient.  Can you point to any precedents in the IETF for
such an approach?  I can't offhand... See the RFC 5756 reference above for what
I mean by English prose for each diff.

> btw do we still need to do any minor edit/fixes to instruction-set.rst before tagging it
> as RFC9669 ?

Yes, we need to backport the formatting/nits from the RFC editor pass.

Dave

-- 
Bpf mailing list -- bpf@ietf.org
To unsubscribe send an email to bpf-leave@ietf.org

Re: [PATCH bpf-next] docs/bpf: Document some special sdiv/smod operations

[Yonghong Song]

On 11/8/24 10:53 AM, Dave Thaler wrote:
>> -----Original Message-----
>> From: Alexei Starovoitov <alexei.starovoitov@gmail.com>
>> Sent: Friday, November 8, 2024 10:38 AM
>> To: Dave Thaler <dthaler1968@googlemail.com>
>> Cc: Yonghong Song <yonghong.song@linux.dev>; bpf@ietf.org; bpf
>> <bpf@vger.kernel.org>; Alexei Starovoitov <ast@kernel.org>; Andrii Nakryiko
>> <andrii@kernel.org>; Daniel Borkmann <daniel@iogearbox.net>; Martin KaFai Lau
>> <martin.lau@kernel.org>
>> Subject: Re: [PATCH bpf-next] docs/bpf: Document some special sdiv/smod
>> operations
>>
>> On Thu, Nov 7, 2024 at 6:30 PM Dave Thaler <dthaler1968@googlemail.com>
>> wrote:
>>>
>>> Alexei Starovoitov <alexei.starovoitov@gmail.com> wrote:
>>>> On Tue, Oct 1, 2024 at 12:54 PM Dave Thaler
>>>> <dthaler1968@googlemail.com>
>>>> wrote:
>>> [...]
>>>>> I'm adding bpf@ietf.org to the To line since all changes in the
>>>>> standardization directory should include that mailing list.
>>>>>
>>>>> The WG should discuss whether any changes should be done via a new
>>>>> RFC that obsoletes the first one, or as RFCs that Update and just
>>>>> describe deltas (additions, etc.).
>>>>>
>>>>> There are precedents both ways and I don't have a strong
>>>>> preference, but I have a weak preference for delta-based ones
>>>>> since they're shorter and are less likely to re-open discussion on
>>>>> previously resolved issues, thus often saving the WG time.
>>>> Delta-based additions make sense to me.
>>>>
>>>>> Also FYI to Linux kernel folks:
>>>>> With WG and AD approval, it's also possible (but not ideal) to
>>>>> take changes at AUTH48.  That'd be up to the chairs and AD to
>>>>> decide though, and normally that's just for purely editorial
>>>>> clarifications, e.g., to confusion called out by the RFC editor pass.
>>>> Also agree. We should keep AUTH going its course as-is.
>>>> All ISA additions can be in the future delta RFC.
>>>>
>>>> As far as file logistics... my preference is to keep
>>>> Documentation/bpf/standardization/instruction-set.rst
>>>> up to date.
>>>> Right now it's effectively frozen while awaiting changes (if any) necessary for
>> AUTH.
>>>> After official RFC is issued we can start landing patches into
>>>> instruction-set.rst and git diff 04efaebd72d1..whatever_future_sha
>>>> instruction-set.rst will automatically generate the future delta RFC.
>>>> Once RFC number is issued we can add a git tag for the particular
>>>> sha that was the base for RFC as a documentation step and to simplify future 'git
>> diff'.
>>> My concern is that index.rst says:
>>>> This directory contains documents that are being iterated on as part
>>>> of the BPF standardization effort with the IETF. See the `IETF BPF
>>>> Working Group`_ page for the working group charter, documents, and more.
>>> So having a document that is NOT part of the IETF BPF Working Group
>>> would seem out of place and, in my view, better located up a level (outside
>> standardization).
>>
>> It's a part of bpf wg. It's not a new document.
> RFC 9669 is immutable.  Any additions require a new document, in
> IETF terminology, since would result in a new RFC number.
>
>>> Here’s some examples of delta-based RFCs which explain the gap and
>>> provide the addition or clarification, and formally Update (not
>>> replace/obsolete) the original
>>> RFC:
>>> * https://www.rfc-editor.org/rfc/rfc6585.html: Additional HTTP Status
>>> Codes
>>> * https://www.rfc-editor.org/rfc/rfc6840.html: Clarifications and Implementation
>> Notes
>>>     for DNS Security (DNSSEC)
>>> * https://www.rfc-editor.org/rfc/rfc9295.html: Clarifications for Ed25519, Ed448,
>>>     X25519, and X448 Algorithm Identifiers
>>> * https://www.rfc-editor.org/rfc/rfc5756.html: Updates for RSAES-OAEP and
>>>     RSASSA-PSS Algorithm Parameters
>>>
>>> Having a full document too is valuable but unless the IETF BPF WG
>>> decides to take on a -bis document, I'd suggest keeping it out of the
>> "standardization"
>>> (say up 1 level) to avoid confusion, and just have one or more
>>> delta-based rst files in the standardization directory.
>> This patch is effectively a fix to the standard.
> Two of the examples I provided above fit into that category.
> Two are examples of adding new codepoints.
>
>> It's a standard git development process when fixes are applied to the existing
>> document.
>> Forking the whole doc into a different file just to apply fixes makes no sense to me.
> Welcome to the IETF and immutable RFCs 😊
>
>> The formal delta-s for IETF can be created out of git.
> Not in the IETF per se, since a new document needs new boilerplate, with
> a new abstract, introduction, etc.  At most, part of the document could be created
> out of git, but I'm not convinced that git diffs alone (as opposed to some English
> prose too for each, as in the examples I cited) make for good content in an IETF document.
>
>> We only need to tag the current version and then git diff rfc9669_tag..HEAD will give
>> us that delta.
>> That will satisfy IETF process and won't mess up normal git style kernel
>> development.
> I am not convinced it is sufficient.  Can you point to any precedents in the IETF for
> such an approach?  I can't offhand... See the RFC 5756 reference above for what
> I mean by English prose for each diff.

I think we can add sufficient details in the commit message. What things we need to
put in the commit message to satisfy the rIETF equirement?

>> btw do we still need to do any minor edit/fixes to instruction-set.rst before tagging it
>> as RFC9669 ?
> Yes, we need to backport the formatting/nits from the RFC editor pass.
>
> Dave
>

[Bpf] Re: [PATCH bpf-next] docs/bpf: Document some special sdiv/smod operations

[Yonghong Song]

On 11/8/24 10:53 AM, Dave Thaler wrote:
>> -----Original Message-----
>> From: Alexei Starovoitov <alexei.starovoitov@gmail.com>
>> Sent: Friday, November 8, 2024 10:38 AM
>> To: Dave Thaler <dthaler1968@googlemail.com>
>> Cc: Yonghong Song <yonghong.song@linux.dev>; bpf@ietf.org; bpf
>> <bpf@vger.kernel.org>; Alexei Starovoitov <ast@kernel.org>; Andrii Nakryiko
>> <andrii@kernel.org>; Daniel Borkmann <daniel@iogearbox.net>; Martin KaFai Lau
>> <martin.lau@kernel.org>
>> Subject: Re: [PATCH bpf-next] docs/bpf: Document some special sdiv/smod
>> operations
>>
>> On Thu, Nov 7, 2024 at 6:30 PM Dave Thaler <dthaler1968@googlemail.com>
>> wrote:
>>>
>>> Alexei Starovoitov <alexei.starovoitov@gmail.com> wrote:
>>>> On Tue, Oct 1, 2024 at 12:54 PM Dave Thaler
>>>> <dthaler1968@googlemail.com>
>>>> wrote:
>>> [...]
>>>>> I'm adding bpf@ietf.org to the To line since all changes in the
>>>>> standardization directory should include that mailing list.
>>>>>
>>>>> The WG should discuss whether any changes should be done via a new
>>>>> RFC that obsoletes the first one, or as RFCs that Update and just
>>>>> describe deltas (additions, etc.).
>>>>>
>>>>> There are precedents both ways and I don't have a strong
>>>>> preference, but I have a weak preference for delta-based ones
>>>>> since they're shorter and are less likely to re-open discussion on
>>>>> previously resolved issues, thus often saving the WG time.
>>>> Delta-based additions make sense to me.
>>>>
>>>>> Also FYI to Linux kernel folks:
>>>>> With WG and AD approval, it's also possible (but not ideal) to
>>>>> take changes at AUTH48.  That'd be up to the chairs and AD to
>>>>> decide though, and normally that's just for purely editorial
>>>>> clarifications, e.g., to confusion called out by the RFC editor pass.
>>>> Also agree. We should keep AUTH going its course as-is.
>>>> All ISA additions can be in the future delta RFC.
>>>>
>>>> As far as file logistics... my preference is to keep
>>>> Documentation/bpf/standardization/instruction-set.rst
>>>> up to date.
>>>> Right now it's effectively frozen while awaiting changes (if any) necessary for
>> AUTH.
>>>> After official RFC is issued we can start landing patches into
>>>> instruction-set.rst and git diff 04efaebd72d1..whatever_future_sha
>>>> instruction-set.rst will automatically generate the future delta RFC.
>>>> Once RFC number is issued we can add a git tag for the particular
>>>> sha that was the base for RFC as a documentation step and to simplify future 'git
>> diff'.
>>> My concern is that index.rst says:
>>>> This directory contains documents that are being iterated on as part
>>>> of the BPF standardization effort with the IETF. See the `IETF BPF
>>>> Working Group`_ page for the working group charter, documents, and more.
>>> So having a document that is NOT part of the IETF BPF Working Group
>>> would seem out of place and, in my view, better located up a level (outside
>> standardization).
>>
>> It's a part of bpf wg. It's not a new document.
> RFC 9669 is immutable.  Any additions require a new document, in
> IETF terminology, since would result in a new RFC number.
>
>>> Here’s some examples of delta-based RFCs which explain the gap and
>>> provide the addition or clarification, and formally Update (not
>>> replace/obsolete) the original
>>> RFC:
>>> * https://www.rfc-editor.org/rfc/rfc6585.html: Additional HTTP Status
>>> Codes
>>> * https://www.rfc-editor.org/rfc/rfc6840.html: Clarifications and Implementation
>> Notes
>>>     for DNS Security (DNSSEC)
>>> * https://www.rfc-editor.org/rfc/rfc9295.html: Clarifications for Ed25519, Ed448,
>>>     X25519, and X448 Algorithm Identifiers
>>> * https://www.rfc-editor.org/rfc/rfc5756.html: Updates for RSAES-OAEP and
>>>     RSASSA-PSS Algorithm Parameters
>>>
>>> Having a full document too is valuable but unless the IETF BPF WG
>>> decides to take on a -bis document, I'd suggest keeping it out of the
>> "standardization"
>>> (say up 1 level) to avoid confusion, and just have one or more
>>> delta-based rst files in the standardization directory.
>> This patch is effectively a fix to the standard.
> Two of the examples I provided above fit into that category.
> Two are examples of adding new codepoints.
>
>> It's a standard git development process when fixes are applied to the existing
>> document.
>> Forking the whole doc into a different file just to apply fixes makes no sense to me.
> Welcome to the IETF and immutable RFCs 😊
>
>> The formal delta-s for IETF can be created out of git.
> Not in the IETF per se, since a new document needs new boilerplate, with
> a new abstract, introduction, etc.  At most, part of the document could be created
> out of git, but I'm not convinced that git diffs alone (as opposed to some English
> prose too for each, as in the examples I cited) make for good content in an IETF document.
>
>> We only need to tag the current version and then git diff rfc9669_tag..HEAD will give
>> us that delta.
>> That will satisfy IETF process and won't mess up normal git style kernel
>> development.
> I am not convinced it is sufficient.  Can you point to any precedents in the IETF for
> such an approach?  I can't offhand... See the RFC 5756 reference above for what
> I mean by English prose for each diff.

I think we can add sufficient details in the commit message. What things we need to
put in the commit message to satisfy the rIETF equirement?

>> btw do we still need to do any minor edit/fixes to instruction-set.rst before tagging it
>> as RFC9669 ?
> Yes, we need to backport the formatting/nits from the RFC editor pass.
>
> Dave
>

-- 
Bpf mailing list -- bpf@ietf.org
To unsubscribe send an email to bpf-leave@ietf.org

Re: [PATCH bpf-next] docs/bpf: Document some special sdiv/smod operations

[Alexei Starovoitov]

On Fri, Nov 8, 2024 at 10:53 AM Dave Thaler <dthaler1968@googlemail.com> wrote:
>
> > >
> > > My concern is that index.rst says:
> > > > This directory contains documents that are being iterated on as part
> > > > of the BPF standardization effort with the IETF. See the `IETF BPF
> > > > Working Group`_ page for the working group charter, documents, and more.
> > >
> > > So having a document that is NOT part of the IETF BPF Working Group
> > > would seem out of place and, in my view, better located up a level (outside
> > standardization).
> >
> > It's a part of bpf wg. It's not a new document.
>
> RFC 9669 is immutable.  Any additions require a new document, in
> IETF terminology, since would result in a new RFC number.

Sure. It's an IETF process. Not arguing about that.

> > > Here’s some examples of delta-based RFCs which explain the gap and
> > > provide the addition or clarification, and formally Update (not
> > > replace/obsolete) the original
> > > RFC:
> > > * https://www.rfc-editor.org/rfc/rfc6585.html: Additional HTTP Status
> > > Codes
> > > * https://www.rfc-editor.org/rfc/rfc6840.html: Clarifications and Implementation
> > Notes
> > >    for DNS Security (DNSSEC)
> > > * https://www.rfc-editor.org/rfc/rfc9295.html: Clarifications for Ed25519, Ed448,
> > >    X25519, and X448 Algorithm Identifiers
> > > * https://www.rfc-editor.org/rfc/rfc5756.html: Updates for RSAES-OAEP and
> > >    RSASSA-PSS Algorithm Parameters
> > >
> > > Having a full document too is valuable but unless the IETF BPF WG
> > > decides to take on a -bis document, I'd suggest keeping it out of the
> > "standardization"
> > > (say up 1 level) to avoid confusion, and just have one or more
> > > delta-based rst files in the standardization directory.
> >
> > This patch is effectively a fix to the standard.
>
> Two of the examples I provided above fit into that category.
> Two are examples of adding new codepoints.
>
> > It's a standard git development process when fixes are applied to the existing
> > document.
> > Forking the whole doc into a different file just to apply fixes makes no sense to me.
>
> Welcome to the IETF and immutable RFCs 😊
>
> > The formal delta-s for IETF can be created out of git.
>
> Not in the IETF per se, since a new document needs new boilerplate, with
> a new abstract, introduction, etc.  At most, part of the document could be created
> out of git, but I'm not convinced that git diffs alone (as opposed to some English
> prose too for each, as in the examples I cited) make for good content in an IETF document.

git diff might need another script :)
Just like you did earlier with an old script that took this .rst
and converted it to IETF suitable format.

Now we'd need a new script that will take git diff with new header/footer
and whatever extra words necessary.

> > We only need to tag the current version and then git diff rfc9669_tag..HEAD will give
> > us that delta.
> > That will satisfy IETF process and won't mess up normal git style kernel
> > development.
>
> I am not convinced it is sufficient.  Can you point to any precedents in the IETF for
> such an approach?  I can't offhand... See the RFC 5756 reference above for what
> I mean by English prose for each diff.

It's all a matter of additional scripting.
We're not going to ask every kernel developer to learn IETF process.
People will be sending patches for instruction-set.rst and
this file will keep evolving.
As soon as we land Yonghong's patch it won't be 1-1 with RFC9669 and
it's fine.
Even today it's not 1-1 either. It needs to go through your
existing script to fit IETF rules.
The new patches will keep landing and the file will become a working
document towards the next delta RFC.

> > btw do we still need to do any minor edit/fixes to instruction-set.rst before tagging it
> > as RFC9669 ?
>
> Yes, we need to backport the formatting/nits from the RFC editor pass.

Ok. Please send the patch. Will wait for that first.

[Bpf] Re: [PATCH bpf-next] docs/bpf: Document some special sdiv/smod operations

[Alexei Starovoitov]

On Fri, Nov 8, 2024 at 10:53 AM Dave Thaler <dthaler1968@googlemail.com> wrote:
>
> > >
> > > My concern is that index.rst says:
> > > > This directory contains documents that are being iterated on as part
> > > > of the BPF standardization effort with the IETF. See the `IETF BPF
> > > > Working Group`_ page for the working group charter, documents, and more.
> > >
> > > So having a document that is NOT part of the IETF BPF Working Group
> > > would seem out of place and, in my view, better located up a level (outside
> > standardization).
> >
> > It's a part of bpf wg. It's not a new document.
>
> RFC 9669 is immutable.  Any additions require a new document, in
> IETF terminology, since would result in a new RFC number.

Sure. It's an IETF process. Not arguing about that.

> > > Here’s some examples of delta-based RFCs which explain the gap and
> > > provide the addition or clarification, and formally Update (not
> > > replace/obsolete) the original
> > > RFC:
> > > * https://www.rfc-editor.org/rfc/rfc6585.html: Additional HTTP Status
> > > Codes
> > > * https://www.rfc-editor.org/rfc/rfc6840.html: Clarifications and Implementation
> > Notes
> > >    for DNS Security (DNSSEC)
> > > * https://www.rfc-editor.org/rfc/rfc9295.html: Clarifications for Ed25519, Ed448,
> > >    X25519, and X448 Algorithm Identifiers
> > > * https://www.rfc-editor.org/rfc/rfc5756.html: Updates for RSAES-OAEP and
> > >    RSASSA-PSS Algorithm Parameters
> > >
> > > Having a full document too is valuable but unless the IETF BPF WG
> > > decides to take on a -bis document, I'd suggest keeping it out of the
> > "standardization"
> > > (say up 1 level) to avoid confusion, and just have one or more
> > > delta-based rst files in the standardization directory.
> >
> > This patch is effectively a fix to the standard.
>
> Two of the examples I provided above fit into that category.
> Two are examples of adding new codepoints.
>
> > It's a standard git development process when fixes are applied to the existing
> > document.
> > Forking the whole doc into a different file just to apply fixes makes no sense to me.
>
> Welcome to the IETF and immutable RFCs 😊
>
> > The formal delta-s for IETF can be created out of git.
>
> Not in the IETF per se, since a new document needs new boilerplate, with
> a new abstract, introduction, etc.  At most, part of the document could be created
> out of git, but I'm not convinced that git diffs alone (as opposed to some English
> prose too for each, as in the examples I cited) make for good content in an IETF document.

git diff might need another script :)
Just like you did earlier with an old script that took this .rst
and converted it to IETF suitable format.

Now we'd need a new script that will take git diff with new header/footer
and whatever extra words necessary.

> > We only need to tag the current version and then git diff rfc9669_tag..HEAD will give
> > us that delta.
> > That will satisfy IETF process and won't mess up normal git style kernel
> > development.
>
> I am not convinced it is sufficient.  Can you point to any precedents in the IETF for
> such an approach?  I can't offhand... See the RFC 5756 reference above for what
> I mean by English prose for each diff.

It's all a matter of additional scripting.
We're not going to ask every kernel developer to learn IETF process.
People will be sending patches for instruction-set.rst and
this file will keep evolving.
As soon as we land Yonghong's patch it won't be 1-1 with RFC9669 and
it's fine.
Even today it's not 1-1 either. It needs to go through your
existing script to fit IETF rules.
The new patches will keep landing and the file will become a working
document towards the next delta RFC.

> > btw do we still need to do any minor edit/fixes to instruction-set.rst before tagging it
> > as RFC9669 ?
>
> Yes, we need to backport the formatting/nits from the RFC editor pass.

Ok. Please send the patch. Will wait for that first.

-- 
Bpf mailing list -- bpf@ietf.org
To unsubscribe send an email to bpf-leave@ietf.org

Re: [PATCH bpf-next] docs/bpf: Document some special sdiv/smod operations

[Yonghong Song]

On 10/1/24 12:54 PM, Dave Thaler wrote:
> Yonghong Song <yonghong.song@linux.dev> wrote:
>> On 9/30/24 6:50 PM, Alexei Starovoitov wrote:
>>> On Thu, Sep 26, 2024 at 8:39 PM Yonghong Song <yonghong.song@linux.dev>
>> wrote:
>>>> Patch [1] fixed possible kernel crash due to specific sdiv/smod
>>>> operations in bpf program. The following are related operations and
>>>> the expected results of those operations:
>>>>     - LLONG_MIN/-1 = LLONG_MIN
>>>>     - INT_MIN/-1 = INT_MIN
>>>>     - LLONG_MIN%-1 = 0
>>>>     - INT_MIN%-1 = 0
>>>>
>>>> Those operations are replaced with codes which won't cause kernel
>>>> crash. This patch documents what operations may cause exception and
>>>> what replacement operations are.
>>>>
>>>>     [1]
>>>> https://lore.kernel.org/all/20240913150326.1187788-1-yonghong.song@li
>>>> nux.dev/
>>>>
>>>> Signed-off-by: Yonghong Song <yonghong.song@linux.dev>
>>>> ---
>>>>    .../bpf/standardization/instruction-set.rst   | 25 +++++++++++++++----
>>>>    1 file changed, 20 insertions(+), 5 deletions(-)
>>>>
>>>> diff --git a/Documentation/bpf/standardization/instruction-set.rst
>>>> b/Documentation/bpf/standardization/instruction-set.rst
>>>> index ab820d565052..d150c1d7ad3b 100644
>>>> --- a/Documentation/bpf/standardization/instruction-set.rst
>>>> +++ b/Documentation/bpf/standardization/instruction-set.rst
>>>> @@ -347,11 +347,26 @@ register.
>>>>      =====  =====  =======
>>>> ==========================================================
>>>>
>>>>    Underflow and overflow are allowed during arithmetic operations,
>>>> meaning -the 64-bit or 32-bit value will wrap. If BPF program
>>>> execution would -result in division by zero, the destination register is instead set
>> to zero.
>>>> -If execution would result in modulo by zero, for ``ALU64`` the value of
>>>> -the destination register is unchanged whereas for ``ALU`` the upper
>>>> -32 bits of the destination register are zeroed.
>>>> +the 64-bit or 32-bit value will wrap. There are also a few
>>>> +arithmetic operations which may cause exception for certain
>>>> +architectures. Since crashing the kernel is not an option, those operations are
>> replaced with alternative operations.
>>>> +
>>>> +.. table:: Arithmetic operations with possible exceptions
>>>> +
>>>> +  =====  ==========  =============================
>> ==========================
>>>> +  name   class       original                       replacement
>>>> +  =====  ==========  =============================
>> ==========================
>>>> +  DIV    ALU64/ALU   dst /= 0                       dst = 0
>>>> +  SDIV   ALU64/ALU   dst s/= 0                      dst = 0
>>>> +  MOD    ALU64       dst %= 0                       dst = dst (no replacement)
>>>> +  MOD    ALU         dst %= 0                       dst = (u32)dst
>>>> +  SMOD   ALU64       dst s%= 0                      dst = dst (no replacement)
>>>> +  SMOD   ALU         dst s%= 0                      dst = (u32)dst
> All of the above are already covered in existing Table 5 and in my opinion
> don't need to be repeated.

This tries to separate cases between ALU and ALU64. But I agree that the table
5 should be good enough.

>
> That is, the "original" is not what Table 5 has, so just introduces confusion
> in the document in my opinion.
>
>>>> +  SDIV   ALU64       dst s/= -1 (dst = LLONG_MIN)   dst = LLONG_MIN
>>>> +  SDIV   ALU         dst s/= -1 (dst = INT_MIN)     dst = (u32)INT_MIN
>>>> +  SMOD   ALU64       dst s%= -1 (dst = LLONG_MIN)   dst = 0
>>>> +  SMOD   ALU         dst s%= -1 (dst = INT_MIN)     dst = 0
> The above four are the new ones and I'd prefer a solution that modifies
> existing table 5.  E.g. table 5 has now for SMOD:
>
> dst = (src != 0) ? (dst s% src) : dst
>
> and could have something like this:
>
> dst = (src == 0) ? dst : ((src == -1 && dst == INT_MIN) ? 0 : (dst s% src))

Thanks. This indeed simpler. I can do this.

[Bpf] Re: [PATCH bpf-next] docs/bpf: Document some special sdiv/smod operations

[Yonghong Song]

On 10/1/24 12:54 PM, Dave Thaler wrote:
> Yonghong Song <yonghong.song@linux.dev> wrote:
>> On 9/30/24 6:50 PM, Alexei Starovoitov wrote:
>>> On Thu, Sep 26, 2024 at 8:39 PM Yonghong Song <yonghong.song@linux.dev>
>> wrote:
>>>> Patch [1] fixed possible kernel crash due to specific sdiv/smod
>>>> operations in bpf program. The following are related operations and
>>>> the expected results of those operations:
>>>>     - LLONG_MIN/-1 = LLONG_MIN
>>>>     - INT_MIN/-1 = INT_MIN
>>>>     - LLONG_MIN%-1 = 0
>>>>     - INT_MIN%-1 = 0
>>>>
>>>> Those operations are replaced with codes which won't cause kernel
>>>> crash. This patch documents what operations may cause exception and
>>>> what replacement operations are.
>>>>
>>>>     [1]
>>>> https://lore.kernel.org/all/20240913150326.1187788-1-yonghong.song@li
>>>> nux.dev/
>>>>
>>>> Signed-off-by: Yonghong Song <yonghong.song@linux.dev>
>>>> ---
>>>>    .../bpf/standardization/instruction-set.rst   | 25 +++++++++++++++----
>>>>    1 file changed, 20 insertions(+), 5 deletions(-)
>>>>
>>>> diff --git a/Documentation/bpf/standardization/instruction-set.rst
>>>> b/Documentation/bpf/standardization/instruction-set.rst
>>>> index ab820d565052..d150c1d7ad3b 100644
>>>> --- a/Documentation/bpf/standardization/instruction-set.rst
>>>> +++ b/Documentation/bpf/standardization/instruction-set.rst
>>>> @@ -347,11 +347,26 @@ register.
>>>>      =====  =====  =======
>>>> ==========================================================
>>>>
>>>>    Underflow and overflow are allowed during arithmetic operations,
>>>> meaning -the 64-bit or 32-bit value will wrap. If BPF program
>>>> execution would -result in division by zero, the destination register is instead set
>> to zero.
>>>> -If execution would result in modulo by zero, for ``ALU64`` the value of
>>>> -the destination register is unchanged whereas for ``ALU`` the upper
>>>> -32 bits of the destination register are zeroed.
>>>> +the 64-bit or 32-bit value will wrap. There are also a few
>>>> +arithmetic operations which may cause exception for certain
>>>> +architectures. Since crashing the kernel is not an option, those operations are
>> replaced with alternative operations.
>>>> +
>>>> +.. table:: Arithmetic operations with possible exceptions
>>>> +
>>>> +  =====  ==========  =============================
>> ==========================
>>>> +  name   class       original                       replacement
>>>> +  =====  ==========  =============================
>> ==========================
>>>> +  DIV    ALU64/ALU   dst /= 0                       dst = 0
>>>> +  SDIV   ALU64/ALU   dst s/= 0                      dst = 0
>>>> +  MOD    ALU64       dst %= 0                       dst = dst (no replacement)
>>>> +  MOD    ALU         dst %= 0                       dst = (u32)dst
>>>> +  SMOD   ALU64       dst s%= 0                      dst = dst (no replacement)
>>>> +  SMOD   ALU         dst s%= 0                      dst = (u32)dst
> All of the above are already covered in existing Table 5 and in my opinion
> don't need to be repeated.

This tries to separate cases between ALU and ALU64. But I agree that the table
5 should be good enough.

>
> That is, the "original" is not what Table 5 has, so just introduces confusion
> in the document in my opinion.
>
>>>> +  SDIV   ALU64       dst s/= -1 (dst = LLONG_MIN)   dst = LLONG_MIN
>>>> +  SDIV   ALU         dst s/= -1 (dst = INT_MIN)     dst = (u32)INT_MIN
>>>> +  SMOD   ALU64       dst s%= -1 (dst = LLONG_MIN)   dst = 0
>>>> +  SMOD   ALU         dst s%= -1 (dst = INT_MIN)     dst = 0
> The above four are the new ones and I'd prefer a solution that modifies
> existing table 5.  E.g. table 5 has now for SMOD:
>
> dst = (src != 0) ? (dst s% src) : dst
>
> and could have something like this:
>
> dst = (src == 0) ? dst : ((src == -1 && dst == INT_MIN) ? 0 : (dst s% src))

Thanks. This indeed simpler. I can do this.

-- 
Bpf mailing list -- bpf@ietf.org
To unsubscribe send an email to bpf-leave@ietf.org