[ovs-dev] [PATCH] odp-execute: Optimize IP header modification in OVS datapath

[Zoltán Balogh]

Hi,

I measured the packet processing cost of OVS DPDK datapath for different 
OpenFlow actions. I configured OVS to use a single pmd thread and measured the 
packet throughput in a phy-to-phy setup. I used 10G interfaces bounded to DPDK 
driver and overloaded the vSwitch with 64 byte packets through one of the 10G 
interfaces.

The processing cost of the dec_ttl action seemed to be gratuitously high 
compared with other actions.

I looked into the code and saw that dec_ttl is encoded as a masked nested 
attribute in OVS_ACTION_ATTR_SET_MASKED(OVS_KEY_ATTR_IPV4). That way, OVS 
datapath can modify several IP header fields (TTL, TOS, source and destination 
IP addresses) by a single invocation of packet_set_ipv4() in the odp_set_ipv4() 
function in the lib/odp-execute.c file.
The packet_set_ipv4() function takes the new TOS, TTL and IP addresses as 
arguments, compares them with the actual ones and updates the fields if needed.
This means, that even if only TTL needs to be updated, each of the four IP 
header fields is passed to the callee and is compared to the actual field for 
each packet.

The odp_set_ipv4() caller function possesses information about the fields that 
need to be updated in the 'mask' structure. The idea is to spare invocation of 
thepacket_set_ipv4() function but use its code parts directly. So the 'mask' 
can be used to decide which IP header fields need to be updated. In addition, 
a faster packet processing can be achieved if the values of local variables are 
calculated right before their usage.

These are the results of my measurements. 
        
       | T | T | I | I |  
       | T | O | P | P |  Vanilla OVS  |  OVS + patch
       | L | S | s | d | (nsec/packet) | (nsec/packet)
-------+---+---+---+---+---------------+---------------
output |   |   |   |   |    67.20      |    67.20 
       | X |   |   |   |    74.34      |    70.03
       |   | X |   |   |    74.20      |    71.06
       |   |   | X |   |    83.14      |    78.67
       |   |   |   | X |    83.27      |    78.00
       |   |   | X | X |    96.68      |    91.81
       | X |   | X | X |    99.58      |    96.00
       | X | X | X | X |   103.31      |   100.73

The table shows the average processing cost of packets in nanoseconds for the 
following actions: 
output; output + dec_ttl; output + mod_nw_tos; output + mod_nw_src; 
output + mod_nw_dst and some of their combinations.

I added OVS_LIKELY to the 'if' condition for the TTL field, since as far as I 
know, this field will typically be decremented when any field of the IP header 
is modified.

Please find the patch below.

Signed-off-by: Zoltán Balogh <zoltan.balogh at ericsson.com>

---

diff --git a/lib/odp-execute.c b/lib/odp-execute.c 
index 65a6fcd..131532d 100644
--- a/lib/odp-execute.c
+++ b/lib/odp-execute.c
@@ -33,6 +33,7 @@
 #include "flow.h"
 #include "unaligned.h"
 #include "util.h"
+#include "csum.h"

 /* Masked copy of an ethernet address. 'src' is already properly masked. */  
static void 
@@ -68,13 +69,42 @@ odp_set_ipv4(struct dp_packet *packet, const struct ovs_key_ipv4 *key,
              const struct ovs_key_ipv4 *mask)  
{
     struct ip_header *nh = dp_packet_l3(packet);
+    ovs_be32 ip_src_nh;
+    ovs_be32 ip_dst_nh;
+    ovs_be32 new_ip_src;
+    ovs_be32 new_ip_dst;
+    uint8_t new_tos;
+    uint8_t new_ttl;
+
+    ip_src_nh = get_16aligned_be32(&nh->ip_src);
+    new_ip_src = key->ipv4_src | (ip_src_nh & ~mask->ipv4_src);
+
+    if (mask->ipv4_src && ip_src_nh != new_ip_src) {
+        packet_set_ipv4_addr(packet, &nh->ip_src, new_ip_src);
+    }

-    packet_set_ipv4(
-        packet,
-        key->ipv4_src | (get_16aligned_be32(&nh->ip_src) & ~mask->ipv4_src),
-        key->ipv4_dst | (get_16aligned_be32(&nh->ip_dst) & ~mask->ipv4_dst),
-        key->ipv4_tos | (nh->ip_tos & ~mask->ipv4_tos),
-        key->ipv4_ttl | (nh->ip_ttl & ~mask->ipv4_ttl));
+    ip_dst_nh = get_16aligned_be32(&nh->ip_dst);
+    new_ip_dst = key->ipv4_dst | (ip_dst_nh & ~mask->ipv4_dst);
+
+    if (mask->ipv4_dst && ip_dst_nh != new_ip_dst) {
+        packet_set_ipv4_addr(packet, &nh->ip_dst, new_ip_dst);
+    }
+
+    new_tos = key->ipv4_tos | (nh->ip_tos & ~mask->ipv4_tos);
+
+    if (mask->ipv4_tos && nh->ip_tos != new_tos) {
+        nh->ip_csum = recalc_csum16(nh->ip_csum, htons((uint16_t) nh->ip_tos),
+                                    htons((uint16_t) new_tos));
+        nh->ip_tos = new_tos;
+    }
+
+    new_ttl = key->ipv4_ttl | (nh->ip_ttl & ~mask->ipv4_ttl);
+
+    if (OVS_LIKELY(mask->ipv4_ttl && nh->ip_ttl != new_ttl)) {
+        nh->ip_csum = recalc_csum16(nh->ip_csum, htons(nh->ip_ttl << 8),
+                                    htons(new_ttl << 8));
+        nh->ip_ttl = new_ttl;
+    }
 }

 static const ovs_be32 *
diff --git a/lib/packets.c b/lib/packets.c 
index 990c407..179841a 100644
--- a/lib/packets.c
+++ b/lib/packets.c
@@ -849,7 +849,7 @@ eth_compose(struct dp_packet *b, const struct eth_addr eth_dst,
     return data;
 }

-static void
+void
 packet_set_ipv4_addr(struct dp_packet *packet,
                      ovs_16aligned_be32 *addr, ovs_be32 new_addr) 
 { 
diff --git a/lib/packets.h b/lib/packets.h 
index 21bd35c..97d1b38 100644
--- a/lib/packets.h
+++ b/lib/packets.h
@@ -1093,6 +1093,8 @@ void *snap_compose(struct dp_packet *, const struct eth_addr eth_dst,
                    unsigned int oui, uint16_t snap_type, size_t size);  
void packet_set_ipv4(struct dp_packet *, ovs_be32 src, ovs_be32 dst, uint8_t tos,
                      uint8_t ttl);
+void packet_set_ipv4_addr(struct dp_packet *packet, ovs_16aligned_be32 *addr,
+                          ovs_be32 new_addr);
 void packet_set_ipv6(struct dp_packet *, const ovs_be32 src[4],
                      const ovs_be32 dst[4], uint8_t tc,
                      ovs_be32 fl, uint8_t hlmit);