Thanks for the review Nithin.
We should not set the NDIS state to any state which is not what NDIS maintains.
If the state sent by NDIS is not Teardown the OVS port state should not be
Teardown (should probably be in error state or whatever sent by NDIS).
Forwarding should be done according to the connection state of the port. In any
case we should not have a maintain "NDIS port state" and set it by the driver.
Eitan
-----Original Message-----
From: Nithin Raju
Sent: Wednesday, November 12, 2014 6:20 AM
To: Eitan Eliahu
Cc: <dev@openvswitch.org>
Subject: Re: [ovs-dev] [PATCH] datapath-windows: Remove Hyper-V port name / Do
not set the NDIS port state
On Nov 12, 2014, at 2:03 AM, Eitan Eliahu <elia...@vmware.com> wrote:
> [1] The NDIS port state should always reflect the port state maintained by
> NDIS so it should never be directly updated.
> [2] Remove the "port name" field as we use the "friendly name" instead.
>
> Signed-off-by: Eitan Eliahu <elia...@vmware.com>
Eitan,
Nuking "port name" is fine. However, I don't understand what is the practical
use of port state change. Perhaps a change like this suffices?
- /* add assertion here
- */
+ ASSERT(portParam->PortState == NdisSwitchPortStateTeardown);
- vport->portState = NdisSwitchPortStateTeardown;
+ vport->portState = portParam->PortState;
vport->ovsState = OVS_STATE_PORT_TEAR_DOWN;
One thing to note is that we rely a lot of vport->ovsState while processing
packets. In OvsAddPorts() in Actions.c, we decide to forward packets to an NDIS
port only if it in in the CONNECTED state. If we fail to update the state here
to OVS_STATE_PORT_TEAR_DOWN, we might end up forwarding packets to a vport in
!CONNECTED state, and that can lead to bug checks.
thanks,
-- Nithin
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
