Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ISA: Roman Danyliw's comments #158

Open
dthaler opened this issue Jun 20, 2024 · 3 comments
Open

ISA: Roman Danyliw's comments #158

dthaler opened this issue Jun 20, 2024 · 3 comments
Labels
fixed in editors copy Fixed in github repo ISA Affects ISA draft

Comments

@dthaler
Copy link
Collaborator

dthaler commented Jun 20, 2024


COMMENT:

Thank you to Ines Robles for the GENART review.

** Section 4.3.1
“Historically, each helper function was identified by a static ID encoded in
the 'imm' field. The available helper functions may differ for each program
type, but static IDs are unique across all program types.”

Are static IDs unique across all instances of a given platform running a
program? I don’t understand what a “program type” is.

** Section 4.3.1. As a document reader, I’m not sure what I do with this
section as it defines a concept of helper functions but doesn’t specify it.

** Section 7.1.1
“Registration requests for 'Provisional' registration can be included in an
Internet-Draft; when the documents expire or are approved for publication as an
RFC, the registration will be updated.”

In the case of an expired I-D, what does it mean for the “registration to be
updated”?

** Section 7.2
“This document proposes a new IANA registry for BPF instructions, as follows:”

The text in Section 7.1 says it is creating a “sub-registry”. Here it is a
“registry”. Why the discrepancy?

** Section 7.5

“'Provisional' registrations can be updated by the original registrant or
anyone designated by the original registrant.”

How does one perform this designation?

@dthaler dthaler added the ISA Affects ISA draft label Jun 20, 2024
@dthaler
Copy link
Collaborator Author

dthaler commented Jun 22, 2024

** Section 7.2
“This document proposes a new IANA registry for BPF instructions, as follows:”

The text in Section 7.1 says it is creating a “sub-registry”. Here it is a
“registry”. Why the discrepancy?

This point is covered by issue #157.

@dthaler
Copy link
Collaborator Author

dthaler commented Jun 24, 2024

** Section 4.3.1
“Historically, each helper function was identified by a static ID encoded in
the 'imm' field. The available helper functions may differ for each program
type, but static IDs are unique across all program types.”

Are static IDs unique across all instances of a given platform running a
program? I don’t understand what a “program type” is.

Removed this sentence since defining it is part of a separate WG work item.
Instead put in a sentence that refers to future work.

** Section 4.3.1. As a document reader, I’m not sure what I do with this
section as it defines a concept of helper functions but doesn’t specify it.

Defining it is part of a separate WG work item.
Instead put in a sentence that refers to future work.

Below are the relevant changes from https://mailarchive.ietf.org/arch/msg/bpf/qhwuPOLbC-C44NcGzELHLC5O2-s/

 Historically, each helper function was identified by a static ID
-encoded in the 'imm' field.  The available helper functions may differ
-for each program type, but static IDs are unique across all program types.
+encoded in the 'imm' field.  Further documentation of helper functions
+is outside the scope of this document and standardization is left for
+future work, but use is widely deployed and more information can be
+found in platform-specific documentation (e.g., Linux kernel documentation).
 
 Platforms that support the BPF Type Format (BTF) support identifying
 a helper function by a BTF ID encoded in the 'imm' field, where the BTF ID
 identifies the helper name and type.  Further documentation of BTF
-is outside the scope of this document and is left for future work.
+is outside the scope of this document and standardization is left for
+future work, but use is widely deployed and more information can be
+found in platform-specific documentation (e.g., Linux kernel documentation).

@dthaler dthaler changed the title ISA: Roman Ranyliw's comments ISA: Roman Danyliw's comments Jun 24, 2024
dthaler added a commit that referenced this issue Jun 24, 2024
@dthaler dthaler added the have proposed text Proposed patch posted label Jun 24, 2024
dthaler added a commit that referenced this issue Jun 25, 2024
@dthaler dthaler added fixed in bpf-next Awaiting propagation to ebpf-docs fixed in editors copy Fixed in github repo and removed have proposed text Proposed patch posted fixed in bpf-next Awaiting propagation to ebpf-docs labels Jun 25, 2024
@dthaler
Copy link
Collaborator Author

dthaler commented Jun 25, 2024

Fixed in draft -04

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fixed in editors copy Fixed in github repo ISA Affects ISA draft
Projects
None yet
Development

No branches or pull requests

1 participant