diff mbox series

[ovs-dev] conntrack: Exclude l2 pad bytes from L4 checksum validation

Message ID 1548694155-28734-1-git-send-email-vishal.deep.ajmera@ericsson.com
State Superseded
Headers show
Series [ovs-dev] conntrack: Exclude l2 pad bytes from L4 checksum validation | expand

Commit Message

Vishal Deep Ajmera Jan. 28, 2019, 8:48 a.m. UTC 
Userspace conntrack implementation in OVS is including L2 (Ethernet) pad bytes
as well for L4 checksum validation. This is a bug and any packet with non-zero
L2 padding bytes will be treated as invalid packet by conntrack due to checksum
failures.

This patch fixes the conntrack implementation by ignoring L2 pad bytes for L4
checksum validation.

Signed-off-by: Vishal Deep Ajmera <vishal.deep.ajmera@ericsson.com>
Co-authored-by: Venkatesan Pradeep <venkatesan.pradeep@ericsson.com>
Co-authored-by: Nitin Katiyar <nitin.katiyar@ericsson.com>
Signed-off-by: Venkatesan Pradeep <venkatesan.pradeep@ericsson.com>
Signed-off-by: Nitin Katiyar <nitin.katiyar@ericsson.com>

---
 lib/conntrack.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

0-day Robot Jan. 28, 2019, 8:59 a.m. UTC | #1 
Bleep bloop.  Greetings Vishal Deep Ajmera, I am a robot and I have tried out your patch.
Thanks for your contribution.

I encountered some error that I wasn't expecting.  See the details below.


checkpatch:
WARNING: Line is 83 characters long (recommended limit is 79)
#33 FILE: lib/conntrack.c:1976:
            if (extract_l4(&ctx->key, l4, (tail - l4) - dp_packet_l2_pad_size(pkt),

Lines checked: 41, Warnings: 1, Errors: 0


Please check this out.  If you feel there has been an error, please email aconole@bytheb.org

Thanks,
0-day Robot
Darrell Ball Jan. 28, 2019, 6:19 p.m. UTC | #2 
Thanks for the patch

I sent an alternative as part of a 2 patch series

https://patchwork.ozlabs.org/project/openvswitch/list/?series=88620

Darrell

On Mon, Jan 28, 2019 at 12:50 AM Vishal Deep Ajmera <
vishal.deep.ajmera@ericsson.com> wrote:

> Userspace conntrack implementation in OVS is including L2 (Ethernet) pad
> bytes
> as well for L4 checksum validation. This is a bug and any packet with
> non-zero
> L2 padding bytes will be treated as invalid packet by conntrack due to
> checksum
> failures.
>
> This patch fixes the conntrack implementation by ignoring L2 pad bytes for
> L4
> checksum validation.
>
> Signed-off-by: Vishal Deep Ajmera <vishal.deep.ajmera@ericsson.com>
> Co-authored-by: Venkatesan Pradeep <venkatesan.pradeep@ericsson.com>
> Co-authored-by: Nitin Katiyar <nitin.katiyar@ericsson.com>
> Signed-off-by: Venkatesan Pradeep <venkatesan.pradeep@ericsson.com>
> Signed-off-by: Nitin Katiyar <nitin.katiyar@ericsson.com>
>
> ---
>  lib/conntrack.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/lib/conntrack.c b/lib/conntrack.c
> index f732b9e..1f6dccf 100644
> --- a/lib/conntrack.c
> +++ b/lib/conntrack.c
> @@ -1973,8 +1973,8 @@ conn_key_extract(struct conntrack *ct, struct
> dp_packet *pkt, ovs_be16 dl_type,
>          if (!hwol_bad_l4_csum) {
>              bool  hwol_good_l4_csum = dp_packet_l4_checksum_valid(pkt);
>              /* Validate the checksum only when hwol is not supported. */
> -            if (extract_l4(&ctx->key, l4, tail - l4, &ctx->icmp_related,
> l3,
> -                           !hwol_good_l4_csum)) {
> +            if (extract_l4(&ctx->key, l4, (tail - l4) -
> dp_packet_l2_pad_size(pkt),
> +                           &ctx->icmp_related, l3, !hwol_good_l4_csum)) {
>                  ctx->hash = conn_key_hash(&ctx->key, ct->hash_basis);
>                  return true;
>              }
> --
> 1.9.1
>
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
diff mbox series

Patch

diff --git a/lib/conntrack.c b/lib/conntrack.c
index f732b9e..1f6dccf 100644
--- a/lib/conntrack.c
+++ b/lib/conntrack.c
@@ -1973,8 +1973,8 @@  conn_key_extract(struct conntrack *ct, struct dp_packet *pkt, ovs_be16 dl_type,
         if (!hwol_bad_l4_csum) {
             bool  hwol_good_l4_csum = dp_packet_l4_checksum_valid(pkt);
             /* Validate the checksum only when hwol is not supported. */
-            if (extract_l4(&ctx->key, l4, tail - l4, &ctx->icmp_related, l3,
-                           !hwol_good_l4_csum)) {
+            if (extract_l4(&ctx->key, l4, (tail - l4) - dp_packet_l2_pad_size(pkt),
+                           &ctx->icmp_related, l3, !hwol_good_l4_csum)) {
                 ctx->hash = conn_key_hash(&ctx->key, ct->hash_basis);
                 return true;
             }