| Message ID | 1580340184-53551-1-git-send-email-u9012063@gmail.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [ovs-dev] userspace: Enable TSO support for non-DPDK. | expand |
Hi. I didn't test or carefully review this. Just a couple of comments inline. Best regards, Ilya Maximets. On 30.01.2020 00:23, William Tu wrote: > For some cases, we want to use userspace datapath but not with > DPDK library, ex: using AF_XDP. AF_XDP is not a good example here because netdev-afxdp is not able to send or receive TSO packets. All such packets will be just dropped. The most sane scenario right now, I think, is to run system tests with TSO enabled to see how different parts of OVS reacts to oversized packets, however we could do that even now since we don't need to actually enable DPDK, we just need to build with it. > The patch enables non-DPDK support > for TSO (TCP Segmentation Offload). I measured the performance > using: > iperf3 -c (ns0) -> veth peer -> OVS -> veth peer -> iperf3 -s (ns1) > And got around 6Gbps, similar to that with DPDK. > > Tested-at: https://travis-ci.org/williamtu/ovs-travis/builds/643599592 > Signed-off-by: William Tu <u9012063@gmail.com> > --- > lib/dp-packet.h | 95 ++++++++++++++++++++++++++++++++--------------------- > lib/userspace-tso.c | 9 ++--- > 2 files changed, 62 insertions(+), 42 deletions(-) > > diff --git a/lib/dp-packet.h b/lib/dp-packet.h > index 69ae5dfac7f0..d0a964b87fb8 100644 > --- a/lib/dp-packet.h > +++ b/lib/dp-packet.h > @@ -53,7 +53,25 @@ enum OVS_PACKED_ENUM dp_packet_source { > enum dp_packet_offload_mask { > DP_PACKET_OL_RSS_HASH_MASK = 0x1, /* Is the 'rss_hash' valid? */ > DP_PACKET_OL_FLOW_MARK_MASK = 0x2, /* Is the 'flow_mark' valid? */ > + DP_PACKET_OL_RX_L4_CKSUM_BAD = 1 << 3, > + DP_PACKET_OL_RX_IP_CKSUM_BAD = 1 << 4, > + DP_PACKET_OL_RX_L4_CKSUM_GOOD = 1 << 5, > + DP_PACKET_OL_RX_IP_CKSUM_GOOD = 1 << 6, > + DP_PACKET_OL_TX_TCP_SEG = 1 << 7, > + DP_PACKET_OL_TX_IPV4 = 1 << 8, > + DP_PACKET_OL_TX_IPV6 = 1 << 9, > + DP_PACKET_OL_TX_TCP_CKSUM = 1 << 10, > + DP_PACKET_OL_TX_UDP_CKSUM = 1 << 11, > + DP_PACKET_OL_TX_SCTP_CKSUM = 1 << 12, > }; > + > +#define DP_PACKET_OL_TX_L4_MASK (DP_PACKET_OL_TX_TCP_CKSUM | \ > + DP_PACKET_OL_TX_UDP_CKSUM | \ > + DP_PACKET_OL_TX_SCTP_CKSUM) > +#define DP_PACKET_OL_RX_IP_CKSUM_MASK (DP_PACKET_OL_RX_IP_CKSUM_GOOD | \ > + DP_PACKET_OL_RX_IP_CKSUM_BAD) > +#define DP_PACKET_OL_RX_L4_CKSUM_MASK (DP_PACKET_OL_RX_L4_CKSUM_GOOD | \ > + DP_PACKET_OL_RX_L4_CKSUM_BAD) > #else > /* DPDK mbuf ol_flags that are not really an offload flags. These are mostly > * related to mbuf memory layout and OVS should not touch/clear them. */ > @@ -737,82 +755,79 @@ dp_packet_set_allocated(struct dp_packet *b, uint16_t s) > b->allocated_ = s; > } > > -/* There are no implementation when not DPDK enabled datapath. */ > static inline bool > -dp_packet_hwol_is_tso(const struct dp_packet *b OVS_UNUSED) > +dp_packet_hwol_is_tso(const struct dp_packet *b) > { > - return false; > + return !!(b->ol_flags & DP_PACKET_OL_TX_TCP_SEG); > } > > -/* There are no implementation when not DPDK enabled datapath. */ > static inline bool > -dp_packet_hwol_is_ipv4(const struct dp_packet *b OVS_UNUSED) > +dp_packet_hwol_is_ipv4(const struct dp_packet *b) > { > - return false; > + return !!(b->ol_flags & DP_PACKET_OL_TX_IPV4); > } > > -/* There are no implementation when not DPDK enabled datapath. */ > static inline uint64_t > -dp_packet_hwol_l4_mask(const struct dp_packet *b OVS_UNUSED) > +dp_packet_hwol_l4_mask(const struct dp_packet *b) > { > - return 0; > + return b->ol_flags & DP_PACKET_OL_TX_L4_MASK; > } > > -/* There are no implementation when not DPDK enabled datapath. */ > static inline bool > -dp_packet_hwol_l4_is_tcp(const struct dp_packet *b OVS_UNUSED) > +dp_packet_hwol_l4_is_tcp(const struct dp_packet *b) > { > - return false; > + return (b->ol_flags & DP_PACKET_OL_TX_L4_MASK) == > + DP_PACKET_OL_TX_TCP_CKSUM; > } > > -/* There are no implementation when not DPDK enabled datapath. */ > static inline bool > -dp_packet_hwol_l4_is_udp(const struct dp_packet *b OVS_UNUSED) > +dp_packet_hwol_l4_is_udp(const struct dp_packet *b) > { > - return false; > + return (b->ol_flags & DP_PACKET_OL_TX_L4_MASK) == > + DP_PACKET_OL_TX_UDP_CKSUM; > } > > -/* There are no implementation when not DPDK enabled datapath. */ > static inline bool > -dp_packet_hwol_l4_is_sctp(const struct dp_packet *b OVS_UNUSED) > +dp_packet_hwol_l4_is_sctp(const struct dp_packet *b) > { > - return false; > + return (b->ol_flags & DP_PACKET_OL_TX_L4_MASK) == > + DP_PACKET_OL_TX_SCTP_CKSUM; > } > > -/* There are no implementation when not DPDK enabled datapath. */ > static inline void > -dp_packet_hwol_set_tx_ipv4(struct dp_packet *b OVS_UNUSED) > +dp_packet_hwol_set_tx_ipv4(struct dp_packet *b) > { > + b->ol_flags |= DP_PACKET_OL_TX_IPV4; > } > > -/* There are no implementation when not DPDK enabled datapath. */ > static inline void > -dp_packet_hwol_set_tx_ipv6(struct dp_packet *b OVS_UNUSED) > +dp_packet_hwol_set_tx_ipv6(struct dp_packet *b) > { > + b->ol_flags |= DP_PACKET_OL_TX_IPV6; > } > > -/* There are no implementation when not DPDK enabled datapath. */ > static inline void > -dp_packet_hwol_set_csum_tcp(struct dp_packet *b OVS_UNUSED) > +dp_packet_hwol_set_csum_tcp(struct dp_packet *b) > { > + b->ol_flags |= DP_PACKET_OL_TX_TCP_CKSUM; > } > > -/* There are no implementation when not DPDK enabled datapath. */ > static inline void > -dp_packet_hwol_set_csum_udp(struct dp_packet *b OVS_UNUSED) > +dp_packet_hwol_set_csum_udp(struct dp_packet *b) > { > + b->ol_flags |= DP_PACKET_OL_TX_UDP_CKSUM; > } > > -/* There are no implementation when not DPDK enabled datapath. */ > static inline void > -dp_packet_hwol_set_csum_sctp(struct dp_packet *b OVS_UNUSED) > +dp_packet_hwol_set_csum_sctp(struct dp_packet *b) > { > + b->ol_flags |= DP_PACKET_OL_TX_SCTP_CKSUM; > } > > -/* There are no implementation when not DPDK enabled datapath. */ > static inline void > -dp_packet_hwol_set_tcp_seg(struct dp_packet *b OVS_UNUSED) > +dp_packet_hwol_set_tcp_seg(struct dp_packet *b) > { > + b->ol_flags |= DP_PACKET_OL_TX_TCP_SEG; > } > > /* Returns the RSS hash of the packet 'p'. Note that the returned value is > @@ -843,27 +858,31 @@ dp_packet_reset_offload(struct dp_packet *p) > } > > static inline bool > -dp_packet_ip_checksum_valid(const struct dp_packet *p OVS_UNUSED) > +dp_packet_ip_checksum_valid(const struct dp_packet *p) > { > - return false; > + return (p->ol_flags & DP_PACKET_OL_RX_IP_CKSUM_MASK) == > + DP_PACKET_OL_RX_IP_CKSUM_GOOD; > } > > static inline bool > -dp_packet_ip_checksum_bad(const struct dp_packet *p OVS_UNUSED) > +dp_packet_ip_checksum_bad(const struct dp_packet *p) > { > - return false; > + return (p->ol_flags & DP_PACKET_OL_RX_IP_CKSUM_MASK) == > + DP_PACKET_OL_RX_IP_CKSUM_BAD; > } > > static inline bool > -dp_packet_l4_checksum_valid(const struct dp_packet *p OVS_UNUSED) > +dp_packet_l4_checksum_valid(const struct dp_packet *p) > { > - return false; > + return (p->ol_flags & DP_PACKET_OL_RX_L4_CKSUM_MASK) == > + DP_PACKET_OL_RX_L4_CKSUM_GOOD; > } > > static inline bool > -dp_packet_l4_checksum_bad(const struct dp_packet *p OVS_UNUSED) > +dp_packet_l4_checksum_bad(const struct dp_packet *p) > { > - return false; > + return (p->ol_flags & DP_PACKET_OL_RX_L4_CKSUM_MASK) == > + DP_PACKET_OL_RX_L4_CKSUM_BAD; > } > > static inline bool > diff --git a/lib/userspace-tso.c b/lib/userspace-tso.c > index 6a4a0149b7f5..a1a5e1de714f 100644 > --- a/lib/userspace-tso.c > +++ b/lib/userspace-tso.c > @@ -35,12 +35,13 @@ userspace_tso_init(const struct smap *ovs_other_config) > > if (ovsthread_once_start(&once)) { > #ifdef DPDK_NETDEV > - VLOG_INFO("Userspace TCP Segmentation Offloading support enabled"); > - userspace_tso = true; > + VLOG_INFO("Userspace TCP Segmentation Offloading support enabled" > + " by DPDK"); > #else > - VLOG_WARN("Userspace TCP Segmentation Offloading can not be enabled" > - "since OVS is built without DPDK support."); > + VLOG_INFO("Userspace TCP Segmentation Offloading support enabled" > + " by native OVS"); > #endif This is very confusing. Just remove the '#ifdef' and drop the '#else' branch. > + userspace_tso = true; > ovsthread_once_done(&once); > } > } >
Hi Ilya, Thanks for your quick response. On Thu, Jan 30, 2020 at 1:14 AM Ilya Maximets <i.maximets@ovn.org> wrote: > > Hi. > I didn't test or carefully review this. Just a couple of comments > inline. > > Best regards, Ilya Maximets. > > On 30.01.2020 00:23, William Tu wrote: > > For some cases, we want to use userspace datapath but not with > > DPDK library, ex: using AF_XDP. > > AF_XDP is not a good example here because netdev-afxdp is not able > to send or receive TSO packets. All such packets will be just dropped. Yes. AF_XDP can not support TSO. > > The most sane scenario right now, I think, is to run system tests > with TSO enabled to see how different parts of OVS reacts to oversized > packets, however we could do that even now since we don't need to > actually enable DPDK, we just need to build with it. That's a great point, I will test it. > > > The patch enables non-DPDK support > > for TSO (TCP Segmentation Offload). I measured the performance > > using: > > iperf3 -c (ns0) -> veth peer -> OVS -> veth peer -> iperf3 -s (ns1) > > And got around 6Gbps, similar to that with DPDK. > > > > Tested-at: https://travis-ci.org/williamtu/ovs-travis/builds/643599592 > > Signed-off-by: William Tu <u9012063@gmail.com> > > --- snip > > static inline bool > > diff --git a/lib/userspace-tso.c b/lib/userspace-tso.c > > index 6a4a0149b7f5..a1a5e1de714f 100644 > > --- a/lib/userspace-tso.c > > +++ b/lib/userspace-tso.c > > @@ -35,12 +35,13 @@ userspace_tso_init(const struct smap *ovs_other_config) > > > > if (ovsthread_once_start(&once)) { > > #ifdef DPDK_NETDEV > > - VLOG_INFO("Userspace TCP Segmentation Offloading support enabled"); > > - userspace_tso = true; > > + VLOG_INFO("Userspace TCP Segmentation Offloading support enabled" > > + " by DPDK"); > > #else > > - VLOG_WARN("Userspace TCP Segmentation Offloading can not be enabled" > > - "since OVS is built without DPDK support."); > > + VLOG_INFO("Userspace TCP Segmentation Offloading support enabled" > > + " by native OVS"); > > #endif > > This is very confusing. Just remove the '#ifdef' and drop the '#else' branch. > OK Thanks William
diff --git a/lib/dp-packet.h b/lib/dp-packet.h index 69ae5dfac7f0..d0a964b87fb8 100644 --- a/lib/dp-packet.h +++ b/lib/dp-packet.h @@ -53,7 +53,25 @@ enum OVS_PACKED_ENUM dp_packet_source { enum dp_packet_offload_mask { DP_PACKET_OL_RSS_HASH_MASK = 0x1, /* Is the 'rss_hash' valid? */ DP_PACKET_OL_FLOW_MARK_MASK = 0x2, /* Is the 'flow_mark' valid? */ + DP_PACKET_OL_RX_L4_CKSUM_BAD = 1 << 3, + DP_PACKET_OL_RX_IP_CKSUM_BAD = 1 << 4, + DP_PACKET_OL_RX_L4_CKSUM_GOOD = 1 << 5, + DP_PACKET_OL_RX_IP_CKSUM_GOOD = 1 << 6, + DP_PACKET_OL_TX_TCP_SEG = 1 << 7, + DP_PACKET_OL_TX_IPV4 = 1 << 8, + DP_PACKET_OL_TX_IPV6 = 1 << 9, + DP_PACKET_OL_TX_TCP_CKSUM = 1 << 10, + DP_PACKET_OL_TX_UDP_CKSUM = 1 << 11, + DP_PACKET_OL_TX_SCTP_CKSUM = 1 << 12, }; + +#define DP_PACKET_OL_TX_L4_MASK (DP_PACKET_OL_TX_TCP_CKSUM | \ + DP_PACKET_OL_TX_UDP_CKSUM | \ + DP_PACKET_OL_TX_SCTP_CKSUM) +#define DP_PACKET_OL_RX_IP_CKSUM_MASK (DP_PACKET_OL_RX_IP_CKSUM_GOOD | \ + DP_PACKET_OL_RX_IP_CKSUM_BAD) +#define DP_PACKET_OL_RX_L4_CKSUM_MASK (DP_PACKET_OL_RX_L4_CKSUM_GOOD | \ + DP_PACKET_OL_RX_L4_CKSUM_BAD) #else /* DPDK mbuf ol_flags that are not really an offload flags. These are mostly * related to mbuf memory layout and OVS should not touch/clear them. */ @@ -737,82 +755,79 @@ dp_packet_set_allocated(struct dp_packet *b, uint16_t s) b->allocated_ = s; } -/* There are no implementation when not DPDK enabled datapath. */ static inline bool -dp_packet_hwol_is_tso(const struct dp_packet *b OVS_UNUSED) +dp_packet_hwol_is_tso(const struct dp_packet *b) { - return false; + return !!(b->ol_flags & DP_PACKET_OL_TX_TCP_SEG); } -/* There are no implementation when not DPDK enabled datapath. */ static inline bool -dp_packet_hwol_is_ipv4(const struct dp_packet *b OVS_UNUSED) +dp_packet_hwol_is_ipv4(const struct dp_packet *b) { - return false; + return !!(b->ol_flags & DP_PACKET_OL_TX_IPV4); } -/* There are no implementation when not DPDK enabled datapath. */ static inline uint64_t -dp_packet_hwol_l4_mask(const struct dp_packet *b OVS_UNUSED) +dp_packet_hwol_l4_mask(const struct dp_packet *b) { - return 0; + return b->ol_flags & DP_PACKET_OL_TX_L4_MASK; } -/* There are no implementation when not DPDK enabled datapath. */ static inline bool -dp_packet_hwol_l4_is_tcp(const struct dp_packet *b OVS_UNUSED) +dp_packet_hwol_l4_is_tcp(const struct dp_packet *b) { - return false; + return (b->ol_flags & DP_PACKET_OL_TX_L4_MASK) == + DP_PACKET_OL_TX_TCP_CKSUM; } -/* There are no implementation when not DPDK enabled datapath. */ static inline bool -dp_packet_hwol_l4_is_udp(const struct dp_packet *b OVS_UNUSED) +dp_packet_hwol_l4_is_udp(const struct dp_packet *b) { - return false; + return (b->ol_flags & DP_PACKET_OL_TX_L4_MASK) == + DP_PACKET_OL_TX_UDP_CKSUM; } -/* There are no implementation when not DPDK enabled datapath. */ static inline bool -dp_packet_hwol_l4_is_sctp(const struct dp_packet *b OVS_UNUSED) +dp_packet_hwol_l4_is_sctp(const struct dp_packet *b) { - return false; + return (b->ol_flags & DP_PACKET_OL_TX_L4_MASK) == + DP_PACKET_OL_TX_SCTP_CKSUM; } -/* There are no implementation when not DPDK enabled datapath. */ static inline void -dp_packet_hwol_set_tx_ipv4(struct dp_packet *b OVS_UNUSED) +dp_packet_hwol_set_tx_ipv4(struct dp_packet *b) { + b->ol_flags |= DP_PACKET_OL_TX_IPV4; } -/* There are no implementation when not DPDK enabled datapath. */ static inline void -dp_packet_hwol_set_tx_ipv6(struct dp_packet *b OVS_UNUSED) +dp_packet_hwol_set_tx_ipv6(struct dp_packet *b) { + b->ol_flags |= DP_PACKET_OL_TX_IPV6; } -/* There are no implementation when not DPDK enabled datapath. */ static inline void -dp_packet_hwol_set_csum_tcp(struct dp_packet *b OVS_UNUSED) +dp_packet_hwol_set_csum_tcp(struct dp_packet *b) { + b->ol_flags |= DP_PACKET_OL_TX_TCP_CKSUM; } -/* There are no implementation when not DPDK enabled datapath. */ static inline void -dp_packet_hwol_set_csum_udp(struct dp_packet *b OVS_UNUSED) +dp_packet_hwol_set_csum_udp(struct dp_packet *b) { + b->ol_flags |= DP_PACKET_OL_TX_UDP_CKSUM; } -/* There are no implementation when not DPDK enabled datapath. */ static inline void -dp_packet_hwol_set_csum_sctp(struct dp_packet *b OVS_UNUSED) +dp_packet_hwol_set_csum_sctp(struct dp_packet *b) { + b->ol_flags |= DP_PACKET_OL_TX_SCTP_CKSUM; } -/* There are no implementation when not DPDK enabled datapath. */ static inline void -dp_packet_hwol_set_tcp_seg(struct dp_packet *b OVS_UNUSED) +dp_packet_hwol_set_tcp_seg(struct dp_packet *b) { + b->ol_flags |= DP_PACKET_OL_TX_TCP_SEG; } /* Returns the RSS hash of the packet 'p'. Note that the returned value is @@ -843,27 +858,31 @@ dp_packet_reset_offload(struct dp_packet *p) } static inline bool -dp_packet_ip_checksum_valid(const struct dp_packet *p OVS_UNUSED) +dp_packet_ip_checksum_valid(const struct dp_packet *p) { - return false; + return (p->ol_flags & DP_PACKET_OL_RX_IP_CKSUM_MASK) == + DP_PACKET_OL_RX_IP_CKSUM_GOOD; } static inline bool -dp_packet_ip_checksum_bad(const struct dp_packet *p OVS_UNUSED) +dp_packet_ip_checksum_bad(const struct dp_packet *p) { - return false; + return (p->ol_flags & DP_PACKET_OL_RX_IP_CKSUM_MASK) == + DP_PACKET_OL_RX_IP_CKSUM_BAD; } static inline bool -dp_packet_l4_checksum_valid(const struct dp_packet *p OVS_UNUSED) +dp_packet_l4_checksum_valid(const struct dp_packet *p) { - return false; + return (p->ol_flags & DP_PACKET_OL_RX_L4_CKSUM_MASK) == + DP_PACKET_OL_RX_L4_CKSUM_GOOD; } static inline bool -dp_packet_l4_checksum_bad(const struct dp_packet *p OVS_UNUSED) +dp_packet_l4_checksum_bad(const struct dp_packet *p) { - return false; + return (p->ol_flags & DP_PACKET_OL_RX_L4_CKSUM_MASK) == + DP_PACKET_OL_RX_L4_CKSUM_BAD; } static inline bool diff --git a/lib/userspace-tso.c b/lib/userspace-tso.c index 6a4a0149b7f5..a1a5e1de714f 100644 --- a/lib/userspace-tso.c +++ b/lib/userspace-tso.c @@ -35,12 +35,13 @@ userspace_tso_init(const struct smap *ovs_other_config) if (ovsthread_once_start(&once)) { #ifdef DPDK_NETDEV - VLOG_INFO("Userspace TCP Segmentation Offloading support enabled"); - userspace_tso = true; + VLOG_INFO("Userspace TCP Segmentation Offloading support enabled" + " by DPDK"); #else - VLOG_WARN("Userspace TCP Segmentation Offloading can not be enabled" - "since OVS is built without DPDK support."); + VLOG_INFO("Userspace TCP Segmentation Offloading support enabled" + " by native OVS"); #endif + userspace_tso = true; ovsthread_once_done(&once); } }
For some cases, we want to use userspace datapath but not with DPDK library, ex: using AF_XDP. The patch enables non-DPDK support for TSO (TCP Segmentation Offload). I measured the performance using: iperf3 -c (ns0) -> veth peer -> OVS -> veth peer -> iperf3 -s (ns1) And got around 6Gbps, similar to that with DPDK. Tested-at: https://travis-ci.org/williamtu/ovs-travis/builds/643599592 Signed-off-by: William Tu <u9012063@gmail.com> --- lib/dp-packet.h | 95 ++++++++++++++++++++++++++++++++--------------------- lib/userspace-tso.c | 9 ++--- 2 files changed, 62 insertions(+), 42 deletions(-)