| Message ID | BYAPR05MB6246E93A989329BFA7647EA7D4420@BYAPR05MB6246.namprd05.prod.outlook.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [ovs-dev] datapath-windows: Don't delete internal port | expand |
> On 3 Dec 2019, at 06:38, Jinjun Gao <jinjung@vmware.com> wrote: > > According to the microsoft doc: > https://docs.microsoft.com/en-us/windows-hardware/drivers/network/hyper-v-extensible-switch-port-and-network-adapter-states > Below OID request sequence is validation: > OID_SWITCH_NIC_CONNECT -> OID_SWITCH_NIC_DISCONNECT > ^ | > | V > OID_SWITCH_NIC_CREATE <- OID_SWITCH_NIC_DELETE > > In above sequence, the windows extensible switch interface assumes the > OID_SWITCH_PORT_CREATE has issued and the port has been created > successfully. If delete the internal port in HvDisconnectNic(), > HvCreateNic() will fail when received OID_SWITCH_NIC_CREATE late because > there is no corresponding port. > > Signed-off-by: Jinjun Gao <jinjung@vmware.com> > --- Thanks for sending the patch out! I tried to apply the patch from patchworks and it fails to apply on master: $ git apply mbox error: corrupt patch at line 183 Indeed we should not call OvsRemoveAndDeleteVport, however we should keep OvsPostVportEvent and also call it for all the ports so that the userspace will be notified of the changes. Can you please fold in the following: diff --git a/datapath-windows/ovsext/Vport.c b/datapath-windows/ovsext/Vport.c index 57e75109d..9f1587f44 100644 --- a/datapath-windows/ovsext/Vport.c +++ b/datapath-windows/ovsext/Vport.c @@ -628,6 +628,7 @@ HvDisconnectNic(POVS_SWITCH_CONTEXT switchContext, event.upcallPid = vport->upcallPid; RtlCopyMemory(&event.ovsName, &vport->ovsName, sizeof event.ovsName); event.type = OVS_EVENT_LINK_DOWN; + OvsPostVportEvent(&event); /* * Delete the port from the hash tables accessible to userspace. After this @@ -635,13 +636,18 @@ HvDisconnectNic(POVS_SWITCH_CONTEXT switchContext, */ if (OvsIsRealExternalVport(vport)) { OvsRemoveAndDeleteVport(NULL, switchContext, vport, FALSE, TRUE); - OvsPostVportEvent(&event); } if (isInternalPort) { OvsUnBindVportWithIpHelper(vport, switchContext); - OvsRemoveAndDeleteVport(NULL, switchContext, vport, TRUE, TRUE); - OvsPostVportEvent(&event); + /* + * Don't delete the port from the hash tables here for internal port + * because the internal port cannot be recreated in HvCreateNic(). It + * only can be created in HvCreatePort() by issuing + * OID_SWITCH_PORT_CREATE. We should wait extensible switch interface + * to issue OID_SWITCH_PORT_TEARDOWN and OID_SWITCH_PORT_DELETE to + * delete the internal port. + */ } NdisReleaseRWLock(switchContext->dispatchLock, &lockState);
Applied on master! Thank you!
On 6 Dec 2019, at 16:39, Alin Serdean <aserdean@cloudbasesolutions.com<mailto:aserdean@cloudbasesolutions.com>> wrote:
On 3 Dec 2019, at 06:38, Jinjun Gao <jinjung@vmware.com<mailto:jinjung@vmware.com>> wrote:
According to the microsoft doc:
https://docs.microsoft.com/en-us/windows-hardware/drivers/network/hyper-v-extensible-switch-port-and-network-adapter-states
Below OID request sequence is validation:
OID_SWITCH_NIC_CONNECT -> OID_SWITCH_NIC_DISCONNECT
^ |
| V
OID_SWITCH_NIC_CREATE <- OID_SWITCH_NIC_DELETE
In above sequence, the windows extensible switch interface assumes the
OID_SWITCH_PORT_CREATE has issued and the port has been created
successfully. If delete the internal port in HvDisconnectNic(),
HvCreateNic() will fail when received OID_SWITCH_NIC_CREATE late because
there is no corresponding port.
Signed-off-by: Jinjun Gao <jinjung@vmware.com>
---
Thanks for sending the patch out!
I tried to apply the patch from patchworks and it fails to apply on master:
$ git apply mbox
error: corrupt patch at line 183
Indeed we should not call OvsRemoveAndDeleteVport, however we should keep
OvsPostVportEvent and also call it for all the ports so that the userspace will be
notified of the changes.
Can you please fold in the following:
diff --git a/datapath-windows/ovsext/Vport.c b/datapath-windows/ovsext/Vport.c
index 57e75109d..9f1587f44 100644
--- a/datapath-windows/ovsext/Vport.c
+++ b/datapath-windows/ovsext/Vport.c
@@ -628,6 +628,7 @@ HvDisconnectNic(POVS_SWITCH_CONTEXT switchContext,
event.upcallPid = vport->upcallPid;
RtlCopyMemory(&event.ovsName, &vport->ovsName, sizeof event.ovsName);
event.type = OVS_EVENT_LINK_DOWN;
+ OvsPostVportEvent(&event);
/*
* Delete the port from the hash tables accessible to userspace. After this
@@ -635,13 +636,18 @@ HvDisconnectNic(POVS_SWITCH_CONTEXT switchContext,
*/
if (OvsIsRealExternalVport(vport)) {
OvsRemoveAndDeleteVport(NULL, switchContext, vport, FALSE, TRUE);
- OvsPostVportEvent(&event);
}
if (isInternalPort) {
OvsUnBindVportWithIpHelper(vport, switchContext);
- OvsRemoveAndDeleteVport(NULL, switchContext, vport, TRUE, TRUE);
- OvsPostVportEvent(&event);
+ /*
+ * Don't delete the port from the hash tables here for internal port
+ * because the internal port cannot be recreated in HvCreateNic(). It
+ * only can be created in HvCreatePort() by issuing
+ * OID_SWITCH_PORT_CREATE. We should wait extensible switch interface
+ * to issue OID_SWITCH_PORT_TEARDOWN and OID_SWITCH_PORT_DELETE to
+ * delete the internal port.
+ */
}
NdisReleaseRWLock(switchContext->dispatchLock, &lockState);
diff --git a/datapath-windows/ovsext/Vport.c b/datapath-windows/ovsext/Vport.c index 57e7510..c3362d7 100644 --- a/datapath-windows/ovsext/Vport.c +++ b/datapath-windows/ovsext/Vport.c @@ -640,8 +640,14 @@ HvDisconnectNic(POVS_SWITCH_CONTEXT switchContext, if (isInternalPort) { OvsUnBindVportWithIpHelper(vport, switchContext); - OvsRemoveAndDeleteVport(NULL, switchContext, vport, TRUE, TRUE); - OvsPostVportEvent(&event); + /* + * Don't delete the port from the hash tables here for internal port + * because the internal port cannot be recreated in HvCreateNic(). It + * only can be created in HvCreatePort() by issuing + * OID_SWITCH_PORT_CREATE. We should wait extensible switch interface + * to issue OID_SWITCH_PORT_TEARDOWN and OID_SWITCH_PORT_DELETE to + * delete the internal port. + */ } NdisReleaseRWLock(switchContext->dispatchLock, &lockState); -- 1.8.5.6 _______________________________________________
According to the microsoft doc: https://docs.microsoft.com/en-us/windows-hardware/drivers/network/hyper-v-extensible-switch-port-and-network-adapter-states Below OID request sequence is validation: OID_SWITCH_NIC_CONNECT -> OID_SWITCH_NIC_DISCONNECT ^ | | V OID_SWITCH_NIC_CREATE <- OID_SWITCH_NIC_DELETE In above sequence, the windows extensible switch interface assumes the OID_SWITCH_PORT_CREATE has issued and the port has been created successfully. If delete the internal port in HvDisconnectNic(), HvCreateNic() will fail when received OID_SWITCH_NIC_CREATE late because there is no corresponding port. Signed-off-by: Jinjun Gao <jinjung@vmware.com> --- datapath-windows/ovsext/Vport.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) dev mailing list dev@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev