Re: [ovs-dev] [PATCH v6 2/3] userspace: Respect tso/gso segment size.
On 10/31/23 20:51, Mike Pattrick wrote:
> From: Flavio Leitner
>
> Currently OVS will calculate the segment size based on the
> MTU of the egress port. That usually happens to be correct
> when the ports share the same MTU, but that is not always true.
>
> Therefore, if the segment size is provided, then use that and
> make sure the over sized packets are dropped.
>
> Signed-off-by: Flavio Leitner
> Co-authored-by: Mike Pattrick
> Signed-off-by: Mike Pattrick
> ---
> lib/dp-packet.c| 3 ++
> lib/dp-packet.h| 26
> lib/netdev-dpdk.c | 12 +++-
> lib/netdev-linux.c | 76 --
> 4 files changed, 94 insertions(+), 23 deletions(-)
>
> diff --git a/lib/dp-packet.c b/lib/dp-packet.c
> index ed004c3b9..920402369 100644
> --- a/lib/dp-packet.c
> +++ b/lib/dp-packet.c
> @@ -34,6 +34,7 @@ dp_packet_init__(struct dp_packet *b, size_t allocated,
> enum dp_packet_source so
> pkt_metadata_init(>md, 0);
> dp_packet_reset_cutlen(b);
> dp_packet_reset_offload(b);
> +dp_packet_set_tso_segsz(b, 0);
> /* Initialize implementation-specific fields of dp_packet. */
> dp_packet_init_specific(b);
> /* By default assume the packet type to be Ethernet. */
> @@ -203,6 +204,8 @@ dp_packet_clone_with_headroom(const struct dp_packet
> *buffer, size_t headroom)
> *dp_packet_ol_flags_ptr(new_buffer) = *dp_packet_ol_flags_ptr(buffer);
> *dp_packet_ol_flags_ptr(new_buffer) &= DP_PACKET_OL_SUPPORTED_MASK;
>
> +dp_packet_set_tso_segsz(new_buffer, dp_packet_get_tso_segsz(buffer));
> +
> if (dp_packet_rss_valid(buffer)) {
> dp_packet_set_rss_hash(new_buffer, dp_packet_get_rss_hash(buffer));
> }
> diff --git a/lib/dp-packet.h b/lib/dp-packet.h
> index 70ddf8aa4..6a924f3ff 100644
> --- a/lib/dp-packet.h
> +++ b/lib/dp-packet.h
> @@ -126,6 +126,7 @@ struct dp_packet {
> uint32_t ol_flags; /* Offloading flags. */
> uint32_t rss_hash; /* Packet hash. */
> uint32_t flow_mark; /* Packet flow mark. */
> +uint16_t tso_segsz; /* TCP TSO segment size. */
> #endif
> enum dp_packet_source source; /* Source of memory allocated as 'base'.
> */
>
> @@ -166,6 +167,9 @@ static inline void dp_packet_set_size(struct dp_packet *,
> uint32_t);
> static inline uint16_t dp_packet_get_allocated(const struct dp_packet *);
> static inline void dp_packet_set_allocated(struct dp_packet *, uint16_t);
>
> +static inline uint16_t dp_packet_get_tso_segsz(const struct dp_packet *);
> +static inline void dp_packet_set_tso_segsz(struct dp_packet *, uint16_t);
> +
> void *dp_packet_resize_l2(struct dp_packet *, int increment);
> void *dp_packet_resize_l2_5(struct dp_packet *, int increment);
> static inline void *dp_packet_eth(const struct dp_packet *);
> @@ -644,6 +648,17 @@ dp_packet_set_allocated(struct dp_packet *b, uint16_t s)
> b->mbuf.buf_len = s;
> }
>
> +static inline uint16_t
> +dp_packet_get_tso_segsz(const struct dp_packet *p)
> +{
> +return p->mbuf.tso_segsz;
> +}
> +
> +static inline void
> +dp_packet_set_tso_segsz(struct dp_packet *p, uint16_t s)
> +{
> +p->mbuf.tso_segsz = s;
> +}
> #else /* DPDK_NETDEV */
>
> static inline void
> @@ -700,6 +715,17 @@ dp_packet_set_allocated(struct dp_packet *b, uint16_t s)
> b->allocated_ = s;
> }
>
> +static inline uint16_t
> +dp_packet_get_tso_segsz(const struct dp_packet *p)
> +{
> +return p->tso_segsz;
> +}
> +
> +static inline void
> +dp_packet_set_tso_segsz(struct dp_packet *p, uint16_t s)
> +{
> +p->tso_segsz = s;
> +}
> #endif /* DPDK_NETDEV */
>
> static inline void
> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
> index 55700250d..9f20cc689 100644
> --- a/lib/netdev-dpdk.c
> +++ b/lib/netdev-dpdk.c
> @@ -2453,6 +2453,7 @@ netdev_dpdk_prep_hwol_packet(struct netdev_dpdk *dev,
> struct rte_mbuf *mbuf)
>
> if (mbuf->ol_flags & RTE_MBUF_F_TX_TCP_SEG) {
> struct tcp_header *th = dp_packet_l4(pkt);
> +int hdr_len;
>
> if (!th) {
> VLOG_WARN_RL(, "%s: TCP Segmentation without L4 header"
> @@ -2462,7 +2463,15 @@ netdev_dpdk_prep_hwol_packet(struct netdev_dpdk *dev,
> struct rte_mbuf *mbuf)
>
> mbuf->l4_len = TCP_OFFSET(th->tcp_ctl) * 4;
> mbuf->ol_flags |= RTE_MBUF_F_TX_TCP_CKSUM;
> +hdr_len = mbuf->l2_len + mbuf->l3_len + mbuf->l4_len;
> mbuf->tso_segsz = dev->mtu - mbuf->l3_len - mbuf->l4_len;
> +if (OVS_UNLIKELY((hdr_len + mbuf->tso_segsz) > dev->max_packet_len))
> {
> +VLOG_WARN_RL(, "%s: Oversized TSO packet. "
> + "hdr: %"PRIu32", gso: %"PRIu32", max len:
> %"PRIu32"",
> + dev->up.name, hdr_len, mbuf->tso_segsz,
> + dev->max_packet_len);
> +return false;
> +}
>
> if (mbuf->ol_flags & RTE_MBUF_F_TX_IPV4) {
> mbuf->ol_flags |=
Re: [ovs-dev] [PATCH v6 2/3] userspace: Respect tso/gso segment size.
On Tue, Oct 31, 2023 at 03:51:37PM -0400, Mike Pattrick wrote:
> From: Flavio Leitner
>
> Currently OVS will calculate the segment size based on the
> MTU of the egress port. That usually happens to be correct
> when the ports share the same MTU, but that is not always true.
>
> Therefore, if the segment size is provided, then use that and
> make sure the over sized packets are dropped.
>
> Signed-off-by: Flavio Leitner
> Co-authored-by: Mike Pattrick
> Signed-off-by: Mike Pattrick
...
> diff --git a/lib/dp-packet.h b/lib/dp-packet.h
> index 70ddf8aa4..6a924f3ff 100644
> --- a/lib/dp-packet.h
> +++ b/lib/dp-packet.h
> @@ -126,6 +126,7 @@ struct dp_packet {
> uint32_t ol_flags; /* Offloading flags. */
> uint32_t rss_hash; /* Packet hash. */
> uint32_t flow_mark; /* Packet flow mark. */
> +uint16_t tso_segsz; /* TCP TSO segment size. */
Hi Mike,
I don't think there is any need to respin just because of this,
but in my view "TCP TSO" is a tautology. This is because, AFAIK,
the T in TSO stands for TCP.
The above notwithstanding this patch looks good to me.
Acked-by: Simon Horman
...
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH v6 2/3] userspace: Respect tso/gso segment size.
From: Flavio Leitner
Currently OVS will calculate the segment size based on the
MTU of the egress port. That usually happens to be correct
when the ports share the same MTU, but that is not always true.
Therefore, if the segment size is provided, then use that and
make sure the over sized packets are dropped.
Signed-off-by: Flavio Leitner
Co-authored-by: Mike Pattrick
Signed-off-by: Mike Pattrick
---
lib/dp-packet.c| 3 ++
lib/dp-packet.h| 26
lib/netdev-dpdk.c | 12 +++-
lib/netdev-linux.c | 76 --
4 files changed, 94 insertions(+), 23 deletions(-)
diff --git a/lib/dp-packet.c b/lib/dp-packet.c
index ed004c3b9..920402369 100644
--- a/lib/dp-packet.c
+++ b/lib/dp-packet.c
@@ -34,6 +34,7 @@ dp_packet_init__(struct dp_packet *b, size_t allocated, enum
dp_packet_source so
pkt_metadata_init(>md, 0);
dp_packet_reset_cutlen(b);
dp_packet_reset_offload(b);
+dp_packet_set_tso_segsz(b, 0);
/* Initialize implementation-specific fields of dp_packet. */
dp_packet_init_specific(b);
/* By default assume the packet type to be Ethernet. */
@@ -203,6 +204,8 @@ dp_packet_clone_with_headroom(const struct dp_packet
*buffer, size_t headroom)
*dp_packet_ol_flags_ptr(new_buffer) = *dp_packet_ol_flags_ptr(buffer);
*dp_packet_ol_flags_ptr(new_buffer) &= DP_PACKET_OL_SUPPORTED_MASK;
+dp_packet_set_tso_segsz(new_buffer, dp_packet_get_tso_segsz(buffer));
+
if (dp_packet_rss_valid(buffer)) {
dp_packet_set_rss_hash(new_buffer, dp_packet_get_rss_hash(buffer));
}
diff --git a/lib/dp-packet.h b/lib/dp-packet.h
index 70ddf8aa4..6a924f3ff 100644
--- a/lib/dp-packet.h
+++ b/lib/dp-packet.h
@@ -126,6 +126,7 @@ struct dp_packet {
uint32_t ol_flags; /* Offloading flags. */
uint32_t rss_hash; /* Packet hash. */
uint32_t flow_mark; /* Packet flow mark. */
+uint16_t tso_segsz; /* TCP TSO segment size. */
#endif
enum dp_packet_source source; /* Source of memory allocated as 'base'. */
@@ -166,6 +167,9 @@ static inline void dp_packet_set_size(struct dp_packet *,
uint32_t);
static inline uint16_t dp_packet_get_allocated(const struct dp_packet *);
static inline void dp_packet_set_allocated(struct dp_packet *, uint16_t);
+static inline uint16_t dp_packet_get_tso_segsz(const struct dp_packet *);
+static inline void dp_packet_set_tso_segsz(struct dp_packet *, uint16_t);
+
void *dp_packet_resize_l2(struct dp_packet *, int increment);
void *dp_packet_resize_l2_5(struct dp_packet *, int increment);
static inline void *dp_packet_eth(const struct dp_packet *);
@@ -644,6 +648,17 @@ dp_packet_set_allocated(struct dp_packet *b, uint16_t s)
b->mbuf.buf_len = s;
}
+static inline uint16_t
+dp_packet_get_tso_segsz(const struct dp_packet *p)
+{
+return p->mbuf.tso_segsz;
+}
+
+static inline void
+dp_packet_set_tso_segsz(struct dp_packet *p, uint16_t s)
+{
+p->mbuf.tso_segsz = s;
+}
#else /* DPDK_NETDEV */
static inline void
@@ -700,6 +715,17 @@ dp_packet_set_allocated(struct dp_packet *b, uint16_t s)
b->allocated_ = s;
}
+static inline uint16_t
+dp_packet_get_tso_segsz(const struct dp_packet *p)
+{
+return p->tso_segsz;
+}
+
+static inline void
+dp_packet_set_tso_segsz(struct dp_packet *p, uint16_t s)
+{
+p->tso_segsz = s;
+}
#endif /* DPDK_NETDEV */
static inline void
diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index 55700250d..9f20cc689 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -2453,6 +2453,7 @@ netdev_dpdk_prep_hwol_packet(struct netdev_dpdk *dev,
struct rte_mbuf *mbuf)
if (mbuf->ol_flags & RTE_MBUF_F_TX_TCP_SEG) {
struct tcp_header *th = dp_packet_l4(pkt);
+int hdr_len;
if (!th) {
VLOG_WARN_RL(, "%s: TCP Segmentation without L4 header"
@@ -2462,7 +2463,15 @@ netdev_dpdk_prep_hwol_packet(struct netdev_dpdk *dev,
struct rte_mbuf *mbuf)
mbuf->l4_len = TCP_OFFSET(th->tcp_ctl) * 4;
mbuf->ol_flags |= RTE_MBUF_F_TX_TCP_CKSUM;
+hdr_len = mbuf->l2_len + mbuf->l3_len + mbuf->l4_len;
mbuf->tso_segsz = dev->mtu - mbuf->l3_len - mbuf->l4_len;
+if (OVS_UNLIKELY((hdr_len + mbuf->tso_segsz) > dev->max_packet_len)) {
+VLOG_WARN_RL(, "%s: Oversized TSO packet. "
+ "hdr: %"PRIu32", gso: %"PRIu32", max len: %"PRIu32"",
+ dev->up.name, hdr_len, mbuf->tso_segsz,
+ dev->max_packet_len);
+return false;
+}
if (mbuf->ol_flags & RTE_MBUF_F_TX_IPV4) {
mbuf->ol_flags |= RTE_MBUF_F_TX_IP_CKSUM;
@@ -2737,7 +2746,8 @@ netdev_dpdk_filter_packet_len(struct netdev_dpdk *dev,
struct rte_mbuf **pkts,
int cnt = 0;
struct rte_mbuf *pkt;
-/* Filter oversized packets, unless are marked for TSO. */
+/* Filter oversized packets. The TSO packets are filtered out
+ * during the
