search

sonic-net / DASH

Disaggregated APIs for SONiC Hosts
Apache License 2.0
80 stars 89 forks source link

IPv4 options #468

Closed jhsmt closed 8 months ago

jhsmt commented 9 months ago

This is more a clarification question: The IPv4 headers (and parser) include IPv4 options. Is the motivation to look at IP options driven by desire for correctness? If yes, what about TCP options (which seem to be missing)? It is a little unusual to have IP options in data centre traffic but otoh very common to have TCP options.

KrisNey-MSFT commented 9 months ago

Will take a look and get back re: this. @sharmasushant Do we want a DASH device to be able to look past IPv4 options (to ask another way).

jafingerhut commented 9 months ago

For example, even if a DASH device does not need to pay careful attention to the contents of IPv4 options, if present, is it important that a DASH device be able to skip over IPv4 options in order to find the L4 header, and if it is TCP or UDP, extract the source & destination port from the packet in order to perform DASH ACL classification, and/or matching the packet against a cached flow record in the fast path.

jhsmt commented 9 months ago

On Wed, Dec 6, 2023 at 12:07 PM Andy Fingerhut @.***> wrote:

For example, even if a DASH device does not need to pay careful attention to the contents of IPv4 options, if present, is it important that a DASH device be able to skip over IPv4 options in order to find the L4 header, and if it is TCP or UDP, extract the source & destination port from the packet in order to perform DASH ACL classification, and/or matching the packet against a cached flow record in the fast path.

Assuming that DASH does not care about anything beyond the transport header or any transport header options then this is a reasonable explanation.

— Reply to this email directly, view it on GitHub https://github.com/sonic-net/DASH/issues/468#issuecomment-1843309020, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAHFPMR4OATNDW2SZE6FN6TYICQ6RAVCNFSM6AAAAABADQVX5WVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQNBTGMYDSMBSGA . You are receiving this because you authored the thread.Message ID: @.***>

r12f commented 9 months ago

@jhsmt , yes. in the current supported scenarios, such as VNET, PrivateLink and etc, the TCP options are not used, hence not parsed.

jhsmt commented 9 months ago

On Thu, Dec 7, 2023 at 6:10 PM Riff @.***> wrote:

@jhsmt https://github.com/jhsmt , yes. in the current supported scenarios, such as VNET, PrivateLink and etc, the TCP options are not used, hence not parsed.

Ok, I think we can close this issue then.

Reply to this email directly, view it on GitHub https://github.com/sonic-net/DASH/issues/468#issuecomment-1846245648, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAHFPMUSM3HUKCML4K3SU7LYIJEETAVCNFSM6AAAAABADQVX5WVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQNBWGI2DKNRUHA . You are receiving this because you were mentioned.Message ID: @.***>

chrispsommers commented 9 months ago

Closed per author's advice.