Re: [ovs-dev] [PATCH v3 2/2] userspace: Add Generic Segmentation Offloading.
On Tue, Jul 4, 2023 at 9:00 PM Ilya Maximets wrote: > > On 6/21/23 22:36, Mike Pattrick wrote: > > From: Flavio Leitner > > > > This provides a software implementation in the case > > the egress netdev doesn't support segmentation in hardware. > > > > The challenge here is to guarantee packet ordering in the > > original batch that may be full of TSO packets. Each TSO > > packet can go up to ~64kB, so with segment size of 1440 > > that means about 44 packets for each TSO. Each batch has > > 32 packets, so the total batch amounts to 1408 normal > > packets. > > > > The segmentation estimates the total number of packets > > and then the total number of batches. Then allocate > > enough memory and finally do the work. > > > > Finally each batch is sent in order to the netdev. > > > > Signed-off-by: Flavio Leitner > > Co-authored-by: Mike Pattrick > > Signed-off-by: Mike Pattrick > > --- > > lib/automake.mk | 2 + > > lib/dp-packet-gso.c | 172 > > lib/dp-packet-gso.h | 24 +++ > > lib/dp-packet.h | 11 +++ > > lib/netdev-dpdk.c | 46 > > lib/netdev-linux.c | 58 --- > > lib/netdev.c| 120 ++- > > lib/packets.c | 4 +- > > 8 files changed, 314 insertions(+), 123 deletions(-) > > create mode 100644 lib/dp-packet-gso.c > > create mode 100644 lib/dp-packet-gso.h > > > > diff --git a/lib/automake.mk b/lib/automake.mk > > index e64ee76ce..49a92958d 100644 > > --- a/lib/automake.mk > > +++ b/lib/automake.mk > > @@ -118,6 +118,8 @@ lib_libopenvswitch_la_SOURCES = \ > > lib/dpctl.h \ > > lib/dp-packet.h \ > > lib/dp-packet.c \ > > + lib/dp-packet-gso.c \ > > + lib/dp-packet-gso.h \ > > lib/dpdk.h \ > > lib/dpif-netdev-extract-study.c \ > > lib/dpif-netdev-lookup.h \ > > diff --git a/lib/dp-packet-gso.c b/lib/dp-packet-gso.c > > new file mode 100644 > > index 0..bc72e2f90 > > --- /dev/null > > +++ b/lib/dp-packet-gso.c > > @@ -0,0 +1,172 @@ > > +/* > > + * Copyright (c) 2021 Red Hat, Inc. > > Need to adjust the year. :) > > > + * > > + * Licensed under the Apache License, Version 2.0 (the "License"); > > + * you may not use this file except in compliance with the License. > > + * You may obtain a copy of the License at: > > + * > > + * http://www.apache.org/licenses/LICENSE-2.0 > > + * > > + * Unless required by applicable law or agreed to in writing, software > > + * distributed under the License is distributed on an "AS IS" BASIS, > > + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. > > + * See the License for the specific language governing permissions and > > + * limitations under the License. > > + */ > > + > > +#include > > +#include > > +#include > > + > > +#include "coverage.h" > > +#include "dp-packet.h" > > +#include "dp-packet-gso.h" > > +#include "netdev-provider.h" > > +#include "openvswitch/vlog.h" > > + > > +VLOG_DEFINE_THIS_MODULE(dp_packet_gso); > > + > > +COVERAGE_DEFINE(soft_seg_good); > > + > > +/* Retuns a new packet that is a segment of packet 'p'. > > + * > > + * The new packet is initialized with 'hdr_len' bytes from the > > + * start of packet 'p' and then appended with 'data_len' bytes > > + * from the 'data' buffer. > > + * > > + * Note: The packet headers are not updated. */ > > +static struct dp_packet * > > +dp_packet_gso_seg_new(const struct dp_packet *p, size_t hdr_len, > > + const char *data, size_t data_len) > > +{ > > +struct dp_packet *seg = dp_packet_new_with_headroom(hdr_len + data_len, > > + > > dp_packet_headroom(p)); > > + > > +/* Append the original packet headers and then the payload. */ > > +dp_packet_put(seg, dp_packet_data(p), hdr_len); > > +dp_packet_put(seg, data, data_len); > > + > > +/* The new segment should have the same offsets. */ > > +seg->l2_5_ofs = p->l2_5_ofs; > > +seg->l3_ofs = p->l3_ofs; > > +seg->l4_ofs = p->l4_ofs; > > + > > +/* The protocol headers remain the same, so preserve hash and mark. */ > > +*dp_packet_rss_ptr(seg) = dp_packet_get_rss_hash(p); > > +*dp_packet_flow_mark_ptr(seg) = *dp_packet_flow_mark_ptr(p); > > + > > +/* The segment should inherit all the offloading flags from the > > + * original packet, except for the TCP segmentation, external > > + * buffer and indirect buffer flags. */ > > +*dp_packet_ol_flags_ptr(seg) = *dp_packet_ol_flags_ptr(p) > > +& ~(DP_PACKET_OL_TX_TCP_SEG | DP_PACKET_OL_EXTERNAL > > +| DP_PACKET_OL_INDIRECT); > > We should inherit DP_PACKET_OL_SUPPORTED_MASK & ~DP_PACKET_OL_TX_TCP_SEG. > So, > > *dp_packet_ol_flags_ptr(seg) &= DP_PACKET_OL_SUPPORTED_MASK > & ~DP_PACKET_OL_TX_TCP_SEG; > > Is that right? > > > + > > +dp_packet_hwol_reset_tcp_seg(seg); > > Also, this function just resets the DP_PACK
Re: [ovs-dev] [PATCH v3 2/2] userspace: Add Generic Segmentation Offloading.
On 6/21/23 22:36, Mike Pattrick wrote: > From: Flavio Leitner > > This provides a software implementation in the case > the egress netdev doesn't support segmentation in hardware. > > The challenge here is to guarantee packet ordering in the > original batch that may be full of TSO packets. Each TSO > packet can go up to ~64kB, so with segment size of 1440 > that means about 44 packets for each TSO. Each batch has > 32 packets, so the total batch amounts to 1408 normal > packets. > > The segmentation estimates the total number of packets > and then the total number of batches. Then allocate > enough memory and finally do the work. > > Finally each batch is sent in order to the netdev. > > Signed-off-by: Flavio Leitner > Co-authored-by: Mike Pattrick > Signed-off-by: Mike Pattrick > --- > lib/automake.mk | 2 + > lib/dp-packet-gso.c | 172 > lib/dp-packet-gso.h | 24 +++ > lib/dp-packet.h | 11 +++ > lib/netdev-dpdk.c | 46 > lib/netdev-linux.c | 58 --- > lib/netdev.c| 120 ++- > lib/packets.c | 4 +- > 8 files changed, 314 insertions(+), 123 deletions(-) > create mode 100644 lib/dp-packet-gso.c > create mode 100644 lib/dp-packet-gso.h > > diff --git a/lib/automake.mk b/lib/automake.mk > index e64ee76ce..49a92958d 100644 > --- a/lib/automake.mk > +++ b/lib/automake.mk > @@ -118,6 +118,8 @@ lib_libopenvswitch_la_SOURCES = \ > lib/dpctl.h \ > lib/dp-packet.h \ > lib/dp-packet.c \ > + lib/dp-packet-gso.c \ > + lib/dp-packet-gso.h \ > lib/dpdk.h \ > lib/dpif-netdev-extract-study.c \ > lib/dpif-netdev-lookup.h \ > diff --git a/lib/dp-packet-gso.c b/lib/dp-packet-gso.c > new file mode 100644 > index 0..bc72e2f90 > --- /dev/null > +++ b/lib/dp-packet-gso.c > @@ -0,0 +1,172 @@ > +/* > + * Copyright (c) 2021 Red Hat, Inc. Need to adjust the year. :) > + * > + * Licensed under the Apache License, Version 2.0 (the "License"); > + * you may not use this file except in compliance with the License. > + * You may obtain a copy of the License at: > + * > + * http://www.apache.org/licenses/LICENSE-2.0 > + * > + * Unless required by applicable law or agreed to in writing, software > + * distributed under the License is distributed on an "AS IS" BASIS, > + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. > + * See the License for the specific language governing permissions and > + * limitations under the License. > + */ > + > +#include > +#include > +#include > + > +#include "coverage.h" > +#include "dp-packet.h" > +#include "dp-packet-gso.h" > +#include "netdev-provider.h" > +#include "openvswitch/vlog.h" > + > +VLOG_DEFINE_THIS_MODULE(dp_packet_gso); > + > +COVERAGE_DEFINE(soft_seg_good); > + > +/* Retuns a new packet that is a segment of packet 'p'. > + * > + * The new packet is initialized with 'hdr_len' bytes from the > + * start of packet 'p' and then appended with 'data_len' bytes > + * from the 'data' buffer. > + * > + * Note: The packet headers are not updated. */ > +static struct dp_packet * > +dp_packet_gso_seg_new(const struct dp_packet *p, size_t hdr_len, > + const char *data, size_t data_len) > +{ > +struct dp_packet *seg = dp_packet_new_with_headroom(hdr_len + data_len, > + > dp_packet_headroom(p)); > + > +/* Append the original packet headers and then the payload. */ > +dp_packet_put(seg, dp_packet_data(p), hdr_len); > +dp_packet_put(seg, data, data_len); > + > +/* The new segment should have the same offsets. */ > +seg->l2_5_ofs = p->l2_5_ofs; > +seg->l3_ofs = p->l3_ofs; > +seg->l4_ofs = p->l4_ofs; > + > +/* The protocol headers remain the same, so preserve hash and mark. */ > +*dp_packet_rss_ptr(seg) = dp_packet_get_rss_hash(p); > +*dp_packet_flow_mark_ptr(seg) = *dp_packet_flow_mark_ptr(p); > + > +/* The segment should inherit all the offloading flags from the > + * original packet, except for the TCP segmentation, external > + * buffer and indirect buffer flags. */ > +*dp_packet_ol_flags_ptr(seg) = *dp_packet_ol_flags_ptr(p) > +& ~(DP_PACKET_OL_TX_TCP_SEG | DP_PACKET_OL_EXTERNAL > +| DP_PACKET_OL_INDIRECT); We should inherit DP_PACKET_OL_SUPPORTED_MASK & ~DP_PACKET_OL_TX_TCP_SEG. So, *dp_packet_ol_flags_ptr(seg) &= DP_PACKET_OL_SUPPORTED_MASK & ~DP_PACKET_OL_TX_TCP_SEG; Is that right? > + > +dp_packet_hwol_reset_tcp_seg(seg); Also, this function just resets the DP_PACKET_OL_TX_TCP_SEG. So, above should be just: *dp_packet_ol_flags_ptr(seg) &= DP_PACKET_OL_SUPPORTED_MASK; The same as in the clone function. Either way one of these operations is redundant. > + > +return seg; > +} > + > +/* Returns the calculated number of TCP segments in packet 'p'. */ > +int > +