On Mon, Aug 8, 2016 at 3:03 AM, Sugesh Chandran
<sugesh.chand...@intel.com> wrote:
> To enable checksum offloading at rx side while adding a port, add the
> 'rx-checksum-offload' option to the 'ovs-vsctl add-port' command-line as
> below,
>
> 'ovs-vsctl add-port br0 dpdk0 -- \
> set Interface dpdk0 type=dpdk options:rx-checksum-offload=true'
Is there a reason not to enable this by default? It seems like it
would be beneficial to more users that way.
I think we should also add some documentation for this new option.
> 2) Normally, OVS generates checksum for tunnel packets. It is more efficient
> to
> calculate the checksum at 'tunnel_push' operation itself as all the relevant
> fields are already present in the cache and also no additional validation
> overhead is involved as in checksum offloading.
I don't quite understand this comment. This is more efficient compared
to what? Presumably, at least for large packets, there would still be
some benefit if the issue with losing vectorization wasn't present.
> diff --git a/lib/dp-packet.h b/lib/dp-packet.h
> index 7c1e637..f2e9d4f 100644
> --- a/lib/dp-packet.h
> +++ b/lib/dp-packet.h
> @@ -38,6 +38,14 @@ enum OVS_PACKED_ENUM dp_packet_source {
>
> #define DP_PACKET_CONTEXT_SIZE 64
>
> +enum hw_pkt_ol_flags {
> + OL_RX_IP_CKSUM_GOOD = (1 << 0),
> + OL_RX_L4_CKSUM_GOOD = (1 << 1),
> + OL_RX_IP_CKSUM_BAD = (1 << 2),
> + OL_RX_L4_CKSUM_BAD = (1 << 3),
Is it necessary to redefine this fields in OVS instead of just using
them from the DPDK mbuf directly? This means that we have to do
mapping and other checks at runtime, which if I remember correctly
resulted in a performance hit when you posted numbers before.
I think we could also simplify things by not checking
NETDEV_RX_CHECKSUM_OFFLOAD for each packet. Presumably if checksum
offloading isn't configured on the interface then it wouldn't set
checksum good on the packet.
> diff --git a/lib/netdev-native-tnl.c b/lib/netdev-native-tnl.c
> index ce2582f..bad2c55 100644
> --- a/lib/netdev-native-tnl.c
> +++ b/lib/netdev-native-tnl.c
> @@ -178,7 +179,10 @@ udp_extract_tnl_md(struct dp_packet *packet, struct
> flow_tnl *tnl,
> return NULL;
> }
>
> - if (udp->udp_csum) {
> + if (packet->md.ol_flags & OL_RX_L4_CKSUM_GOOD) {
> + tnl->flags |= FLOW_TNL_F_CSUM;
> + }
> + else if (udp->udp_csum) {
> uint32_t csum;
> if (netdev_tnl_is_header_ipv6(dp_packet_data(packet))) {
> csum = packet_csum_pseudoheader6(dp_packet_l3(packet));
I think I would put the check for OL_RX_L4_CKSUM_GOOD inside the if
(udp->udp_csum). It's more logical that way and also avoids any
possible problems with drivers that might mistakenly report checksum
of 0 as a validated, correct checksum.
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
