Looks good, just one small nit: since `OvsForwardingContext` is public now could you specify it in the commit message?
Acked-by: Alin Gabriel Serdean <[email protected]> > -----Original Message----- > From: [email protected] [mailto:ovs-dev- > [email protected]] On Behalf Of Yin Lin > Sent: Tuesday, April 11, 2017 1:45 AM > To: [email protected] > Subject: [ovs-dev] [PATCH 1/4 v3] datapath-windows: Pass fwdCtx to > conntrack > > --- > datapath-windows/ovsext/Actions.c | 61 > ++----------------------------------- > datapath-windows/ovsext/Actions.h | 58 > +++++++++++++++++++++++++++++++++++ > datapath-windows/ovsext/Conntrack.c | 5 +-- datapath- > windows/ovsext/Conntrack.h | 4 +-- > 4 files changed, 65 insertions(+), 63 deletions(-) > > diff --git a/datapath-windows/ovsext/Actions.c b/datapath- > windows/ovsext/Actions.c > index 46f84bc..3bd00a7 100644 > --- a/datapath-windows/ovsext/Actions.c > +++ b/datapath-windows/ovsext/Actions.c > @@ -71,63 +71,6 @@ typedef struct _OVS_ACTION_STATS { > OVS_ACTION_STATS ovsActionStats; > > /* > - * There a lot of data that needs to be maintained while executing the > pipeline > - * as dictated by the actions of a flow, across different functions at > different > - * levels. Such data is put together in a 'context' structure. Care should be > - * exercised while adding new members to the structure - only add ones > that get > - * used across multiple stages in the pipeline/get used in multiple > functions. > - */ > -typedef struct OvsForwardingContext { > - POVS_SWITCH_CONTEXT switchContext; > - /* The NBL currently used in the pipeline. */ > - PNET_BUFFER_LIST curNbl; > - /* NDIS forwarding detail for 'curNbl'. */ > - PNDIS_SWITCH_FORWARDING_DETAIL_NET_BUFFER_LIST_INFO > fwdDetail; > - /* Array of destination ports for 'curNbl'. */ > - PNDIS_SWITCH_FORWARDING_DESTINATION_ARRAY destinationPorts; > - /* send flags while sending 'curNbl' into NDIS. */ > - ULONG sendFlags; > - /* Total number of output ports, used + unused, in 'curNbl'. */ > - UINT32 destPortsSizeIn; > - /* Total number of used output ports in 'curNbl'. */ > - UINT32 destPortsSizeOut; > - /* > - * If 'curNbl' is not owned by OVS, they need to be tracked, if they > need to > - * be freed/completed. > - */ > - OvsCompletionList *completionList; > - /* > - * vport number of 'curNbl' when it is passed from the PIF bridge to the > INT > - * bridge. ie. during tunneling on the Rx side. > - */ > - UINT32 srcVportNo; > - > - /* > - * Tunnel key: > - * - specified in actions during tunneling Tx > - * - extracted from an NBL during tunneling Rx > - */ > - OvsIPv4TunnelKey tunKey; > - > - /* > - * Tunneling - Tx: > - * To store the output port, when it is a tunneled port. We don't foresee > - * multiple tunneled ports as outport for any given NBL. > - */ > - POVS_VPORT_ENTRY tunnelTxNic; > - > - /* > - * Tunneling - Rx: > - * Points to the Internal port on the PIF Bridge, if the packet needs to > be > - * de-tunneled. > - */ > - POVS_VPORT_ENTRY tunnelRxNic; > - > - /* header information */ > - OVS_PACKET_HDR_INFO layers; > -} OvsForwardingContext; > - > -/* > * -------------------------------------------------------------------------- > * OvsInitForwardingCtx -- > * Function to init/re-init the 'ovsFwdCtx' context as the actions > pipeline > @@ -2032,8 +1975,8 @@ OvsDoExecuteActions(POVS_SWITCH_CONTEXT > switchContext, > } > } > > - status = OvsExecuteConntrackAction(ovsFwdCtx.curNbl, layers, > - key, (const PNL_ATTR)a); > + status = OvsExecuteConntrackAction(&ovsFwdCtx, key, > + (const PNL_ATTR)a); > if (status != NDIS_STATUS_SUCCESS) { > OVS_LOG_ERROR("CT Action failed"); > dropReason = L"OVS-conntrack action failed"; diff --git > a/datapath- > windows/ovsext/Actions.h b/datapath-windows/ovsext/Actions.h > index c56c260..1ce6c20 100644 > --- a/datapath-windows/ovsext/Actions.h > +++ b/datapath-windows/ovsext/Actions.h > @@ -20,6 +20,64 @@ > #include "Switch.h" > #include "PacketIO.h" > > + > +/* > + * There a lot of data that needs to be maintained while executing the > +pipeline > + * as dictated by the actions of a flow, across different functions at > +different > + * levels. Such data is put together in a 'context' structure. Care > +should be > + * exercised while adding new members to the structure - only add ones > +that get > + * used across multiple stages in the pipeline/get used in multiple > functions. > + */ > +typedef struct OvsForwardingContext { > + POVS_SWITCH_CONTEXT switchContext; > + /* The NBL currently used in the pipeline. */ > + PNET_BUFFER_LIST curNbl; > + /* NDIS forwarding detail for 'curNbl'. */ > + PNDIS_SWITCH_FORWARDING_DETAIL_NET_BUFFER_LIST_INFO > fwdDetail; > + /* Array of destination ports for 'curNbl'. */ > + PNDIS_SWITCH_FORWARDING_DESTINATION_ARRAY destinationPorts; > + /* send flags while sending 'curNbl' into NDIS. */ > + ULONG sendFlags; > + /* Total number of output ports, used + unused, in 'curNbl'. */ > + UINT32 destPortsSizeIn; > + /* Total number of used output ports in 'curNbl'. */ > + UINT32 destPortsSizeOut; > + /* > + * If 'curNbl' is not owned by OVS, they need to be tracked, if they need > to > + * be freed/completed. > + */ > + OvsCompletionList *completionList; > + /* > + * vport number of 'curNbl' when it is passed from the PIF bridge to the > INT > + * bridge. ie. during tunneling on the Rx side. > + */ > + UINT32 srcVportNo; > + > + /* > + * Tunnel key: > + * - specified in actions during tunneling Tx > + * - extracted from an NBL during tunneling Rx > + */ > + OvsIPv4TunnelKey tunKey; > + > + /* > + * Tunneling - Tx: > + * To store the output port, when it is a tunneled port. We don't foresee > + * multiple tunneled ports as outport for any given NBL. > + */ > + POVS_VPORT_ENTRY tunnelTxNic; > + > + /* > + * Tunneling - Rx: > + * Points to the Internal port on the PIF Bridge, if the packet needs to > be > + * de-tunneled. > + */ > + POVS_VPORT_ENTRY tunnelRxNic; > + > + /* header information */ > + OVS_PACKET_HDR_INFO layers; > +} OvsForwardingContext; > + > NDIS_STATUS > OvsActionsExecute(POVS_SWITCH_CONTEXT switchContext, > OvsCompletionList *completionList, diff --git a/datapath- > windows/ovsext/Conntrack.c b/datapath-windows/ovsext/Conntrack.c > index 9f41861..23de526 100644 > --- a/datapath-windows/ovsext/Conntrack.c > +++ b/datapath-windows/ovsext/Conntrack.c > @@ -702,8 +702,7 @@ OvsCtExecute_(PNET_BUFFER_LIST curNbl, > *--------------------------------------------------------------------------- > */ > NDIS_STATUS > -OvsExecuteConntrackAction(PNET_BUFFER_LIST curNbl, > - OVS_PACKET_HDR_INFO *layers, > +OvsExecuteConntrackAction(OvsForwardingContext *fwdCtx, > OvsFlowKey *key, > const PNL_ATTR a) { @@ -713,6 +712,8 @@ > OvsExecuteConntrackAction(PNET_BUFFER_LIST curNbl, > MD_MARK *mark = NULL; > MD_LABELS *labels = NULL; > PCHAR helper = NULL; > + PNET_BUFFER_LIST curNbl = fwdCtx->curNbl; > + OVS_PACKET_HDR_INFO *layers = &fwdCtx->layers; > > NDIS_STATUS status; > > diff --git a/datapath-windows/ovsext/Conntrack.h b/datapath- > windows/ovsext/Conntrack.h > index af99885..87d7eeb 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 "Actions.h" > #include <stddef.h> > > #ifdef OVS_DBG_MOD > @@ -155,8 +156,7 @@ OvsGetTcpPayloadLength(PNET_BUFFER_LIST nbl) > VOID OvsCleanupConntrack(VOID); NTSTATUS > OvsInitConntrack(POVS_SWITCH_CONTEXT context); > > -NDIS_STATUS OvsExecuteConntrackAction(PNET_BUFFER_LIST curNbl, > - OVS_PACKET_HDR_INFO *layers, > +NDIS_STATUS OvsExecuteConntrackAction(OvsForwardingContext > *fwdCtx, > OvsFlowKey *key, > const PNL_ATTR a); BOOLEAN > OvsConntrackValidateTcpPacket(const TCPHdr *tcp); > -- > 2.10.2.windows.1 _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
