Re: [ovs-dev] [PATCH] flow: Consistent VXLAN UDP src ports for fragmented packets

2023-10-30 Thread Hemanth Aramadaka via dev
Hi Ilya,

The changes are not required for flow_hash_5tuple() function. I have added 
debug statements, and we are not hitting this function as part of this code 
flow. 

Thanks,
Hemanth.

-Original Message-
From: Ilya Maximets  
Sent: Monday, June 26, 2023 7:59 PM
To: Hemanth Aramadaka ; 'Simon Horman' 
; ovs-dev@openvswitch.org
Cc: i.maxim...@ovn.org
Subject: Re: [ovs-dev] [PATCH] flow: Consistent VXLAN UDP src ports for 
fragmented packets

On 6/26/23 08:59, Hemanth Aramadaka via dev wrote:
> Hi Simon,
> 
> Sorry for the late response. Yes, the changes are specific to IPV6 
> protocol only.

Please, fix the flow_hash_5tuple() function as well.  All variants of the same 
hash function should give the same result.  For some reason you just removed 
the fix for this function from the patch instead of addressing a review comment.

And add the unit test for the issue.

Thanks.
Best regards, Ilya Maximets.

> 
> Thanks,
> Hemanth.
> 
> -Original Message-
> From: Simon Horman 
> Sent: Friday, January 20, 2023 8:52 PM
> To: ovs-dev@openvswitch.org
> Cc: Hemanth Aramadaka 
> Subject: Re: [ovs-dev] [PATCH] flow: Consistent VXLAN UDP src ports 
> for fragmented packets
> 
> On Thu, Jan 05, 2023 at 06:55:03PM +0530, Hemanth Aramadaka via dev wrote:
>> Issue:
>>
>> The src-port for UDP is based on RSS hash in the packet metadata.
>> In case of packets coming from VM it will be 5-tuple, if available, 
>> otherwise just IP addresses.If the VM fragments a large IP packet and 
>> sends the fragments to ovs, only the first fragment will contain the
>> L4 header. Therefore, the first fragment and subsequent fragments get 
>> different UDP src ports in the outgoing VXLAN header.This can lead to 
>> fragment re-ordering in the fabric as packet will take different 
>> paths.
>>
>> Fix:
>>
>> Intention of this is to avoid fragment packets taking different paths.
>> For example, due to presence of firewalls, fragment packets will take 
>> different paths and will get dropped.To avoid this we ignore the L4 
>> header during hash calculation only in the case of fragmented packets.
>>
>> Signed-off-by: Hemanth Aramadaka 
>> ---
>>  lib/flow.c | 18 +++---
>>  1 file changed, 15 insertions(+), 3 deletions(-)
>>
>> diff --git a/lib/flow.c b/lib/flow.c
>> index c3a3aa3ce..4f4197b1c 100644
>> --- a/lib/flow.c
>> +++ b/lib/flow.c
>> @@ -1018,7 +1018,9 @@ miniflow_extract(struct dp_packet *packet, 
>> struct
> miniflow *dst)
>>  miniflow_push_be16(mf, ct_tp_src, ct_tp_src);
>>  miniflow_push_be16(mf, ct_tp_dst, ct_tp_dst);
>>  if (dl_type == htons(ETH_TYPE_IP)) {
>> -dp_packet_update_rss_hash_ipv4_tcp_udp(packet);
>> +if (!(nw_frag & FLOW_NW_FRAG_MASK)) {
>> +
> dp_packet_update_rss_hash_ipv4_tcp_udp(packet);
>> +}
>>  } else if (dl_type == htons(ETH_TYPE_IPV6)) {
>>  dp_packet_update_rss_hash_ipv6_tcp_udp(packet);
>>  }
>> @@ -1033,7 +1035,9 @@ miniflow_extract(struct dp_packet *packet, 
>> struct
> miniflow *dst)
>>  miniflow_push_be16(mf, ct_tp_src, ct_tp_src);
>>  miniflow_push_be16(mf, ct_tp_dst, ct_tp_dst);
>>  if (dl_type == htons(ETH_TYPE_IP)) {
>> -dp_packet_update_rss_hash_ipv4_tcp_udp(packet);
>> +if (!(nw_frag & FLOW_NW_FRAG_MASK)) {
>> +dp_packet_update_rss_hash_ipv4_tcp_udp(packet);
>> +}
>>  } else if (dl_type == htons(ETH_TYPE_IPV6)) {
>>  dp_packet_update_rss_hash_ipv6_tcp_udp(packet);
>>  }
>> @@ -2248,7 +2252,7 @@ miniflow_hash_5tuple(const struct miniflow 
>> *flow, uint32_t basis)
>>  
>>  if (flow) {
>>  ovs_be16 dl_type = MINIFLOW_GET_BE16(flow, dl_type);
>> -uint8_t nw_proto;
>> +uint8_t nw_proto, nw_frag;
>>  
>>  if (dl_type == htons(ETH_TYPE_IPV6)) {
>>  struct flowmap map = FLOWMAP_EMPTY_INITIALIZER; @@
>> -2270,6 +2274,14 @@ miniflow_hash_5tuple(const struct miniflow *flow, 
>> uint32_t basis)
>>  
>>  nw_proto = MINIFLOW_GET_U8(flow, nw_proto);
>>  hash = hash_add(hash, nw_proto);
>> +
>> +/* Skip l4 header fields if IP packet is fragmented since
>> + * only first fragment will carry l4 header.
>> + */
>> +nw_frag = MINI

Re: [ovs-dev] [PATCH] flow: Consistent VXLAN UDP src ports for fragmented packets

2023-06-26 Thread Hemanth Aramadaka via dev
Hi Simon,

Sorry for the late response. Yes, the changes are specific to IPV6 protocol
only.

Thanks,
Hemanth.

-Original Message-
From: Simon Horman  
Sent: Friday, January 20, 2023 8:52 PM
To: ovs-dev@openvswitch.org
Cc: Hemanth Aramadaka 
Subject: Re: [ovs-dev] [PATCH] flow: Consistent VXLAN UDP src ports for
fragmented packets

On Thu, Jan 05, 2023 at 06:55:03PM +0530, Hemanth Aramadaka via dev wrote:
> Issue:
> 
> The src-port for UDP is based on RSS hash in the packet metadata.
> In case of packets coming from VM it will be 5-tuple, if available, 
> otherwise just IP addresses.If the VM fragments a large IP packet and 
> sends the fragments to ovs, only the first fragment will contain the 
> L4 header. Therefore, the first fragment and subsequent fragments get 
> different UDP src ports in the outgoing VXLAN header.This can lead to 
> fragment re-ordering in the fabric as packet will take different 
> paths.
> 
> Fix:
> 
> Intention of this is to avoid fragment packets taking different paths.
> For example, due to presence of firewalls, fragment packets will take 
> different paths and will get dropped.To avoid this we ignore the L4 
> header during hash calculation only in the case of fragmented packets.
> 
> Signed-off-by: Hemanth Aramadaka 
> ---
>  lib/flow.c | 18 +++---
>  1 file changed, 15 insertions(+), 3 deletions(-)
> 
> diff --git a/lib/flow.c b/lib/flow.c
> index c3a3aa3ce..4f4197b1c 100644
> --- a/lib/flow.c
> +++ b/lib/flow.c
> @@ -1018,7 +1018,9 @@ miniflow_extract(struct dp_packet *packet, struct
miniflow *dst)
>  miniflow_push_be16(mf, ct_tp_src, ct_tp_src);
>  miniflow_push_be16(mf, ct_tp_dst, ct_tp_dst);
>  if (dl_type == htons(ETH_TYPE_IP)) {
> -dp_packet_update_rss_hash_ipv4_tcp_udp(packet);
> +if (!(nw_frag & FLOW_NW_FRAG_MASK)) {
> +
dp_packet_update_rss_hash_ipv4_tcp_udp(packet);
> +}
>  } else if (dl_type == htons(ETH_TYPE_IPV6)) {
>  dp_packet_update_rss_hash_ipv6_tcp_udp(packet);
>  }
> @@ -1033,7 +1035,9 @@ miniflow_extract(struct dp_packet *packet, struct
miniflow *dst)
>  miniflow_push_be16(mf, ct_tp_src, ct_tp_src);
>  miniflow_push_be16(mf, ct_tp_dst, ct_tp_dst);
>  if (dl_type == htons(ETH_TYPE_IP)) {
> -dp_packet_update_rss_hash_ipv4_tcp_udp(packet);
> +if (!(nw_frag & FLOW_NW_FRAG_MASK)) {
> +dp_packet_update_rss_hash_ipv4_tcp_udp(packet);
> +}
>  } else if (dl_type == htons(ETH_TYPE_IPV6)) {
>  dp_packet_update_rss_hash_ipv6_tcp_udp(packet);
>  }
> @@ -2248,7 +2252,7 @@ miniflow_hash_5tuple(const struct miniflow 
> *flow, uint32_t basis)
>  
>  if (flow) {
>  ovs_be16 dl_type = MINIFLOW_GET_BE16(flow, dl_type);
> -uint8_t nw_proto;
> +uint8_t nw_proto, nw_frag;
>  
>  if (dl_type == htons(ETH_TYPE_IPV6)) {
>  struct flowmap map = FLOWMAP_EMPTY_INITIALIZER; @@ 
> -2270,6 +2274,14 @@ miniflow_hash_5tuple(const struct miniflow *flow, 
> uint32_t basis)
>  
>  nw_proto = MINIFLOW_GET_U8(flow, nw_proto);
>  hash = hash_add(hash, nw_proto);
> +
> +/* Skip l4 header fields if IP packet is fragmented since
> + * only first fragment will carry l4 header.
> + */
> +nw_frag = MINIFLOW_GET_U8(flow, nw_frag);
> +if (nw_frag & FLOW_NW_FRAG_MASK) {
> +goto out;
> +}

Maybe I am reading things wrong, but it seems to me that this change seems
to effect all protocols, and in particular both
IPv4 and IPv6. Whereas the changes above to miniflow_extract() only affect
IPv6. Is this intentional?

>  if (nw_proto != IPPROTO_TCP && nw_proto != IPPROTO_UDP
>  && nw_proto != IPPROTO_SCTP && nw_proto != IPPROTO_ICMP
>  && nw_proto != IPPROTO_ICMPV6) {
> --
> 2.34.1
> 
> ___
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
> 

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


[ovs-dev] [PATCH] flow: Consistent VXLAN UDP src ports for fragmented packets

2023-01-06 Thread Hemanth Aramadaka via dev
Issue:

The src-port for UDP is based on RSS hash in the packet metadata.
In case of packets coming from VM it will be 5-tuple, if available,
otherwise just IP addresses.If the VM fragments a large IP packet
and sends the fragments to ovs, only the first fragment will contain
the L4 header. Therefore, the first fragment and subsequent fragments
get different UDP src ports in the outgoing VXLAN header.This can
lead to fragment re-ordering in the fabric as packet will take
different paths.

Fix:

Intention of this is to avoid fragment packets taking different paths.
For example, due to presence of firewalls, fragment packets will take
different paths and will get dropped.To avoid this we ignore the L4
header during hash calculation only in the case of fragmented packets.

Signed-off-by: Hemanth Aramadaka 
---
 lib/flow.c | 18 +++---
 1 file changed, 15 insertions(+), 3 deletions(-)

diff --git a/lib/flow.c b/lib/flow.c
index c3a3aa3ce..4f4197b1c 100644
--- a/lib/flow.c
+++ b/lib/flow.c
@@ -1018,7 +1018,9 @@ miniflow_extract(struct dp_packet *packet, struct 
miniflow *dst)
 miniflow_push_be16(mf, ct_tp_src, ct_tp_src);
 miniflow_push_be16(mf, ct_tp_dst, ct_tp_dst);
 if (dl_type == htons(ETH_TYPE_IP)) {
-dp_packet_update_rss_hash_ipv4_tcp_udp(packet);
+if (!(nw_frag & FLOW_NW_FRAG_MASK)) {
+dp_packet_update_rss_hash_ipv4_tcp_udp(packet);
+}
 } else if (dl_type == htons(ETH_TYPE_IPV6)) {
 dp_packet_update_rss_hash_ipv6_tcp_udp(packet);
 }
@@ -1033,7 +1035,9 @@ miniflow_extract(struct dp_packet *packet, struct 
miniflow *dst)
 miniflow_push_be16(mf, ct_tp_src, ct_tp_src);
 miniflow_push_be16(mf, ct_tp_dst, ct_tp_dst);
 if (dl_type == htons(ETH_TYPE_IP)) {
-dp_packet_update_rss_hash_ipv4_tcp_udp(packet);
+if (!(nw_frag & FLOW_NW_FRAG_MASK)) {
+dp_packet_update_rss_hash_ipv4_tcp_udp(packet);
+}
 } else if (dl_type == htons(ETH_TYPE_IPV6)) {
 dp_packet_update_rss_hash_ipv6_tcp_udp(packet);
 }
@@ -2248,7 +2252,7 @@ miniflow_hash_5tuple(const struct miniflow *flow, 
uint32_t basis)
 
 if (flow) {
 ovs_be16 dl_type = MINIFLOW_GET_BE16(flow, dl_type);
-uint8_t nw_proto;
+uint8_t nw_proto, nw_frag;
 
 if (dl_type == htons(ETH_TYPE_IPV6)) {
 struct flowmap map = FLOWMAP_EMPTY_INITIALIZER;
@@ -2270,6 +2274,14 @@ miniflow_hash_5tuple(const struct miniflow *flow, 
uint32_t basis)
 
 nw_proto = MINIFLOW_GET_U8(flow, nw_proto);
 hash = hash_add(hash, nw_proto);
+
+/* Skip l4 header fields if IP packet is fragmented since
+ * only first fragment will carry l4 header.
+ */
+nw_frag = MINIFLOW_GET_U8(flow, nw_frag);
+if (nw_frag & FLOW_NW_FRAG_MASK) {
+goto out;
+}
 if (nw_proto != IPPROTO_TCP && nw_proto != IPPROTO_UDP
 && nw_proto != IPPROTO_SCTP && nw_proto != IPPROTO_ICMP
 && nw_proto != IPPROTO_ICMPV6) {
-- 
2.34.1

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


[ovs-dev] [PATCH] flow: Consistent VXLAN UDP src ports for fragmented packets

2022-12-02 Thread Hemanth Aramadaka via dev
Issue:

The src-port for UDP is based on RSS hash in the packet metadata.
In case of packets coming from VM it will be 5-tuple, if available,
otherwise just IP addresses.If the VM fragments a large IP packet
and sends the fragments to ovs, only the first fragment will contain
the L4 header. Therefore, the first fragment and subsequent fragments
get different UDP src ports in the outgoing VXLAN header.This can
lead to fragment re-ordering in the fabric as packet will take
different paths.

Fix:

Intention of this is to avoid fragment packets taking different paths.
For example, due to presence of firewalls, fragment packets will take
different paths and will get dropped.To avoid this we ignore the L4
header during hash calculation only in the case of fragmented packets.

Signed-off-by: Hemanth Aramadaka 
---
 lib/flow.c | 22 +++---
 1 file changed, 19 insertions(+), 3 deletions(-)

diff --git a/lib/flow.c b/lib/flow.c
index c3a3aa3ce..9a65c304e 100644
--- a/lib/flow.c
+++ b/lib/flow.c
@@ -1018,7 +1018,9 @@ miniflow_extract(struct dp_packet *packet, struct 
miniflow *dst)
 miniflow_push_be16(mf, ct_tp_src, ct_tp_src);
 miniflow_push_be16(mf, ct_tp_dst, ct_tp_dst);
 if (dl_type == htons(ETH_TYPE_IP)) {
-dp_packet_update_rss_hash_ipv4_tcp_udp(packet);
+if (!(nw_frag & FLOW_NW_FRAG_MASK)) {
+dp_packet_update_rss_hash_ipv4_tcp_udp(packet);
+}
 } else if (dl_type == htons(ETH_TYPE_IPV6)) {
 dp_packet_update_rss_hash_ipv6_tcp_udp(packet);
 }
@@ -1033,7 +1035,9 @@ miniflow_extract(struct dp_packet *packet, struct 
miniflow *dst)
 miniflow_push_be16(mf, ct_tp_src, ct_tp_src);
 miniflow_push_be16(mf, ct_tp_dst, ct_tp_dst);
 if (dl_type == htons(ETH_TYPE_IP)) {
-dp_packet_update_rss_hash_ipv4_tcp_udp(packet);
+if (!(nw_frag & FLOW_NW_FRAG_MASK)) {
+dp_packet_update_rss_hash_ipv4_tcp_udp(packet);
+}
 } else if (dl_type == htons(ETH_TYPE_IPV6)) {
 dp_packet_update_rss_hash_ipv6_tcp_udp(packet);
 }
@@ -2248,7 +2252,7 @@ miniflow_hash_5tuple(const struct miniflow *flow, 
uint32_t basis)
 
 if (flow) {
 ovs_be16 dl_type = MINIFLOW_GET_BE16(flow, dl_type);
-uint8_t nw_proto;
+uint8_t nw_proto, nw_frag;
 
 if (dl_type == htons(ETH_TYPE_IPV6)) {
 struct flowmap map = FLOWMAP_EMPTY_INITIALIZER;
@@ -2270,6 +2274,11 @@ miniflow_hash_5tuple(const struct miniflow *flow, 
uint32_t basis)
 
 nw_proto = MINIFLOW_GET_U8(flow, nw_proto);
 hash = hash_add(hash, nw_proto);
+
+nw_frag = MINIFLOW_GET_U8(flow, nw_frag);
+if (nw_frag & FLOW_NW_FRAG_MASK) {
+goto out;
+}
 if (nw_proto != IPPROTO_TCP && nw_proto != IPPROTO_UDP
 && nw_proto != IPPROTO_SCTP && nw_proto != IPPROTO_ICMP
 && nw_proto != IPPROTO_ICMPV6) {
@@ -2292,6 +2301,9 @@ flow_hash_5tuple(const struct flow *flow, uint32_t basis)
 {
 BUILD_ASSERT_DECL(FLOW_WC_SEQ == 42);
 uint32_t hash = basis;
+uint8_t nw_frag;
+struct miniflow *miniflow;
+miniflow = miniflow_create(flow);
 
 if (flow) {
 
@@ -2312,6 +2324,10 @@ flow_hash_5tuple(const struct flow *flow, uint32_t basis)
 }
 
 hash = hash_add(hash, flow->nw_proto);
+nw_frag = MINIFLOW_GET_U8(miniflow, nw_frag);
+if (nw_frag & FLOW_NW_FRAG_MASK) {
+goto out;
+}
 if (flow->nw_proto != IPPROTO_TCP && flow->nw_proto != IPPROTO_UDP
 && flow->nw_proto != IPPROTO_SCTP && flow->nw_proto != IPPROTO_ICMP
 && flow->nw_proto != IPPROTO_ICMPV6) {
-- 
2.34.1

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


[ovs-dev] [PATCH] flow: Consistent VXLAN UDP src ports for fragmented packets

2022-11-04 Thread Hemanth Aramadaka via dev
Issue:

The src-port for UDP is based on RSS hash in the packet metadata.
In case of packets coming from VM it will be 5-tuple, if available,
otherwise just IP addresses.If the VM fragments a large IP packet
and sends the fragments to ovs, only the first fragment will contain
the L4 header. Therefore, the first fragment and subsequent fragments
get different UDP src ports in the outgoing VXLAN header.This can
lead to fragment re-ordering in the fabric as packet will take
different paths.

Fix:

Intention of this is to avoid fragment packets taking different paths.
For example, due to presence of firewalls, fragment packets will take
different paths and will get dropped.To avoid this we ignore the L4
header during hash calculation only in the case of fragmented packets.

Signed-off-by: Hemanth Aramadaka 
---
 lib/flow.c | 20 +---
 1 file changed, 17 insertions(+), 3 deletions(-)

diff --git a/lib/flow.c b/lib/flow.c
index c3a3aa3ce..20cca5937 100644
--- a/lib/flow.c
+++ b/lib/flow.c
@@ -1018,7 +1018,9 @@ miniflow_extract(struct dp_packet *packet, struct 
miniflow *dst)
 miniflow_push_be16(mf, ct_tp_src, ct_tp_src);
 miniflow_push_be16(mf, ct_tp_dst, ct_tp_dst);
 if (dl_type == htons(ETH_TYPE_IP)) {
-dp_packet_update_rss_hash_ipv4_tcp_udp(packet);
+if (!(nw_frag & FLOW_NW_FRAG_MASK)) {
+dp_packet_update_rss_hash_ipv4_tcp_udp(packet);
+}
 } else if (dl_type == htons(ETH_TYPE_IPV6)) {
 dp_packet_update_rss_hash_ipv6_tcp_udp(packet);
 }
@@ -1033,7 +1035,9 @@ miniflow_extract(struct dp_packet *packet, struct 
miniflow *dst)
 miniflow_push_be16(mf, ct_tp_src, ct_tp_src);
 miniflow_push_be16(mf, ct_tp_dst, ct_tp_dst);
 if (dl_type == htons(ETH_TYPE_IP)) {
-dp_packet_update_rss_hash_ipv4_tcp_udp(packet);
+if (!(nw_frag & FLOW_NW_FRAG_MASK)) {
+dp_packet_update_rss_hash_ipv4_tcp_udp(packet);
+}
 } else if (dl_type == htons(ETH_TYPE_IPV6)) {
 dp_packet_update_rss_hash_ipv6_tcp_udp(packet);
 }
@@ -2248,7 +2252,7 @@ miniflow_hash_5tuple(const struct miniflow *flow, 
uint32_t basis)
 
 if (flow) {
 ovs_be16 dl_type = MINIFLOW_GET_BE16(flow, dl_type);
-uint8_t nw_proto;
+uint8_t nw_proto, nw_frag;
 
 if (dl_type == htons(ETH_TYPE_IPV6)) {
 struct flowmap map = FLOWMAP_EMPTY_INITIALIZER;
@@ -2270,6 +2274,11 @@ miniflow_hash_5tuple(const struct miniflow *flow, 
uint32_t basis)
 
 nw_proto = MINIFLOW_GET_U8(flow, nw_proto);
 hash = hash_add(hash, nw_proto);
+
+nw_frag = MINIFLOW_GET_U8(flow, nw_frag);
+if (nw_frag & FLOW_NW_FRAG_MASK) {
+goto out;
+}
 if (nw_proto != IPPROTO_TCP && nw_proto != IPPROTO_UDP
 && nw_proto != IPPROTO_SCTP && nw_proto != IPPROTO_ICMP
 && nw_proto != IPPROTO_ICMPV6) {
@@ -2292,6 +2301,7 @@ flow_hash_5tuple(const struct flow *flow, uint32_t basis)
 {
 BUILD_ASSERT_DECL(FLOW_WC_SEQ == 42);
 uint32_t hash = basis;
+uint8_t nw_frag;
 
 if (flow) {
 
@@ -2312,6 +2322,10 @@ flow_hash_5tuple(const struct flow *flow, uint32_t basis)
 }
 
 hash = hash_add(hash, flow->nw_proto);
+nw_frag = MINIFLOW_GET_U8(flow, nw_frag);
+if (nw_frag & FLOW_NW_FRAG_MASK) {
+goto out;
+}
 if (flow->nw_proto != IPPROTO_TCP && flow->nw_proto != IPPROTO_UDP
 && flow->nw_proto != IPPROTO_SCTP && flow->nw_proto != IPPROTO_ICMP
 && flow->nw_proto != IPPROTO_ICMPV6) {
-- 
2.34.1

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] [PATCH] flow: Consistent VXLAN UDP src ports for fragmented packets

2022-11-04 Thread Hemanth Aramadaka via dev
Hi ,

Thanks for the review and comments. I have addressed the comments and raised 
the review request again. 

Ran these tests (tunnel_push_pop - packet_out and tunnel_push_pop - packet_out 
debug_slow   ). I am attaching the test results as well here.  


R620-10-CSSCI-5:/home/sdn/zarahem/ovs # make check TESTSUITEFLAGS='795'
make  check-am
make[1]: Entering directory '/home/sdn/zarahem/ovs'
make  utilities/ovs-appctl-bashcomp.bash utilities/ovs-vsctl-bashcomp.bash 
tests/atlocal tests/testpki-cacert.pem tests/testpki-cert.pem 
tests/testpki-privkey.pem tests/testpki-req.pem tests/testpki-cert2.pem 
tests/testpki-privkey2.pem tests/testpki-req2.pem
make[2]: Entering directory '/home/sdn/zarahem/ovs'
make[2]: Nothing to be done for 'utilities/ovs-appctl-bashcomp.bash'.
make[2]: Nothing to be done for 'utilities/ovs-vsctl-bashcomp.bash'.
make[2]: 'tests/atlocal' is up to date.
make[2]: 'tests/testpki-cacert.pem' is up to date.
make[2]: 'tests/testpki-cert.pem' is up to date.
make[2]: 'tests/testpki-privkey.pem' is up to date.
make[2]: 'tests/testpki-req.pem' is up to date.
make[2]: 'tests/testpki-cert2.pem' is up to date.
make[2]: 'tests/testpki-privkey2.pem' is up to date.
make[2]: 'tests/testpki-req2.pem' is up to date.
make[2]: Leaving directory '/home/sdn/zarahem/ovs'
make  check-local
make[2]: Entering directory '/home/sdn/zarahem/ovs'
set /bin/sh './tests/testsuite' -C tests 
AUTOTEST_PATH=utilities:vswitchd:ovsdb:vtep:tests:ipsec::; \
"$@" 795 || \
(test -z "$(find /home/sdn/zarahem/ovs/tests/testsuite.dir -name 'asan.*')" && \
 test -z "$(find /home/sdn/zarahem/ovs/tests/testsuite.dir -name 'ubsan.*')" && 
\
 test X'' = Xyes && "$@" --recheck)
Illegal "police"
## -- ##
## openvswitch 3.0.90 test suite. ##
## -- ##
795: tunnel_push_pop - packet_outok

## - ##
## Test results. ##
## - ##

1 test was successful.
make[2]: Leaving directory '/home/sdn/zarahem/ovs'
make[1]: Leaving directory '/home/sdn/zarahem/ovs'
R620-10-CSSCI-5:/home/sdn/zarahem/ovs # make check TESTSUITEFLAGS='796'
make  check-am
make[1]: Entering directory '/home/sdn/zarahem/ovs'
make  utilities/ovs-appctl-bashcomp.bash utilities/ovs-vsctl-bashcomp.bash 
tests/atlocal tests/testpki-cacert.pem tests/testpki-cert.pem 
tests/testpki-privkey.pem tests/testpki-req.pem tests/testpki-cert2.pem 
tests/testpki-privkey2.pem tests/testpki-req2.pem
make[2]: Entering directory '/home/sdn/zarahem/ovs'
make[2]: Nothing to be done for 'utilities/ovs-appctl-bashcomp.bash'.
make[2]: Nothing to be done for 'utilities/ovs-vsctl-bashcomp.bash'.
make[2]: 'tests/atlocal' is up to date.
make[2]: 'tests/testpki-cacert.pem' is up to date.
make[2]: 'tests/testpki-cert.pem' is up to date.
make[2]: 'tests/testpki-privkey.pem' is up to date.
make[2]: 'tests/testpki-req.pem' is up to date.
make[2]: 'tests/testpki-cert2.pem' is up to date.
make[2]: 'tests/testpki-privkey2.pem' is up to date.
make[2]: 'tests/testpki-req2.pem' is up to date.
make[2]: Leaving directory '/home/sdn/zarahem/ovs'
make  check-local
make[2]: Entering directory '/home/sdn/zarahem/ovs'
set /bin/sh './tests/testsuite' -C tests 
AUTOTEST_PATH=utilities:vswitchd:ovsdb:vtep:tests:ipsec::; \
"$@" 796 || \
(test -z "$(find /home/sdn/zarahem/ovs/tests/testsuite.dir -name 'asan.*')" && \
 test -z "$(find /home/sdn/zarahem/ovs/tests/testsuite.dir -name 'ubsan.*')" && 
\
 test X'' = Xyes && "$@" --recheck)
Illegal "police"
## -- ##
## openvswitch 3.0.90 test suite. ##
## -- ##
796: tunnel_push_pop - packet_out debug_slow ok

## - ##
## Test results. ##
## - ##

1 test was successful.
make[2]: Leaving directory '/home/sdn/zarahem/ovs'
make[1]: Leaving directory '/home/sdn/zarahem/ovs'


Thanks,
Hemanth.

-Original Message-
From: Ilya Maximets  
Sent: 04 November 2022 03:56
To: Hemanth Aramadaka ; ovs-dev@openvswitch.org
Cc: i.maxim...@ovn.org
Subject: Re: [ovs-dev] [PATCH] flow: Consistent VXLAN UDP src ports for 
fragmented packets

On 11/3/22 13:11, Hemanth Aramadaka via dev wrote:
> Issue:
> 
> The src-port for UDP is based on RSS hash in the packet metadata.
> In case of packets coming from VM it will be 5-tuple, if available, 
> otherwise just IP addresses.If the VM fragments a large IP packet and 
> sends the fragments to ovs, only the first fragment will contain the 
> L4 header. Therefore, the first fragment and subsequent fragments get 
> different UDP src ports in the outgoing VXLAN header.This can lead to 
> fragment re-ordering in the fabric as packet will take different 
> paths.
> 
> Fix:
> 
> Intention of this is to avoid fragment packets taking different paths.
> For example, due to presence of firew

[ovs-dev] [PATCH] flow: Consistent VXLAN UDP src ports for fragmented packets

2022-11-04 Thread Hemanth Aramadaka via dev
Issue:

The src-port for UDP is based on RSS hash in the packet metadata.
In case of packets coming from VM it will be 5-tuple, if available,
otherwise just IP addresses.If the VM fragments a large IP packet
and sends the fragments to ovs, only the first fragment will contain
the L4 header. Therefore, the first fragment and subsequent fragments
get different UDP src ports in the outgoing VXLAN header.This can
lead to fragment re-ordering in the fabric as packet will take
different paths.

Fix:

Intention of this is to avoid fragment packets taking different paths.
For example, due to presence of firewalls, fragment packets will take
different paths and will get dropped.To avoid this we ignore the L4
header during hash calculation only in the case of fragmented packets.

Signed-off-by: Hemanth Aramadaka 
---
 lib/flow.c | 17 ++---
 1 file changed, 14 insertions(+), 3 deletions(-)

diff --git a/lib/flow.c b/lib/flow.c
index c3a3aa3ce..e8a2dc74e 100644
--- a/lib/flow.c
+++ b/lib/flow.c
@@ -1018,7 +1018,9 @@ miniflow_extract(struct dp_packet *packet, struct 
miniflow *dst)
 miniflow_push_be16(mf, ct_tp_src, ct_tp_src);
 miniflow_push_be16(mf, ct_tp_dst, ct_tp_dst);
 if (dl_type == htons(ETH_TYPE_IP)) {
-dp_packet_update_rss_hash_ipv4_tcp_udp(packet);
+if (!(nw_frag & FLOW_NW_FRAG_MASK)) {
+dp_packet_update_rss_hash_ipv4_tcp_udp(packet);
+}
 } else if (dl_type == htons(ETH_TYPE_IPV6)) {
 dp_packet_update_rss_hash_ipv6_tcp_udp(packet);
 }
@@ -1033,7 +1035,9 @@ miniflow_extract(struct dp_packet *packet, struct 
miniflow *dst)
 miniflow_push_be16(mf, ct_tp_src, ct_tp_src);
 miniflow_push_be16(mf, ct_tp_dst, ct_tp_dst);
 if (dl_type == htons(ETH_TYPE_IP)) {
-dp_packet_update_rss_hash_ipv4_tcp_udp(packet);
+if (!(nw_frag & FLOW_NW_FRAG_MASK)) {
+dp_packet_update_rss_hash_ipv4_tcp_udp(packet);
+}
 } else if (dl_type == htons(ETH_TYPE_IPV6)) {
 dp_packet_update_rss_hash_ipv6_tcp_udp(packet);
 }
@@ -2248,7 +2252,7 @@ miniflow_hash_5tuple(const struct miniflow *flow, 
uint32_t basis)
 
 if (flow) {
 ovs_be16 dl_type = MINIFLOW_GET_BE16(flow, dl_type);
-uint8_t nw_proto;
+uint8_t nw_proto, nw_frag = 0;
 
 if (dl_type == htons(ETH_TYPE_IPV6)) {
 struct flowmap map = FLOWMAP_EMPTY_INITIALIZER;
@@ -2270,6 +2274,9 @@ miniflow_hash_5tuple(const struct miniflow *flow, 
uint32_t basis)
 
 nw_proto = MINIFLOW_GET_U8(flow, nw_proto);
 hash = hash_add(hash, nw_proto);
+if (nw_frag & FLOW_NW_FRAG_MASK) {
+goto out;
+}
 if (nw_proto != IPPROTO_TCP && nw_proto != IPPROTO_UDP
 && nw_proto != IPPROTO_SCTP && nw_proto != IPPROTO_ICMP
 && nw_proto != IPPROTO_ICMPV6) {
@@ -2292,6 +2299,7 @@ flow_hash_5tuple(const struct flow *flow, uint32_t basis)
 {
 BUILD_ASSERT_DECL(FLOW_WC_SEQ == 42);
 uint32_t hash = basis;
+uint8_t nw_frag = 0;
 
 if (flow) {
 
@@ -2312,6 +2320,9 @@ flow_hash_5tuple(const struct flow *flow, uint32_t basis)
 }
 
 hash = hash_add(hash, flow->nw_proto);
+if (nw_frag & FLOW_NW_FRAG_MASK) {
+goto out;
+}
 if (flow->nw_proto != IPPROTO_TCP && flow->nw_proto != IPPROTO_UDP
 && flow->nw_proto != IPPROTO_SCTP && flow->nw_proto != IPPROTO_ICMP
 && flow->nw_proto != IPPROTO_ICMPV6) {
-- 
2.34.1

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


[ovs-dev] [PATCH] flow: Consistent VXLAN UDP src ports for fragmented packets

2022-11-03 Thread Hemanth Aramadaka via dev
Issue:

The src-port for UDP is based on RSS hash in the packet metadata.
In case of packets coming from VM it will be 5-tuple, if available,
otherwise just IP addresses.If the VM fragments a large IP packet
and sends the fragments to ovs, only the first fragment will contain
the L4 header. Therefore, the first fragment and subsequent fragments
get different UDP src ports in the outgoing VXLAN header.This can
lead to fragment re-ordering in the fabric as packet will take
different paths.

Fix:

Intention of this is to avoid fragment packets taking different paths.
For example, due to presence of firewalls, fragment packets will take
different paths and will get dropped.To avoid this we ignore the L4
header during hash calculation only in the case of fragmented packets.

Signed-off-by: Hemanth Aramadaka 
---
 lib/flow.c | 17 ++---
 1 file changed, 14 insertions(+), 3 deletions(-)

diff --git a/lib/flow.c b/lib/flow.c
index c3a3aa3ce..170c825ab 100644
--- a/lib/flow.c
+++ b/lib/flow.c
@@ -1018,7 +1018,9 @@ miniflow_extract(struct dp_packet *packet, struct 
miniflow *dst)
 miniflow_push_be16(mf, ct_tp_src, ct_tp_src);
 miniflow_push_be16(mf, ct_tp_dst, ct_tp_dst);
 if (dl_type == htons(ETH_TYPE_IP)) {
-dp_packet_update_rss_hash_ipv4_tcp_udp(packet);
+if (!(nw_frag & FLOW_NW_FRAG_MASK)) {
+dp_packet_update_rss_hash_ipv4_tcp_udp(packet);
+}
 } else if (dl_type == htons(ETH_TYPE_IPV6)) {
 dp_packet_update_rss_hash_ipv6_tcp_udp(packet);
 }
@@ -1033,7 +1035,9 @@ miniflow_extract(struct dp_packet *packet, struct 
miniflow *dst)
 miniflow_push_be16(mf, ct_tp_src, ct_tp_src);
 miniflow_push_be16(mf, ct_tp_dst, ct_tp_dst);
 if (dl_type == htons(ETH_TYPE_IP)) {
-dp_packet_update_rss_hash_ipv4_tcp_udp(packet);
+if (!(nw_frag & FLOW_NW_FRAG_MASK)) {
+dp_packet_update_rss_hash_ipv4_tcp_udp(packet);
+}
 } else if (dl_type == htons(ETH_TYPE_IPV6)) {
 dp_packet_update_rss_hash_ipv6_tcp_udp(packet);
 }
@@ -2248,7 +2252,7 @@ miniflow_hash_5tuple(const struct miniflow *flow, 
uint32_t basis)
 
 if (flow) {
 ovs_be16 dl_type = MINIFLOW_GET_BE16(flow, dl_type);
-uint8_t nw_proto;
+uint8_t nw_proto, nw_frag;
 
 if (dl_type == htons(ETH_TYPE_IPV6)) {
 struct flowmap map = FLOWMAP_EMPTY_INITIALIZER;
@@ -2270,6 +2274,9 @@ miniflow_hash_5tuple(const struct miniflow *flow, 
uint32_t basis)
 
 nw_proto = MINIFLOW_GET_U8(flow, nw_proto);
 hash = hash_add(hash, nw_proto);
+if (nw_frag & FLOW_NW_FRAG_MASK) {
+goto out;
+}
 if (nw_proto != IPPROTO_TCP && nw_proto != IPPROTO_UDP
 && nw_proto != IPPROTO_SCTP && nw_proto != IPPROTO_ICMP
 && nw_proto != IPPROTO_ICMPV6) {
@@ -2292,6 +2299,7 @@ flow_hash_5tuple(const struct flow *flow, uint32_t basis)
 {
 BUILD_ASSERT_DECL(FLOW_WC_SEQ == 42);
 uint32_t hash = basis;
+uint8_t nw_frag;
 
 if (flow) {
 
@@ -2312,6 +2320,9 @@ flow_hash_5tuple(const struct flow *flow, uint32_t basis)
 }
 
 hash = hash_add(hash, flow->nw_proto);
+if (nw_frag & FLOW_NW_FRAG_MASK) {
+goto out;
+}
 if (flow->nw_proto != IPPROTO_TCP && flow->nw_proto != IPPROTO_UDP
 && flow->nw_proto != IPPROTO_SCTP && flow->nw_proto != IPPROTO_ICMP
 && flow->nw_proto != IPPROTO_ICMPV6) {
-- 
2.34.1

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] [PATCH] flow: Consistent VXLAN UDP src ports for fragmented packets

2022-05-12 Thread Hemanth Aramadaka via dev
Hi Peng,

 

Did the changes got merged ?

 

Thanks,

Hemanth

 

From: Peng He  
Sent: 09 May 2022 09:09
To: Hemanth Aramadaka 
Cc: Sriharsha Basavapatna via dev ; Ilya Maximets 

Subject: Re: [ovs-dev] [PATCH] flow: Consistent VXLAN UDP src ports for 
fragmented packets

 

sorry, ack name is wrong:

 

Acked-by: Peng He mailto:hepeng.0...@bytedance.com> 
>

 

Peng He mailto:xnhp0...@gmail.com> > 于2022年5月9日周一 11:36写道:

the patch looks good to me. and I also tried to merge it on the master, it 
works.

 

Acked-by: hepeng.0320 mailto:hepeng.0...@bytedance.com> >

 

 

 

 

 

Hemanth Aramadaka mailto:hemant...@acldigital.com> > 
于2022年5月4日周三 18:00写道:

Hi Peng,

 

Could you please review the below request.

 

Thanks,

Hemanth.

 

From: Peng He mailto:xnhp0...@gmail.com> > 
Sent: 08 April 2022 07:50
To: Hemanth Aramadaka mailto:hemant...@acldigital.com> >
Cc: Sriharsha Basavapatna via dev mailto:ovs-dev@openvswitch.org> >; Ilya Maximets mailto:i.maxim...@ovn.org> >
Subject: Re: [ovs-dev] [PATCH] flow: Consistent VXLAN UDP src ports for 
fragmented packets

 

got it. thanks!

 

 

 

Hemanth Aramadaka mailto:hemant...@acldigital.com> > 
于2022年4月8日周五 00:36写道:

Hi,

 

Here we are not fragmenting VXLAN packets in the outer IP header.The packets 
originating from VM with the larger size than the configured MTU 

will get fragmented and these inner packets are encapsulated with vxlan header 
in which we have the different source ports for UDP.

 

Thanks,

Hemanth.

 

From: Peng He mailto:xnhp0...@gmail.com> > 
Sent: 16 March 2022 11:30
To: Hemanth Aramadaka mailto:hemant...@acldigital.com> >
Cc: Sriharsha Basavapatna via dev mailto:ovs-dev@openvswitch.org> >; Ilya Maximets mailto:i.maxim...@ovn.org> >
Subject: Re: [ovs-dev] [PATCH] flow: Consistent VXLAN UDP src ports for 
fragmented packets

 

Normally, VXLAN packets will be set to DF( don't frag) in the outter IP header.

I am trying to understand why fragmentation happens in the first place.

 

Hemanth Aramadaka via dev mailto:ovs-dev@openvswitch.org> > 于2022年3月15日周二 22:38写道:

Issue:

The src-port for UDP is based on RSS hash in the packet metadata.
In case of packets coming from VM it will be 5-tuple, if available,
otherwise just IP addresses.If the VM fragments a large IP packet
and sends the fragments to ovs, only the first fragment will contain
the L4 header. Therefore, the first fragment and subsequent fragments
get different UDP src ports in the outgoing VXLAN header.This can
lead to fragment re-ordering in the fabric as packet will take
different paths.

Fix:

Intention of this is to avoid fragment packets taking different paths.
For example, due to presence of firewalls, fragment packets will take
different paths and will get dropped.To avoid this we ignore the L4
header during hash calculation only in the case of fragmented packets.

P.S: There is already a review request raised for the same.
https://mail.openvswitch.org/pipermail/ovs-dev/2021-January/379395.html. 
<https://mail.openvswitch.org/pipermail/ovs-dev/2021-January/379395.html.Re-iterating>
 
Re-iterating the same patch request on the latest master code.

Signed-off-by: Hemanth Aramadaka mailto:hemant...@acldigital.com> >
---
 lib/flow.c | 10 +-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/lib/flow.c b/lib/flow.c
index dd523c889..17bd47724 100644
--- a/lib/flow.c
+++ b/lib/flow.c
@@ -2238,7 +2238,7 @@ miniflow_hash_5tuple(const struct miniflow *flow, 
uint32_t basis)

 if (flow) {
 ovs_be16 dl_type = MINIFLOW_GET_BE16(flow, dl_type);
-uint8_t nw_proto;
+uint8_t nw_proto,nw_frag;

 if (dl_type == htons(ETH_TYPE_IPV6)) {
 struct flowmap map = FLOWMAP_EMPTY_INITIALIZER;
@@ -2260,6 +2260,14 @@ miniflow_hash_5tuple(const struct miniflow *flow, 
uint32_t basis)

 nw_proto = MINIFLOW_GET_U8(flow, nw_proto);
 hash = hash_add(hash, nw_proto);
+/* Skip l4 header fields if IP packet is fragmented since
+ * only first fragment will carry l4 header.
+ */
+nw_frag = MINIFLOW_GET_U8(flow, nw_frag);
+if ((nw_frag)) {
+goto out;
+}
+
 if (nw_proto != IPPROTO_TCP && nw_proto != IPPROTO_UDP
 && nw_proto != IPPROTO_SCTP && nw_proto != IPPROTO_ICMP
 && nw_proto != IPPROTO_ICMPV6) {
-- 
2.25.1

___
dev mailing list
d...@openvswitch.org <mailto:d...@openvswitch.org> 
https://mail.openvswitch.org/mailman/listinfo/ovs-dev




 

-- 

hepeng




 

-- 

hepeng




 

-- 

hepeng




 

-- 

hepeng

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] [PATCH] flow: Consistent VXLAN UDP src ports for fragmented packets

2022-05-04 Thread Hemanth Aramadaka via dev
Hi Peng,

 

Could you please review the below request.

 

Thanks,

Hemanth.

 

From: Peng He  
Sent: 08 April 2022 07:50
To: Hemanth Aramadaka 
Cc: Sriharsha Basavapatna via dev ; Ilya Maximets 

Subject: Re: [ovs-dev] [PATCH] flow: Consistent VXLAN UDP src ports for 
fragmented packets

 

got it. thanks!

 

 

 

Hemanth Aramadaka mailto:hemant...@acldigital.com> > 
于2022年4月8日周五 00:36写道:

Hi,

 

Here we are not fragmenting VXLAN packets in the outer IP header.The packets 
originating from VM with the larger size than the configured MTU 

will get fragmented and these inner packets are encapsulated with vxlan header 
in which we have the different source ports for UDP.

 

Thanks,

Hemanth.

 

From: Peng He mailto:xnhp0...@gmail.com> > 
Sent: 16 March 2022 11:30
To: Hemanth Aramadaka mailto:hemant...@acldigital.com> >
Cc: Sriharsha Basavapatna via dev mailto:ovs-dev@openvswitch.org> >; Ilya Maximets mailto:i.maxim...@ovn.org> >
Subject: Re: [ovs-dev] [PATCH] flow: Consistent VXLAN UDP src ports for 
fragmented packets

 

Normally, VXLAN packets will be set to DF( don't frag) in the outter IP header.

I am trying to understand why fragmentation happens in the first place.

 

Hemanth Aramadaka via dev mailto:ovs-dev@openvswitch.org> > 于2022年3月15日周二 22:38写道:

Issue:

The src-port for UDP is based on RSS hash in the packet metadata.
In case of packets coming from VM it will be 5-tuple, if available,
otherwise just IP addresses.If the VM fragments a large IP packet
and sends the fragments to ovs, only the first fragment will contain
the L4 header. Therefore, the first fragment and subsequent fragments
get different UDP src ports in the outgoing VXLAN header.This can
lead to fragment re-ordering in the fabric as packet will take
different paths.

Fix:

Intention of this is to avoid fragment packets taking different paths.
For example, due to presence of firewalls, fragment packets will take
different paths and will get dropped.To avoid this we ignore the L4
header during hash calculation only in the case of fragmented packets.

P.S: There is already a review request raised for the same.
https://mail.openvswitch.org/pipermail/ovs-dev/2021-January/379395.html. 
<https://mail.openvswitch.org/pipermail/ovs-dev/2021-January/379395.html.Re-iterating>
 
Re-iterating the same patch request on the latest master code.

Signed-off-by: Hemanth Aramadaka mailto:hemant...@acldigital.com> >
---
 lib/flow.c | 10 +-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/lib/flow.c b/lib/flow.c
index dd523c889..17bd47724 100644
--- a/lib/flow.c
+++ b/lib/flow.c
@@ -2238,7 +2238,7 @@ miniflow_hash_5tuple(const struct miniflow *flow, 
uint32_t basis)

 if (flow) {
 ovs_be16 dl_type = MINIFLOW_GET_BE16(flow, dl_type);
-uint8_t nw_proto;
+uint8_t nw_proto,nw_frag;

 if (dl_type == htons(ETH_TYPE_IPV6)) {
 struct flowmap map = FLOWMAP_EMPTY_INITIALIZER;
@@ -2260,6 +2260,14 @@ miniflow_hash_5tuple(const struct miniflow *flow, 
uint32_t basis)

 nw_proto = MINIFLOW_GET_U8(flow, nw_proto);
 hash = hash_add(hash, nw_proto);
+/* Skip l4 header fields if IP packet is fragmented since
+ * only first fragment will carry l4 header.
+ */
+nw_frag = MINIFLOW_GET_U8(flow, nw_frag);
+if ((nw_frag)) {
+goto out;
+}
+
 if (nw_proto != IPPROTO_TCP && nw_proto != IPPROTO_UDP
 && nw_proto != IPPROTO_SCTP && nw_proto != IPPROTO_ICMP
 && nw_proto != IPPROTO_ICMPV6) {
-- 
2.25.1

___
dev mailing list
d...@openvswitch.org <mailto:d...@openvswitch.org> 
https://mail.openvswitch.org/mailman/listinfo/ovs-dev




 

-- 

hepeng




 

-- 

hepeng

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] [PATCH] flow: Consistent VXLAN UDP src ports for fragmented packets

2022-04-07 Thread Hemanth Aramadaka via dev
Hi,

 

Here we are not fragmenting VXLAN packets in the outer IP header.The packets 
originating from VM with the larger size than the configured MTU 

will get fragmented and these inner packets are encapsulated with vxlan header 
in which we have the different source ports for UDP.

 

Thanks,

Hemanth.

 

From: Peng He  
Sent: 16 March 2022 11:30
To: Hemanth Aramadaka 
Cc: Sriharsha Basavapatna via dev ; Ilya Maximets 

Subject: Re: [ovs-dev] [PATCH] flow: Consistent VXLAN UDP src ports for 
fragmented packets

 

Normally, VXLAN packets will be set to DF( don't frag) in the outter IP header.

I am trying to understand why fragmentation happens in the first place.

 

Hemanth Aramadaka via dev mailto:ovs-dev@openvswitch.org> > 于2022年3月15日周二 22:38写道:

Issue:

The src-port for UDP is based on RSS hash in the packet metadata.
In case of packets coming from VM it will be 5-tuple, if available,
otherwise just IP addresses.If the VM fragments a large IP packet
and sends the fragments to ovs, only the first fragment will contain
the L4 header. Therefore, the first fragment and subsequent fragments
get different UDP src ports in the outgoing VXLAN header.This can
lead to fragment re-ordering in the fabric as packet will take
different paths.

Fix:

Intention of this is to avoid fragment packets taking different paths.
For example, due to presence of firewalls, fragment packets will take
different paths and will get dropped.To avoid this we ignore the L4
header during hash calculation only in the case of fragmented packets.

P.S: There is already a review request raised for the same.
https://mail.openvswitch.org/pipermail/ovs-dev/2021-January/379395.html. 
<https://mail.openvswitch.org/pipermail/ovs-dev/2021-January/379395.html.Re-iterating>
 
Re-iterating the same patch request on the latest master code.

Signed-off-by: Hemanth Aramadaka mailto:hemant...@acldigital.com> >
---
 lib/flow.c | 10 +-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/lib/flow.c b/lib/flow.c
index dd523c889..17bd47724 100644
--- a/lib/flow.c
+++ b/lib/flow.c
@@ -2238,7 +2238,7 @@ miniflow_hash_5tuple(const struct miniflow *flow, 
uint32_t basis)

 if (flow) {
 ovs_be16 dl_type = MINIFLOW_GET_BE16(flow, dl_type);
-uint8_t nw_proto;
+uint8_t nw_proto,nw_frag;

 if (dl_type == htons(ETH_TYPE_IPV6)) {
 struct flowmap map = FLOWMAP_EMPTY_INITIALIZER;
@@ -2260,6 +2260,14 @@ miniflow_hash_5tuple(const struct miniflow *flow, 
uint32_t basis)

 nw_proto = MINIFLOW_GET_U8(flow, nw_proto);
 hash = hash_add(hash, nw_proto);
+/* Skip l4 header fields if IP packet is fragmented since
+ * only first fragment will carry l4 header.
+ */
+nw_frag = MINIFLOW_GET_U8(flow, nw_frag);
+if ((nw_frag)) {
+goto out;
+}
+
 if (nw_proto != IPPROTO_TCP && nw_proto != IPPROTO_UDP
 && nw_proto != IPPROTO_SCTP && nw_proto != IPPROTO_ICMP
 && nw_proto != IPPROTO_ICMPV6) {
-- 
2.25.1

___
dev mailing list
d...@openvswitch.org <mailto:d...@openvswitch.org> 
https://mail.openvswitch.org/mailman/listinfo/ovs-dev




 

-- 

hepeng

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


[ovs-dev] [PATCH] flow: Consistent VXLAN UDP src ports for fragmented packets

2022-03-15 Thread Hemanth Aramadaka via dev
Issue:

The src-port for UDP is based on RSS hash in the packet metadata.
In case of packets coming from VM it will be 5-tuple, if available,
otherwise just IP addresses.If the VM fragments a large IP packet
and sends the fragments to ovs, only the first fragment will contain
the L4 header. Therefore, the first fragment and subsequent fragments
get different UDP src ports in the outgoing VXLAN header.This can
lead to fragment re-ordering in the fabric as packet will take
different paths.

Fix:

Intention of this is to avoid fragment packets taking different paths.
For example, due to presence of firewalls, fragment packets will take
different paths and will get dropped.To avoid this we ignore the L4
header during hash calculation only in the case of fragmented packets.

P.S: There is already a review request raised for the same.
https://mail.openvswitch.org/pipermail/ovs-dev/2021-January/379395.html.
Re-iterating the same patch request on the latest master code.

Signed-off-by: Hemanth Aramadaka 
---
 lib/flow.c | 10 +-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/lib/flow.c b/lib/flow.c
index dd523c889..17bd47724 100644
--- a/lib/flow.c
+++ b/lib/flow.c
@@ -2238,7 +2238,7 @@ miniflow_hash_5tuple(const struct miniflow *flow, 
uint32_t basis)
 
 if (flow) {
 ovs_be16 dl_type = MINIFLOW_GET_BE16(flow, dl_type);
-uint8_t nw_proto;
+uint8_t nw_proto,nw_frag;
 
 if (dl_type == htons(ETH_TYPE_IPV6)) {
 struct flowmap map = FLOWMAP_EMPTY_INITIALIZER;
@@ -2260,6 +2260,14 @@ miniflow_hash_5tuple(const struct miniflow *flow, 
uint32_t basis)
 
 nw_proto = MINIFLOW_GET_U8(flow, nw_proto);
 hash = hash_add(hash, nw_proto);
+/* Skip l4 header fields if IP packet is fragmented since
+ * only first fragment will carry l4 header.
+ */
+nw_frag = MINIFLOW_GET_U8(flow, nw_frag);
+if ((nw_frag)) {
+goto out;
+}
+
 if (nw_proto != IPPROTO_TCP && nw_proto != IPPROTO_UDP
 && nw_proto != IPPROTO_SCTP && nw_proto != IPPROTO_ICMP
 && nw_proto != IPPROTO_ICMPV6) {
-- 
2.25.1

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev