Fixed NAT service and modified utils function#238
Fixed NAT service and modified utils function#238frisso merged 4 commits intopolycube-network:masterfrom
Conversation
DavideAG
force-pushed
the
packetcapture_service
branch
from
78bf3e2 to
106430b
Compare
… if the problem persists for future transparent services
DavideAG
force-pushed
the
packetcapture_service
branch
from
106430b to
77003ff
Compare
| if (data + sizeof(*eth) + sizeof(*ip) + sizeof(*tcp) > data_end) | ||
| uint8_t header_len = 4 * ip->ihl; | ||
| struct tcphdr *tcp = data + sizeof(*eth) + header_len; | ||
| if (data + sizeof(*eth) + header_len + sizeof(*tcp) > data_end) |
There was a problem hiding this comment.
Would it be possible to write this as:
if ( (tcp+ sizeof(*tcp)) > data_end)
The same for other similar lines below.
There was a problem hiding this comment.
Definitely feasible, although the way it is written now is in line with the kernel code (e.g., https://github.com/torvalds/linux/blob/2027cabe6afea5d471401ec704b65ce18f958fdc/samples/bpf/parse_simple.c#L36)
However, if you decide to change it I would suggest to write it in this way:
if ( (void *)tcp + sizeof(*tcp) > data_end )
this would avoid annoying warnings on different pointer types comparisons.
There was a problem hiding this comment.
Fine, thank you for your help @sebymiano. I will do it
There was a problem hiding this comment.
@sebymiano thanks for the comment.
I was just wondering how smart is the compiler. Since "*tcp = data + sizeof(*eth) + header_len;" was computed the line before, I was simply proposing the reuse that result instead of computing it again in the next line.
So, if the compiler is smart, this code rewriting is useless. If it is not so smart, this saves a couple of ASM instructions (and, to me, looks more clear).
DavideAG
force-pushed
the
packetcapture_service
branch
2 times, most recently
from
29e795e to
ed1b49e
Compare
Update Nat_dp.c
DavideAG
force-pushed
the
packetcapture_service
branch
from
a9ab073 to
9c7e6ad
Compare
4 participants
This pull request will fix NAT service.
We need to investigate if the problem persists for future transparent services.