[ovs-dev] [PATCH] datapath-windows: check source port during tunnel Tx

[Nithin Raju]

> I saw that Ben already applied this patch yesterday. However, I just have one comment below.

Np. I'll take care of review comments in a followup patch.

>> +        /*
>> +         * Tx:
>> +         * The destination port is a tunnel port. Encapsulation must be
>> +         * performed only on packets that originate from a VIF port or from
>> +         * userspace (default port)
>> +         *
>> +         * If the packet will not be encapsulated, consume the tunnel context
>> +         * by clearing it.
>> +         */
>> +        if (ovsFwdCtx->srcVportNo != 0 &&
>> +            !OvsIsVifVportNo(ovsFwdCtx->srcVportNo)) {
>> +            ovsFwdCtx->tunKey.dst = 0;
>> +        }
>> +
>> +        /* Tunnel the packet only if tunnel context is set. */
>> +        if (ovsFwdCtx->tunKey.dst != 0) {
>> +            ovsActionStats.txVxlan++;
>> +            ovsFwdCtx->tunnelTxNic = dstVport;
>> +        }
>> +
> 
> It looks like the two if's can be combined. I don't see that variable being of any relevance if we are not adding the output port.
> Also, we should use NDIS_DEFAULT_SWITCH_PORT_ID instead of 0.

The first 'if' if TRUE is also clearing the tunnel context. This kind of renders the second 'if' to be FALSE.

We cannot use NDIS_DEFAULT_SWITCH_PORT_ID since that constant is for a NDIS port ID. I just sent out a patch where I defined a OVS_VPORTNO_DEFAULT instead of using 0.

thanks,
Nithin