[ovs-dev] [PATCH v2] datapath-windows: Update flow key in SET action

Jinjun Gao jinjung at vmware.com
Wed Jul 29 03:33:18 UTC 2020


The flow key is not updated when process OVS_ACTION_ATTR_SET action.
It will impact follow-up actions, such as, conntrack module cannot
find created conntrack entry if passing old flow key to it.

Reported-by: Rui Cao <rcao at vmware.com>
Signed-off-by: Jinjun Gao <jinjung at vmware.com>
---
v2: Separated assignment to happy MSFT SDV tool
---
 datapath-windows/ovsext/Actions.c | 31 ++++++++++++++++++++++++-------
 datapath-windows/ovsext/Actions.h |  3 +++
 2 files changed, 27 insertions(+), 7 deletions(-)

diff --git a/datapath-windows/ovsext/Actions.c b/datapath-windows/ovsext/Actions.c
index 4a11cea..4f43369 100644
--- a/datapath-windows/ovsext/Actions.c
+++ b/datapath-windows/ovsext/Actions.c
@@ -1259,6 +1259,7 @@ OvsActionMplsPush(OvsForwardingContext *ovsFwdCtx,
  */
 static __inline NDIS_STATUS
 OvsUpdateEthHeader(OvsForwardingContext *ovsFwdCtx,
+                   OvsFlowKey *key,
                    const struct ovs_key_ethernet *ethAttr)
 {
     PNET_BUFFER curNb;
@@ -1285,9 +1286,11 @@ OvsUpdateEthHeader(OvsForwardingContext *ovsFwdCtx,
     }
     ethHdr = (EthHdr *)(bufferStart + NET_BUFFER_CURRENT_MDL_OFFSET(curNb));

-    RtlCopyMemory(ethHdr->Destination, ethAttr->eth_dst,
-                   sizeof ethHdr->Destination);
-    RtlCopyMemory(ethHdr->Source, ethAttr->eth_src, sizeof ethHdr->Source);
+    RtlCopyMemory(ethHdr->Destination, ethAttr->eth_dst, ETH_ADDR_LENGTH);
+    RtlCopyMemory(ethHdr->Source, ethAttr->eth_src, ETH_ADDR_LENGTH);
+    /* Update l2 flow key */
+    RtlCopyMemory(key->l2.dlDst, ethAttr->eth_dst, ETH_ADDR_LENGTH);
+    RtlCopyMemory(key->l2.dlSrc, ethAttr->eth_src, ETH_ADDR_LENGTH);

     return NDIS_STATUS_SUCCESS;
 }
@@ -1376,6 +1379,7 @@ PUINT8 OvsGetHeaderBySize(OvsForwardingContext *ovsFwdCtx,
  */
 NDIS_STATUS
 OvsUpdateUdpPorts(OvsForwardingContext *ovsFwdCtx,
+                  OvsFlowKey *key,
                   const struct ovs_key_udp *udpAttr)
 {
     PUINT8 bufferStart;
@@ -1400,15 +1404,19 @@ OvsUpdateUdpPorts(OvsForwardingContext *ovsFwdCtx,
             udpHdr->check = ChecksumUpdate16(udpHdr->check, udpHdr->source,
                                              udpAttr->udp_src);
             udpHdr->source = udpAttr->udp_src;
+            key->ipKey.l4.tpSrc = udpAttr->udp_src;
         }
         if (udpHdr->dest != udpAttr->udp_dst) {
             udpHdr->check = ChecksumUpdate16(udpHdr->check, udpHdr->dest,
                                              udpAttr->udp_dst);
             udpHdr->dest = udpAttr->udp_dst;
+            key->ipKey.l4.tpDst = udpAttr->udp_dst;
         }
     } else {
         udpHdr->source = udpAttr->udp_src;
+        key->ipKey.l4.tpSrc = udpAttr->udp_src;
         udpHdr->dest = udpAttr->udp_dst;
+        key->ipKey.l4.tpDst = udpAttr->udp_dst;
     }

     return NDIS_STATUS_SUCCESS;
@@ -1423,6 +1431,7 @@ OvsUpdateUdpPorts(OvsForwardingContext *ovsFwdCtx,
  */
 NDIS_STATUS
 OvsUpdateTcpPorts(OvsForwardingContext *ovsFwdCtx,
+                  OvsFlowKey *key,
                   const struct ovs_key_tcp *tcpAttr)
 {
     PUINT8 bufferStart;
@@ -1447,11 +1456,13 @@ OvsUpdateTcpPorts(OvsForwardingContext *ovsFwdCtx,
         tcpHdr->check = ChecksumUpdate16(tcpHdr->check, tcpHdr->source,
                                          tcpAttr->tcp_src);
         tcpHdr->source = tcpAttr->tcp_src;
+        key->ipKey.l4.tpSrc = tcpAttr->tcp_src;
     }
     if (tcpHdr->dest != tcpAttr->tcp_dst) {
         tcpHdr->check = ChecksumUpdate16(tcpHdr->check, tcpHdr->dest,
                                          tcpAttr->tcp_dst);
         tcpHdr->dest = tcpAttr->tcp_dst;
+        key->ipKey.l4.tpDst = tcpAttr->tcp_dst;
     }

     return NDIS_STATUS_SUCCESS;
@@ -1579,6 +1590,7 @@ OvsUpdateAddressAndPort(OvsForwardingContext *ovsFwdCtx,
  */
 NDIS_STATUS
 OvsUpdateIPv4Header(OvsForwardingContext *ovsFwdCtx,
+                    OvsFlowKey *key,
                     const struct ovs_key_ipv4 *ipAttr)
 {
     PUINT8 bufferStart;
@@ -1632,6 +1644,7 @@ OvsUpdateIPv4Header(OvsForwardingContext *ovsFwdCtx,
                                             ipAttr->ipv4_src);
         }
         ipHdr->saddr = ipAttr->ipv4_src;
+        key->ipKey.nwSrc = ipAttr->ipv4_src;
     }
     if (ipHdr->daddr != ipAttr->ipv4_dst) {
         if (tcpHdr) {
@@ -1647,6 +1660,7 @@ OvsUpdateIPv4Header(OvsForwardingContext *ovsFwdCtx,
                                             ipAttr->ipv4_dst);
         }
         ipHdr->daddr = ipAttr->ipv4_dst;
+        key->ipKey.nwDst = ipAttr->ipv4_dst;
     }
     if (ipHdr->protocol != ipAttr->ipv4_proto) {
         UINT16 oldProto = (ipHdr->protocol << 16) & 0xff00;
@@ -1661,6 +1675,7 @@ OvsUpdateIPv4Header(OvsForwardingContext *ovsFwdCtx,
             ipHdr->check = ChecksumUpdate16(ipHdr->check, oldProto, newProto);
         }
         ipHdr->protocol = ipAttr->ipv4_proto;
+        key->ipKey.nwProto = ipAttr->ipv4_proto;
     }
     if (ipHdr->ttl != ipAttr->ipv4_ttl) {
         UINT16 oldTtl = (ipHdr->ttl) & 0xff;
@@ -1669,6 +1684,7 @@ OvsUpdateIPv4Header(OvsForwardingContext *ovsFwdCtx,
             ipHdr->check = ChecksumUpdate16(ipHdr->check, oldTtl, newTtl);
         }
         ipHdr->ttl = ipAttr->ipv4_ttl;
+        key->ipKey.nwTtl = ipAttr->ipv4_ttl;
     }

     return NDIS_STATUS_SUCCESS;
@@ -1691,12 +1707,12 @@ OvsExecuteSetAction(OvsForwardingContext *ovsFwdCtx,

     switch (type) {
     case OVS_KEY_ATTR_ETHERNET:
-        status = OvsUpdateEthHeader(ovsFwdCtx,
+        status = OvsUpdateEthHeader(ovsFwdCtx, key,
             NlAttrGetUnspec(a, sizeof(struct ovs_key_ethernet)));
         break;

     case OVS_KEY_ATTR_IPV4:
-        status = OvsUpdateIPv4Header(ovsFwdCtx,
+        status = OvsUpdateIPv4Header(ovsFwdCtx, key,
             NlAttrGetUnspec(a, sizeof(struct ovs_key_ipv4)));
         break;

@@ -1709,16 +1725,17 @@ OvsExecuteSetAction(OvsForwardingContext *ovsFwdCtx,
         status = SUCCEEDED(convertStatus) ? NDIS_STATUS_SUCCESS : NDIS_STATUS_FAILURE;
         ASSERT(status == NDIS_STATUS_SUCCESS);
         RtlCopyMemory(&ovsFwdCtx->tunKey, &tunKey, sizeof ovsFwdCtx->tunKey);
+        RtlCopyMemory(&key->tunKey, &tunKey, sizeof key->tunKey);
         break;
     }

     case OVS_KEY_ATTR_UDP:
-        status = OvsUpdateUdpPorts(ovsFwdCtx,
+        status = OvsUpdateUdpPorts(ovsFwdCtx, key,
             NlAttrGetUnspec(a, sizeof(struct ovs_key_udp)));
         break;

     case OVS_KEY_ATTR_TCP:
-        status = OvsUpdateTcpPorts(ovsFwdCtx,
+        status = OvsUpdateTcpPorts(ovsFwdCtx, key,
             NlAttrGetUnspec(a, sizeof(struct ovs_key_tcp)));
         break;

diff --git a/datapath-windows/ovsext/Actions.h b/datapath-windows/ovsext/Actions.h
index fd050d5..bc12e11 100644
--- a/datapath-windows/ovsext/Actions.h
+++ b/datapath-windows/ovsext/Actions.h
@@ -115,14 +115,17 @@ PUINT8 OvsGetHeaderBySize(OvsForwardingContext *ovsFwdCtx,

 NDIS_STATUS
 OvsUpdateUdpPorts(OvsForwardingContext *ovsFwdCtx,
+                  OvsFlowKey *key,
                   const struct ovs_key_udp *udpAttr);

 NDIS_STATUS
 OvsUpdateTcpPorts(OvsForwardingContext *ovsFwdCtx,
+                  OvsFlowKey *key,
                   const struct ovs_key_tcp *tcpAttr);

 NDIS_STATUS
 OvsUpdateIPv4Header(OvsForwardingContext *ovsFwdCtx,
+                    OvsFlowKey *key,
                     const struct ovs_key_ipv4 *ipAttr);

 NDIS_STATUS
--
2.7.4



More information about the dev mailing list