Hi Ankur, OvsAddPorts() is the function that gets called if the action specifies an output port. This function is equipped to handle any kind of output port. The called does not have do any kind of special casing depending on the port.
So the change is necessary to equip the function to handle a new port type - bridge-internal port. The functionality itself OSA no-op since there is no backing for the bridge-internal port. We cannot ask NDIS to forward to a non-existent port. So, the no-op. Thanks, -- Nithin - sent from mobile - > On Oct 24, 2014, at 15:25, "Ankur Sharma" <ankursha...@vmware.com> wrote: > > Hi Nithin, > > Some minor comments: > > why did we add if (OvsIsBridgeInternalVport(vport)) check in OvsAddPorts? > > > minor comment > in OvsIsBridgeInternalVport > we can just do return vport->isBridgeInternal > instead of return vport->isBridgeInternal == TRUE > > > Acked-by: Ankur Sharma <ankursha...@vmware.com> > ________________________________________ > From: dev <dev-boun...@openvswitch.org> on behalf of Alin Serdean > <aserd...@cloudbasesolutions.com> > Sent: Friday, October 24, 2014 3:15 PM > To: Nithin Raju; dev@openvswitch.org > Subject: Re: [ovs-dev] [PATCH 3/7] datapath-windows: introduce > bridge-internal ports > > Acked-by: Alin Gabriel Serdean <aserd...@cloudbasesolutions.com> > > Tested-by: Alin Gabriel Serdean <aserd...@cloudbasesolutions.com> > > > > > > -----Mesaj original----- > > De la: dev [mailto:dev-boun...@openvswitch.org] În numele Nithin Raju > > Trimis: Friday, October 24, 2014 3:33 AM > > Către: dev@openvswitch.org > > Subiect: [ovs-dev] [PATCH 3/7] datapath-windows: introduce bridge-internal > ports > > > > In this patch, we provide explanation and the reasoning for bridge-internal > ports. The code to add such a port in come in later patch in the series. > > > > We also fix some formatting issues in PacketIO.c. > > > > Signed-off-by: Nithin Raju <nit...@vmware.com> > > --- > > datapath-windows/ovsext/Actions.c | 13 +++++++++---- > > datapath-windows/ovsext/PacketIO.c | 11 ++++------- > > datapath-windows/ovsext/Vport.h | 29 +++++++++++++++++++++++++++++ > > 3 files changed, 42 insertions(+), 11 deletions(-) > > > > diff --git a/datapath-windows/ovsext/Actions.c > b/datapath-windows/ovsext/Actions.c > > index 3f8c351..14d1f8f 100644 > > --- a/datapath-windows/ovsext/Actions.c > > +++ b/datapath-windows/ovsext/Actions.c > > @@ -248,10 +248,11 @@ OvsDetectTunnelPkt(OvsForwardingContext *ovsFwdCtx, > > * port or if it is being executed from userspace, the source port is > > * default port. > > */ > > - BOOLEAN validSrcPort = (ovsFwdCtx->fwdDetail->SourcePortId == > > - > ovsFwdCtx->switchContext->virtualExternalPortId) || > > - (ovsFwdCtx->fwdDetail->SourcePortId == > > - NDIS_SWITCH_DEFAULT_PORT_ID); > > + BOOLEAN validSrcPort = > > + (ovsFwdCtx->fwdDetail->SourcePortId == > > + ovsFwdCtx->switchContext->virtualExternalPortId) || > > + (ovsFwdCtx->fwdDetail->SourcePortId == > > + NDIS_SWITCH_DEFAULT_PORT_ID); > > > > if (validSrcPort && OvsDetectTunnelRxPkt(ovsFwdCtx, flowKey)) { > > ASSERT(ovsFwdCtx->tunnelTxNic == NULL); @@ -343,6 +344,10 @@ > OvsAddPorts(OvsForwardingContext *ovsFwdCtx, > > vport->stats.txBytes += > > NET_BUFFER_DATA_LENGTH(NET_BUFFER_LIST_FIRST_NB(ovsFwdCtx->curNbl)); > > > > + if (OvsIsBridgeInternalVport(vport)) { > > + return NDIS_STATUS_SUCCESS; > > + } > > + > > if (OvsDetectTunnelPkt(ovsFwdCtx, vport, flowKey)) { > > return NDIS_STATUS_SUCCESS; > > } > > diff --git a/datapath-windows/ovsext/PacketIO.c > b/datapath-windows/ovsext/PacketIO.c > > index 027b42a..5223125 100644 > > --- a/datapath-windows/ovsext/PacketIO.c > > +++ b/datapath-windows/ovsext/PacketIO.c > > @@ -236,7 +236,7 @@ OvsStartNBLIngress(POVS_SWITCH_CONTEXT switchContext, > > dispatch); > > > > ctx = OvsInitExternalNBLContext(switchContext, curNbl, > > - sourcePort == > switchContext->virtualExternalPortId); > > + sourcePort == > > + switchContext->virtualExternalPortId); > > if (ctx == NULL) { > > RtlInitUnicodeString(&filterReason, > > L"Cannot allocate external NBL > context."); @@ -288,12 +288,9 @@ OvsStartNBLIngress(POVS_SWITCH_CONTEXT > switchContext, > > > > datapath->misses++; > > status = OvsCreateAndAddPackets(NULL, 0, OVS_PACKET_CMD_MISS, > > - portNo, > > - &key, curNbl, > > - sourcePort == > > - > switchContext->virtualExternalPortId, > > - &layers, switchContext, > > - &missedPackets, &num); > > + portNo, &key, curNbl, > > + sourcePort == > switchContext->virtualExternalPortId, > > + &layers, switchContext, &missedPackets, > > + &num); > > if (status == NDIS_STATUS_SUCCESS) { > > /* Complete the packet since it was copied to user > > * buffer. */ > > diff --git a/datapath-windows/ovsext/Vport.h > b/datapath-windows/ovsext/Vport.h index 9fe9f54..73ab80f 100644 > > --- a/datapath-windows/ovsext/Vport.h > > +++ b/datapath-windows/ovsext/Vport.h > > @@ -32,6 +32,11 @@ > > */ > > #define OVS_DPPORT_NUMBER_LOCAL 0 > > > > +#define OVS_DPPORT_INTERNAL_NAME_A "internal" > > +#define OVS_DPPORT_INTERNAL_NAME_W L"internal" > > +#define OVS_DPPORT_EXTERNAL_NAME_A "external" > > +#define OVS_DPPORT_EXTERNAL_NAME_W L"external" > > + > > /* > > * A Vport, or Virtual Port, is a port on the OVS. It can be one of the > > * following types. Some of the Vports are "real" ports on the hyper-v > switch, @@ -106,6 +111,21 @@ typedef struct _OVS_VPORT_ENTRY { > > NDIS_SWITCH_NIC_NAME nicName; > > NDIS_VM_NAME vmName; > > GUID netCfgInstanceId; > > + /* > > + * OVS userpace has a notion of bridges which basically defines an > > + * L2-domain. Each "bridge" has an "internal" port of type > > + * OVS_VPORT_TYPE_INTERNAL. Such a port is connected to the OVS datapath > in > > + * one end, and the other end is a virtual adapter on the hypervisor > host. > > + * This is akin to the Hyper-V "internal" NIC. It is intuitive to map the > > + * Hyper-V "internal" NIC to the OVS bridge's "internal" port, but > there's > > + * only one Hyper-V NIC but multiple bridges. To support multiple OVS > bridge > > + * "internal" ports, we use the flag 'isBridgeInternal' in each vport. We > > + * support addition of multiple bridge-internal ports. A vport with > > + * 'isBridgeInternal' == TRUE is a dummy port and has no backing > currently. > > + * If a flow actions specifies the output port to be a bridge-internal > port, > > + * the port is silently ignored. > > + */ > > + BOOLEAN isBridgeInternal; > > BOOLEAN isExternal; > > UINT32 upcallPid; /* netlink upcall port id */ > > PNL_ATTR portOptions; > > @@ -165,6 +185,15 @@ OvsIsInternalVportType(OVS_VPORT_TYPE ovsType) > > return ovsType == OVS_VPORT_TYPE_INTERNAL; } > > > > +static __inline BOOLEAN > > +OvsIsBridgeInternalVport(POVS_VPORT_ENTRY vport) { > > + if (vport->isBridgeInternal) { > > + ASSERT(vport->ovsType == OVS_VPORT_TYPE_INTERNAL); > > + } > > + return vport->isBridgeInternal == TRUE; } > > + > > static __inline UINT32 > > OvsGetExternalMtu() > > { > > -- > > 1.7.4.1 > > > > _______________________________________________ > > dev mailing list > > dev@openvswitch.org > > https://urldefense.proofpoint.com/v1/url?u=http://openvswitch.org/mailman/listinfo/dev&k=oIvRg1%2BdGAgOoM1BIlLLqw%3D%3D%0A&r=f6EhnZ0ORGZNt5QbYmRaOxfWfx%2Bqd3KEiPf3%2FYaollU%3D%0A&m=li0lVwh5ySmSTTL%2BGTCGmVK8Zsi94pTDoobHMfpP2%2BE%3D%0A&s=011a644bd25e88d446b32de5c4a91d75d418d9433241cccc36623211468cef77 > > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://urldefense.proofpoint.com/v1/url?u=http://openvswitch.org/mailman/listinfo/dev&k=oIvRg1%2BdGAgOoM1BIlLLqw%3D%3D%0A&r=f6EhnZ0ORGZNt5QbYmRaOxfWfx%2Bqd3KEiPf3%2FYaollU%3D%0A&m=li0lVwh5ySmSTTL%2BGTCGmVK8Zsi94pTDoobHMfpP2%2BE%3D%0A&s=011a644bd25e88d446b32de5c4a91d75d418d9433241cccc36623211468cef77 _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
