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

PSA: add packet length standard metadata #925

Closed

Conversation

jafingerhut
Copy link
Contributor

No description provided.

@jafingerhut
Copy link
Contributor Author

I do not consider this done, as there is a "TBD" in this PR as I write this comment for fleshing out more details of the value that the new packet_length field will contain for cloned packets. I think the part that is written so far is useful as a starting point for discussion with those interested in the PSA specification and this field.

@jafingerhut
Copy link
Contributor Author

I am recommending that we consider PR #927 over this one, but I will leave them both open for now.

Comment on lines +1049 to +1052
If the input port on which the packet arrived was an Ethernet port,
this includes the 4 bytes of Ethernet Frame Check Sequence, and
targets may restrict this value to be the minimum Ethernet frame
length of 64 bytes.
Copy link
Contributor

@blp blp Apr 5, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't really like this, because virtual Ethernet packets and virtual packet processing exist and there's no FCS in that case.

I'd prefer something like:

If the input port on which the packet arrived was an Ethernet port,
this includes the 4 bytes of Ethernet Frame Check Sequence if and
only if the FCS is included in the bytes available to the parser.
Targets may restrict this value to be the minimum Ethernet frame
length of 64 bytes (68 bytes including FCS).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO, this kind of detail is another reason to prefer the PR #927 over this one -- getting multiple implementations to agree on this level of detail is not easy.

FYI, I am pretty sure that the minimum Ethernet frame size is 60 bytes of (Ethernet header + Ethernet payload) + 4 bytes of FCS, totaling 64 bytes (not counting those fun things like preamble and inter-frame gaps that appear on physical Ethernet cables, at least, and whatever WiFi does, which I've never learned in detail).

@blp
Copy link
Contributor

blp commented Apr 5, 2021

Conversation in the LDWG meeting leaned toward this not being portable and therefore against including it in the spec.

@jafingerhut
Copy link
Contributor Author

Merged in PR #927 instead of this one. Closing this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants