Re: [ovs-dev] [PATCH] conntrack: Fix tcp payload length in case multi-segments.
Hi Zhike
Thanks for clarifying
There is presently no support for multi-segment mbufs in OVS, so the patch
would
not be needed and in general patches are only proposed at the time they are
relevant.
Also note that in the hypothetical case of multisegment mbufs, the packet
would be linearized at
this point.
Also note that this patch still relies on pointers..
As Ilya already mentioned there is no need to use multi-segment mbufs on
OVS as large buffers
can be used instead.
On Sun, Nov 10, 2019 at 9:50 PM 王志克 wrote:
> Hi Darrell,
>
> In TSO case, the packet may use multi-segments mbuf, and I do not think we
> need to make it linearal. In this case, we can NOT use pointer to calculate
> the tcp length.
>
> Br,
>
> Zhike Wang
> JDCloud, Product Development, IaaS
>
>
> Mobile／+86 13466719566
> E- mail／wangzh...@jd.com
> Address／5F Building A,North-Star Century Center,8 Beichen West
> Street,Chaoyang District Beijing
> Https://JDCloud.com
>
>
>
>
> From: Darrell Ball [mailto:dlu...@gmail.com]
> Sent: Saturday, November 09, 2019 8:12 AM
> To: Zhike Wang
> Cc: ovs dev; 王志克
> Subject: Re: [ovs-dev] [PATCH] conntrack: Fix tcp payload length in case
> multi-segments.
>
> Thanks for the patch
>
> Would you mind describing the use case that this patch is aiming to
> support ?
>
> On Fri, Nov 8, 2019 at 1:23 AM Zhike Wang wrote:
> Signed-off-by: Zhike Wang
> ---
> lib/conntrack-private.h | 16 ++--
> 1 file changed, 10 insertions(+), 6 deletions(-)
>
> diff --git a/lib/conntrack-private.h b/lib/conntrack-private.h
> index 590f139..1d21f6e 100644
> --- a/lib/conntrack-private.h
> +++ b/lib/conntrack-private.h
> @@ -233,13 +233,17 @@ conn_update_expiration(struct conntrack *ct, struct
> conn *conn,
> static inline uint32_t
> tcp_payload_length(struct dp_packet *pkt)
> {
> -const char *tcp_payload = dp_packet_get_tcp_payload(pkt);
> -if (tcp_payload) {
> -return ((char *) dp_packet_tail(pkt) - dp_packet_l2_pad_size(pkt)
> -- tcp_payload);
> -} else {
> -return 0;
> +size_t l4_size = dp_packet_l4_size(pkt);
> +
> +if (OVS_LIKELY(l4_size >= TCP_HEADER_LEN)) {
> +struct tcp_header *tcp = dp_packet_l4(pkt);
> +int tcp_len = TCP_OFFSET(tcp->tcp_ctl) * 4;
> +
> +if (OVS_LIKELY(tcp_len >= TCP_HEADER_LEN && tcp_len <= l4_size)) {
> +return (l4_size - tcp_len);
> +}
>
> Maybe I missed something, but it looks like the same calculation is
> arrived at.
>
> }
> +return 0;
> }
>
> #endif /* conntrack-private.h */
> --
> 1.8.3.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
Re: [ovs-dev] [PATCH] conntrack: Fix tcp payload length in case multi-segments.
> Hi Darrell,
>
> In TSO case, the packet may use multi-segments mbuf, and I do not> think we
> need to make it linearal. In this case, we can NOT use
> pointer to calculate the tcp length.
Hi. Just wanted to mention that current main course for TSO enabling in
OVS seems to be using of extbuf memory in vhost. And corresponding patches
are already accepted [1] to DPDK. So, there will be no multi-segment mbufs.
[1] c3ff0ac70acb ("vhost: improve performance by supporting large buffer")
Best regards, Ilya Maximets.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] conntrack: Fix tcp payload length in case multi-segments.
Hi Darrell,
In TSO case, the packet may use multi-segments mbuf, and I do not think we need
to make it linearal. In this case, we can NOT use pointer to calculate the tcp
length.
Br,
Zhike Wang
JDCloud, Product Development, IaaS
Mobile／+86 13466719566
E- mail／wangzh...@jd.com
Address／5F Building A,North-Star Century Center,8 Beichen West Street,Chaoyang
District Beijing
Https://JDCloud.com
From: Darrell Ball [mailto:dlu...@gmail.com]
Sent: Saturday, November 09, 2019 8:12 AM
To: Zhike Wang
Cc: ovs dev; 王志克
Subject: Re: [ovs-dev] [PATCH] conntrack: Fix tcp payload length in case
multi-segments.
Thanks for the patch
Would you mind describing the use case that this patch is aiming to support ?
On Fri, Nov 8, 2019 at 1:23 AM Zhike Wang wrote:
Signed-off-by: Zhike Wang
---
lib/conntrack-private.h | 16 ++--
1 file changed, 10 insertions(+), 6 deletions(-)
diff --git a/lib/conntrack-private.h b/lib/conntrack-private.h
index 590f139..1d21f6e 100644
--- a/lib/conntrack-private.h
+++ b/lib/conntrack-private.h
@@ -233,13 +233,17 @@ conn_update_expiration(struct conntrack *ct, struct conn
*conn,
static inline uint32_t
tcp_payload_length(struct dp_packet *pkt)
{
- const char *tcp_payload = dp_packet_get_tcp_payload(pkt);
- if (tcp_payload) {
- return ((char *) dp_packet_tail(pkt) - dp_packet_l2_pad_size(pkt)
- - tcp_payload);
- } else {
- return 0;
+ size_t l4_size = dp_packet_l4_size(pkt);
+
+ if (OVS_LIKELY(l4_size >= TCP_HEADER_LEN)) {
+ struct tcp_header *tcp = dp_packet_l4(pkt);
+ int tcp_len = TCP_OFFSET(tcp->tcp_ctl) * 4;
+
+ if (OVS_LIKELY(tcp_len >= TCP_HEADER_LEN && tcp_len <= l4_size)) {
+ return (l4_size - tcp_len);
+ }
Maybe I missed something, but it looks like the same calculation is arrived at.
}
+ return 0;
}
#endif /* conntrack-private.h */
--
1.8.3.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
Re: [ovs-dev] [PATCH] conntrack: Fix tcp payload length in case multi-segments.
Thanks for the patch
Would you mind describing the use case that this patch is aiming to support
?
On Fri, Nov 8, 2019 at 1:23 AM Zhike Wang wrote:
> Signed-off-by: Zhike Wang
> ---
> lib/conntrack-private.h | 16 ++--
> 1 file changed, 10 insertions(+), 6 deletions(-)
>
> diff --git a/lib/conntrack-private.h b/lib/conntrack-private.h
> index 590f139..1d21f6e 100644
> --- a/lib/conntrack-private.h
> +++ b/lib/conntrack-private.h
> @@ -233,13 +233,17 @@ conn_update_expiration(struct conntrack *ct, struct
> conn *conn,
> static inline uint32_t
> tcp_payload_length(struct dp_packet *pkt)
> {
> -const char *tcp_payload = dp_packet_get_tcp_payload(pkt);
> -if (tcp_payload) {
> -return ((char *) dp_packet_tail(pkt) - dp_packet_l2_pad_size(pkt)
> -- tcp_payload);
> -} else {
> -return 0;
> +size_t l4_size = dp_packet_l4_size(pkt);
> +
> +if (OVS_LIKELY(l4_size >= TCP_HEADER_LEN)) {
> +struct tcp_header *tcp = dp_packet_l4(pkt);
> +int tcp_len = TCP_OFFSET(tcp->tcp_ctl) * 4;
> +
> +if (OVS_LIKELY(tcp_len >= TCP_HEADER_LEN && tcp_len <= l4_size)) {
> +return (l4_size - tcp_len);
> +}
>
Maybe I missed something, but it looks like the same calculation is arrived
at.
> }
> +return 0;
> }
>
> #endif /* conntrack-private.h */
> --
> 1.8.3.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] conntrack: Fix tcp payload length in case multi-segments.
Signed-off-by: Zhike Wang
---
lib/conntrack-private.h | 16 ++--
1 file changed, 10 insertions(+), 6 deletions(-)
diff --git a/lib/conntrack-private.h b/lib/conntrack-private.h
index 590f139..1d21f6e 100644
--- a/lib/conntrack-private.h
+++ b/lib/conntrack-private.h
@@ -233,13 +233,17 @@ conn_update_expiration(struct conntrack *ct, struct conn
*conn,
static inline uint32_t
tcp_payload_length(struct dp_packet *pkt)
{
-const char *tcp_payload = dp_packet_get_tcp_payload(pkt);
-if (tcp_payload) {
-return ((char *) dp_packet_tail(pkt) - dp_packet_l2_pad_size(pkt)
-- tcp_payload);
-} else {
-return 0;
+size_t l4_size = dp_packet_l4_size(pkt);
+
+if (OVS_LIKELY(l4_size >= TCP_HEADER_LEN)) {
+struct tcp_header *tcp = dp_packet_l4(pkt);
+int tcp_len = TCP_OFFSET(tcp->tcp_ctl) * 4;
+
+if (OVS_LIKELY(tcp_len >= TCP_HEADER_LEN && tcp_len <= l4_size)) {
+return (l4_size - tcp_len);
+}
}
+return 0;
}
#endif /* conntrack-private.h */
--
1.8.3.1
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
