search

google / gvisor

Application Kernel for Containers
https://gvisor.dev
Apache License 2.0
15.8k stars 1.3k forks source link

packet buffer headers should have protocol number associated. #3810

Open jelischer opened 4 years ago

jelischer commented 4 years ago

Packet headers are now tracked in the packet buffer structure using a special 'header' structure. This structure should maintain what type each header is as well as what layer it is (which is all it has now). In parts of the code where it is important to look at the transport header for example it is expensive to go back to re-examine the IPv6 header to find which kind of transport header is in the transport header storage (tcp? udp? icmp?). Since we already know this information at some stage it is probably worth while to save it with the header.

jelischer commented 4 years ago

It turns out that to do this we probably should fix the fact that ICMP never does a Consume() operation on the packet for the transport header, but leaves it in the Data segment. The reason for this is that ICMP processing happens even if the ICMP protocol is not available for use by upper layers and the setting of the header into its own section would normally be done by the upper ICMP code. The answer would be to have it done at the lower ICMP code in network/ipv[46]/icmp.c which is always present.

tamird commented 4 years ago

cc @kevinGC

kevinGC commented 4 years ago

Yes, there is a confusing coupling between ICMP sockets (enabled via stack.Options.TransportProtocols) and ICMP handling. They should share code, but the latter shouldn't depend on enabling the former.

For context: this came up when we added early header parsing, but wasn't needed immediately and so nobody allocated cycles for it.

jelischer commented 4 years ago

I could add some cycles to it while I have it fully loaded in my head.

github-actions[bot] commented 3 years ago

This issue is stale because it has been open 90 days with no activity. Remove the stale label or comment or this will be closed in 30 days.

ghananigans commented 2 years ago

This is done - packet buffers hold the network/transport protocol numbers

tamird commented 2 years ago

There are still 5 TODOs referencing this issue.

github-actions[bot] commented 1 year ago

A friendly reminder that this issue had no activity for 120 days.

github-actions[bot] commented 10 months ago

This issue has been closed due to lack of activity.

github-actions[bot] commented 10 months ago

There are TODOs still referencing this issue:

  1. pkg/tcpip/network/ipv4/ipv4.go:1296: when we sort out ICMP and transport
  2. pkg/tcpip/transport/tcp/testing/context/context.go:351: Remove when protocol numbers are part
  3. pkg/tcpip/transport/tcp/testing/context/context.go:401: Remove when protocol numbers are part

Search TODO

github-actions[bot] commented 10 months ago

There are TODOs still referencing this issue:

  1. pkg/tcpip/stack/packet_buffer.go:128: This and the network protocol number should

Search TODO

github-actions[bot] commented 6 months ago

A friendly reminder that this issue had no activity for 120 days.