| Message ID | 20240726042846.591226-1-mkp@redhat.com |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | [ovs-dev] dp-packet: Correct IPv4 checksum calculation. | expand |
| Context | Check | Description |
|---|---|---|
| ovsrobot/apply-robot | success | apply and check: success |
| ovsrobot/github-robot-_Build_and_Test | success | github build: passed |
| ovsrobot/intel-ovs-compilation | fail | test: fail |
On 7/26/24 06:28, Mike Pattrick wrote: > During the transition towards checksum offloading, the function to > handle software fallback of IPv4 checksums didn't account for the > options field. > > Fixes: 5d11c47d3ebe ("userspace: Enable IP checksum offloading by default.") > Signed-off-by: Mike Pattrick <mkp@redhat.com> Hi, Mike. Thanks for the patch! Reported-at/by tags would be good to have here. Also, please, add a test case for this issue. > --- > lib/dp-packet.h | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/lib/dp-packet.h b/lib/dp-packet.h > index a75b1c5cd..3e625571e 100644 > --- a/lib/dp-packet.h > +++ b/lib/dp-packet.h > @@ -1394,7 +1394,7 @@ dp_packet_ip_set_header_csum(struct dp_packet *p, bool inner) > > ovs_assert(ip); > ip->ip_csum = 0; > - ip->ip_csum = csum(ip, sizeof *ip); > + ip->ip_csum = csum(ip, IP_IHL(ip->ip_ihl_ver) * 4); We need to check that the header length doesn't exceed the length of the packet. Packets that fail ipv4 header sanity checks still have their l3_ofs set, so we may end up summing using the data from a malformed header. We may still calculate a wrong checksum if the header is wrong, but within the range, but we can't really do anything about that. Another test for a short packet would be also good to have. > } > > /* Returns 'true' if the packet 'p' has good integrity and the Best regards, Ilya Maximets.
diff --git a/lib/dp-packet.h b/lib/dp-packet.h index a75b1c5cd..3e625571e 100644 --- a/lib/dp-packet.h +++ b/lib/dp-packet.h @@ -1394,7 +1394,7 @@ dp_packet_ip_set_header_csum(struct dp_packet *p, bool inner) ovs_assert(ip); ip->ip_csum = 0; - ip->ip_csum = csum(ip, sizeof *ip); + ip->ip_csum = csum(ip, IP_IHL(ip->ip_ihl_ver) * 4); } /* Returns 'true' if the packet 'p' has good integrity and the
During the transition towards checksum offloading, the function to handle software fallback of IPv4 checksums didn't account for the options field. Fixes: 5d11c47d3ebe ("userspace: Enable IP checksum offloading by default.") Signed-off-by: Mike Pattrick <mkp@redhat.com> --- lib/dp-packet.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)