[ovs-dev] [PATCH v2 2/5] datapath-windows: Added Ipv4 fragments support in Conntrack
Anand Kumar
kumaranand at vmware.com
Fri Jan 27 22:21:16 UTC 2017
Hi Sairam,
Thank you for reviewing the patch series. I have address the comments and sent out a new version of the patch.
Regards,
Anand Kumar
On 1/23/17, 3:42 PM, "Sairam Venugopal" <vsairam at vmware.com> wrote:
Please find the comments inline. I have prefixed them with “sai:”
Thanks,
Sairam
On 1/12/17, 1:13 PM, "ovs-dev-bounces at openvswitch.org on behalf of Anand Kumar" <ovs-dev-bounces at openvswitch.org on behalf of kumaranand at vmware.com> wrote:
>This patch adds support for Ipv4 fragments in conntrack module.
>Individual fragments are not tracked, the reassembled Ipv4 datagram is treated
>as a single ct entry.
>
>Added MRU field in OvsForwardingContext, to keep track of Maximum recieved unit
>from all the recieved IPv4 fragments.
>---
> datapath-windows/ovsext/Actions.c | 15 +++++++++++----
> datapath-windows/ovsext/Conntrack.c | 31 +++++++++++++++++++++++++------
> datapath-windows/ovsext/Conntrack.h | 7 ++++++-
> 3 files changed, 42 insertions(+), 11 deletions(-)
>
>diff --git a/datapath-windows/ovsext/Actions.c b/datapath-windows/ovsext/Actions.c
>index b4a286b..f378619 100644
>--- a/datapath-windows/ovsext/Actions.c
>+++ b/datapath-windows/ovsext/Actions.c
>@@ -125,6 +125,9 @@ typedef struct OvsForwardingContext {
>
> /* header information */
> OVS_PACKET_HDR_INFO layers;
>+
>+ /* Maximum Recieving Unit */
>+ UINT16 mru;
> } OvsForwardingContext;
>
> /*
>@@ -1910,11 +1913,15 @@ OvsDoExecuteActions(POVS_SWITCH_CONTEXT switchContext,
> }
> }
>
>- status = OvsExecuteConntrackAction(ovsFwdCtx.curNbl, layers,
>- key, (const PNL_ATTR)a);
>+ status = OvsExecuteConntrackAction(switchContext, &ovsFwdCtx.curNbl, &(ovsFwdCtx.mru),
>+ ovsFwdCtx.completionList,
>+ ovsFwdCtx.fwdDetail->SourcePortId,
>+ layers, key, (const PNL_ATTR)a);
> if (status != NDIS_STATUS_SUCCESS) {
>- OVS_LOG_ERROR("CT Action failed");
>- dropReason = L"OVS-conntrack action failed";
>+ if (status != NDIS_STATUS_PENDING) {
>+ OVS_LOG_ERROR("CT Action failed");
>+ dropReason = L"OVS-conntrack action failed";
saii: Can you align the }. Also add a comment saying Pending Packets are currently consumed by the Defragmentation process.
>+ }
> goto dropit;
> }
> break;
>diff --git a/datapath-windows/ovsext/Conntrack.c b/datapath-windows/ovsext/Conntrack.c
>index d1be480..219680e 100644
>--- a/datapath-windows/ovsext/Conntrack.c
>+++ b/datapath-windows/ovsext/Conntrack.c
>@@ -15,6 +15,7 @@
> */
>
> #include "Conntrack.h"
>+#include "IpFragment.h"
> #include "Jhash.h"
> #include "PacketParser.h"
> #include "Event.h"
>@@ -312,13 +313,25 @@ OvsCtEntryExpired(POVS_CT_ENTRY entry)
> }
>
> static __inline NDIS_STATUS
>-OvsDetectCtPacket(OvsFlowKey *key)
>+OvsDetectCtPacket(POVS_SWITCH_CONTEXT switchContext,
>+ PNET_BUFFER_LIST *curNbl,
>+ OvsCompletionList *completionList,
>+ NDIS_SWITCH_PORT_ID sourcePort,
>+ OvsFlowKey *key,
>+ UINT16 *mru,
>+ PNET_BUFFER_LIST *newNbl)
> {
> /* Currently we support only Unfragmented TCP packets */
> switch (ntohs(key->l2.dlType)) {
> case ETH_TYPE_IPV4:
> if (key->ipKey.nwFrag != OVS_FRAG_TYPE_NONE) {
>- return NDIS_STATUS_NOT_SUPPORTED;
>+ return OvsProcessIpv4Fragment(switchContext,
>+ curNbl,
>+ completionList,
>+ sourcePort,
>+ mru,
>+ key->tunKey.tunnelId,
>+ newNbl);
> }
> if (key->ipKey.nwProto == IPPROTO_TCP
> || key->ipKey.nwProto == IPPROTO_UDP
>@@ -688,7 +701,11 @@ OvsCtExecute_(PNET_BUFFER_LIST curNbl,
> *---------------------------------------------------------------------------
> */
Sai: Update the summary to say we consume the original fragment Nbl.
> NDIS_STATUS
>-OvsExecuteConntrackAction(PNET_BUFFER_LIST curNbl,
>+OvsExecuteConntrackAction(POVS_SWITCH_CONTEXT switchContext,
>+ PNET_BUFFER_LIST *curNbl,
>+ UINT16 *mru,
>+ OvsCompletionList *completionList,
>+ NDIS_SWITCH_PORT_ID sourcePort,
> OVS_PACKET_HDR_INFO *layers,
> OvsFlowKey *key,
> const PNL_ATTR a)
>@@ -699,10 +716,11 @@ OvsExecuteConntrackAction(PNET_BUFFER_LIST curNbl,
> MD_MARK *mark = NULL;
> MD_LABELS *labels = NULL;
> PCHAR helper = NULL;
>-
>+ PNET_BUFFER_LIST newNbl = NULL;
> NDIS_STATUS status;
>
>- status = OvsDetectCtPacket(key);
>+ status = OvsDetectCtPacket(switchContext, curNbl, completionList,
>+ sourcePort, key, mru, &newNbl);
> if (status != NDIS_STATUS_SUCCESS) {
> return status;
> }
>@@ -735,7 +753,8 @@ OvsExecuteConntrackAction(PNET_BUFFER_LIST curNbl,
> }
> }
>
>- status = OvsCtExecute_(curNbl, key, layers,
>+ /* If newNbl is not allocated, use the current Nbl*/
>+ status = OvsCtExecute_(newNbl != NULL ? newNbl : *curNbl, key, layers,
> commit, zone, mark, labels, helper);
> return status;
> }
>diff --git a/datapath-windows/ovsext/Conntrack.h b/datapath-windows/ovsext/Conntrack.h
>index af99885..2e7285b 100644
>--- a/datapath-windows/ovsext/Conntrack.h
>+++ b/datapath-windows/ovsext/Conntrack.h
>@@ -20,6 +20,7 @@
> #include "precomp.h"
> #include "Flow.h"
> #include "Debug.h"
>+#include "PacketIO.h"
> #include <stddef.h>
>
> #ifdef OVS_DBG_MOD
>@@ -155,7 +156,11 @@ OvsGetTcpPayloadLength(PNET_BUFFER_LIST nbl)
> VOID OvsCleanupConntrack(VOID);
> NTSTATUS OvsInitConntrack(POVS_SWITCH_CONTEXT context);
>
>-NDIS_STATUS OvsExecuteConntrackAction(PNET_BUFFER_LIST curNbl,
>+NDIS_STATUS OvsExecuteConntrackAction(POVS_SWITCH_CONTEXT switchContext,
>+ PNET_BUFFER_LIST *curNbl,
>+ UINT16 *mru,
>+ OvsCompletionList *completionList,
>+ NDIS_SWITCH_PORT_ID sourcePort,
> OVS_PACKET_HDR_INFO *layers,
> OvsFlowKey *key,
> const PNL_ATTR a);
>--
>2.9.3.windows.1
>
>_______________________________________________
>dev mailing list
>dev at openvswitch.org
>https://urldefense.proofpoint.com/v2/url?u=https-3A__mail.openvswitch.org_mailman_listinfo_ovs-2Ddev&d=DwICAg&c=uilaK90D4TOVoH58JNXRgQ&r=Z6vowHUOjP5ysP_g372c49Nqc1vEKqHKNBkR5Q5Z7uo&m=Bwari9A-81G4aEK9k3Bi4VbSlFU3dL30d7Cic3AIWWw&s=bgbBOY2sY7t7DSiL88w6St8aWmdHGL7GumOBtY1f9XU&e=
More information about the dev
mailing list