Re: [ovs-dev] [PATCH] datapath-windows: Updated WFP system provider handling
Hi Eitan, Thanks for reviewing my patch. [1] The OvsTunnelEngineClose(HANDLE *handle) is verifying the handle it receives before closing it. [2] In order to retrieve the current state of the filter engine I am using the FwpmBfeStateGet function. But before calling FwpmBfeStateGet, the callout driver MUST call the FwpmBfeStateSubscribeChanges function. That is the reason I am registering the callback no mater of the state of the filter engine. Only if the engine is running the provider is added by opening the engine handle first and closing it after the provider is added, then unsubscribes the callback. If the engine is not running the callback is called and the provider is added only for the service running notification. Hope this answers your comments. Thanks, Sorin -Original Message- From: Eitan Eliahu [mailto:elia...@vmware.com] Sent: Monday, 23 March, 2015 02:32 To: Sorin Vinturis Cc: dev@openvswitch.org Subject: RE: [ovs-dev] [PATCH] datapath-windows: Updated WFP system provider handling Hi Sorin, Thank you for posting this change. Here are few comments: [1] Please call OvsTunnelEngineClose(&handle) only if the handle is not zero. (you might want to add an assertion if it zero). [2] I'm somewhat confused about the registration for engine state change. As I understand you check the state of the engine and if it is running you open the engine handle (in the context of the foreground thread). However, you subscribe to state change anyway and you open the handle to the engine from the callback function too. Did I miss anything? Thanks, Eitan -Original Message- From: dev [mailto:dev-boun...@openvswitch.org] On Behalf Of Sorin Vinturis Sent: Monday, 16 March, 2015 20:08 To: dev@openvswitch.org Subject: [ovs-dev] [PATCH] datapath-windows: Updated WFP system provider handling If the Base Filtering Engine (BFE) is not started, the WFP system provider failed to be added because no session to the engine could be acquired. The solution for this was to registered a BFE notification callback that is called whenever the BFE's state changes. Only if the BFE's state is running the WFP system provider is added. Signed-off-by: Sorin Vinturis Reported-by: Sorin Vinturis Reported-at: https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_openvswitch_ovs-2Dissues_issues_65&d=AwIGaQ&c=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs&r=CWsgHUxi6ExLXY798tmo3LJ4e3geGYp56lkcH-5cLCY&m=MVriWGIsZGTQ5f_haLS0X5H8R3YdfikbqnWKRpKksKU&s=1iWqFul95i3o_mD41jKjZNdLnK-df_QWRzSY1614RTw&e= --- datapath-windows/ovsext/Datapath.c | 20 ++- datapath-windows/ovsext/TunnelFilter.c | 99 -- datapath-windows/ovsext/TunnelIntf.h | 8 +-- 3 files changed, 100 insertions(+), 27 deletions(-) diff --git a/datapath-windows/ovsext/Datapath.c b/datapath-windows/ovsext/Datapath.c index 8eb13f1..c6fe89e 100644 --- a/datapath-windows/ovsext/Datapath.c +++ b/datapath-windows/ovsext/Datapath.c @@ -353,35 +353,19 @@ PNDIS_SPIN_LOCK gOvsCtrlLock; VOID OvsInit() { -HANDLE handle = NULL; - gOvsCtrlLock = &ovsCtrlLockObj; NdisAllocateSpinLock(gOvsCtrlLock); OvsInitEventQueue(); - -OvsTunnelEngineOpen(&handle); -if (handle) { -OvsTunnelAddSystemProvider(handle); -} -OvsTunnelEngineClose(&handle); } VOID OvsCleanup() { -HANDLE handle = NULL; - OvsCleanupEventQueue(); if (gOvsCtrlLock) { NdisFreeSpinLock(gOvsCtrlLock); gOvsCtrlLock = NULL; } - -OvsTunnelEngineOpen(&handle); -if (handle) { -OvsTunnelRemoveSystemProvider(handle); -} -OvsTunnelEngineClose(&handle); } VOID @@ -448,6 +432,8 @@ OvsCreateDeviceObject(NDIS_HANDLE ovsExtDriverHandle) if (ovsExt) { ovsExt->numberOpenInstance = 0; } +} else { +OvsRegisterSystemProvider((PVOID)gOvsDeviceObject); } OVS_LOG_TRACE("DeviceObject: %p", gOvsDeviceObject); @@ -471,6 +457,8 @@ OvsDeleteDeviceObject() NdisDeregisterDeviceEx(gOvsDeviceHandle); gOvsDeviceHandle = NULL; gOvsDeviceObject = NULL; + +OvsUnregisterSystemProvider(); } } diff --git a/datapath-windows/ovsext/TunnelFilter.c b/datapath-windows/ovsext/TunnelFilter.c index e0adc37..4b879c0 100644 --- a/datapath-windows/ovsext/TunnelFilter.c +++ b/datapath-windows/ovsext/TunnelFilter.c @@ -111,6 +111,7 @@ DEFINE_GUID( PDEVICE_OBJECT gDeviceObject; HANDLE gEngineHandle = NULL; +HANDLE gBfeSubscriptionHandle = NULL; UINT32 gCalloutIdV4; @@ -173,17 +174,20 @@ OvsTunnelAddSystemProvider(HANDLE handle) provider.displayData.name = OVS_TUNNEL_PROVIDER_NAME; provider.displayData.description = OVS_TUNNEL_PROVIDER_DESC; /* -* Since we always want the provider to be present, it's easiest to add -* it as persistent object during d
Re: [ovs-dev] [PATCH 0/6] DPDK: simplify configuration
> > > On 19 Mar 2015, at 18:34, Gray, Mark D wrote: > > > >> From: dev [mailto:dev-boun...@openvswitch.org] On Behalf Of Gray, > >> Mark D > >> Sent: Thursday, March 19, 2015 6:19 PM > >> To: Traynor, Kevin; Daniele Di Proietto > >> Subject: Re: [ovs-dev] [PATCH 0/6] DPDK: simplify configuration > >> > >> [snip] > > > > Hi, I've reviewed this patchset - few comments/questions on it… > > Hi Kevin, thanks > > > I haven't tested yet - but I'm wondering what is the impact to the > > dpdk -c parameter. Is it no longer used for OVS? > > > > Yes, that’s correct. It should have no impact on OVS. I think we > should also provide a default, i.e. generate the parameters passed > to dpdk_eal_init at some point. > >> > >> +1 to removing the -c parameter as it seems to be the same as > >> 'other_config:n-pmd-cores' and is a little confusing as to how they > interact. > >> Maybe 'other_config:n-pmd-cores' could be a mandatory option? > > > > I used the wrong config option in above. Let me rephrase it. > > +1 to removing the -c parameter as it seems to be the same as > > 'other_config:pmd-cpu-mask + other_config:nonpmd-cpu-mask' and is a > little confusing as to how they interact. > > Maybe 'other_config:pmd-cpu-mask + other_config:nonpmd-cpu-mask' > could be a mandatory option? > > The idea behind this change is to let advanced users configure their masks, > and provide a simple configuration parameter (n-pmd-cores) for users who > do not want or need to deal with cpu masks. What do you think? Is the -c option needed in either of these cases? In both cases, it could be derived and would simplify the command line. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH] datapath-windows: Removed gOvsCtrlLock global spinlock
There is no need to use gOvsCtrlLock spinlock to guard the switch context, as there is now the switch context's reference count used for this purpose. Now the gOvsCtrlLock spinlock guards only one shared resource, the OVS_OPEN_INSTANCE global instance array. Signed-off-by: Sorin Vinturis --- datapath-windows/ovsext/Datapath.c | 15 --- datapath-windows/ovsext/Flow.c | 39 +- datapath-windows/ovsext/Switch.c | 5 - datapath-windows/ovsext/User.c | 22 - datapath-windows/ovsext/Vport.c| 22 ++--- 5 files changed, 19 insertions(+), 84 deletions(-) diff --git a/datapath-windows/ovsext/Datapath.c b/datapath-windows/ovsext/Datapath.c index 8b561a3..6a76a12 100644 --- a/datapath-windows/ovsext/Datapath.c +++ b/datapath-windows/ovsext/Datapath.c @@ -918,14 +918,11 @@ ValidateNetlinkCmd(UINT32 devOp, /* Validate the DP for commands that require a DP. */ if (nlFamilyOps->cmds[i].validateDpIndex == TRUE) { -OvsAcquireCtrlLock(); if (ovsMsg->ovsHdr.dp_ifindex != (INT)gOvsSwitchContext->dpNo) { status = STATUS_INVALID_PARAMETER; -OvsReleaseCtrlLock(); goto done; } -OvsReleaseCtrlLock(); } /* Validate the PID. */ @@ -1012,7 +1009,6 @@ OvsGetPidCmdHandler(POVS_USER_PARAMS_CONTEXT usrParamsCtx, * -- * Utility function to fill up information about the datapath in a reply to * userspace. - * Assumes that 'gOvsCtrlLock' lock is acquired. * -- */ static NTSTATUS @@ -1212,9 +1208,7 @@ HandleGetDpDump(POVS_USER_PARAMS_CONTEXT usrParamsCtx, NlBufInit(&nlBuf, usrParamsCtx->outputBuffer, usrParamsCtx->outputLength); -OvsAcquireCtrlLock(); status = OvsDpFillInfo(gOvsSwitchContext, msgIn, &nlBuf); -OvsReleaseCtrlLock(); if (status != STATUS_SUCCESS) { *replyLen = 0; @@ -1301,11 +1295,9 @@ HandleDpTransactionCommon(POVS_USER_PARAMS_CONTEXT usrParamsCtx, NlBufInit(&nlBuf, usrParamsCtx->outputBuffer, usrParamsCtx->outputLength); -OvsAcquireCtrlLock(); if (dpAttrs[OVS_DP_ATTR_NAME] != NULL) { if (!OvsCompareString(NlAttrGet(dpAttrs[OVS_DP_ATTR_NAME]), OVS_SYSTEM_DP_NAME)) { -OvsReleaseCtrlLock(); /* Creation of new datapaths is not supported. */ if (usrParamsCtx->ovsMsg->genlMsg.cmd == OVS_DP_CMD_SET) { @@ -1317,19 +1309,16 @@ HandleDpTransactionCommon(POVS_USER_PARAMS_CONTEXT usrParamsCtx, goto cleanup; } } else if ((UINT32)msgIn->ovsHdr.dp_ifindex != gOvsSwitchContext->dpNo) { -OvsReleaseCtrlLock(); nlError = NL_ERROR_NODEV; goto cleanup; } if (usrParamsCtx->ovsMsg->genlMsg.cmd == OVS_DP_CMD_NEW) { -OvsReleaseCtrlLock(); nlError = NL_ERROR_EXIST; goto cleanup; } status = OvsDpFillInfo(gOvsSwitchContext, msgIn, &nlBuf); -OvsReleaseCtrlLock(); *replyLen = NlBufSize(&nlBuf); @@ -1411,7 +1400,6 @@ MapIrpOutputBuffer(PIRP irp, * -- * Utility function to fill up information about the state of a port in a reply * to* userspace. - * Assumes that 'gOvsCtrlLock' lock is acquired. * -- */ static NTSTATUS @@ -1515,8 +1503,6 @@ OvsReadEventCmdHandler(POVS_USER_PARAMS_CONTEXT usrParamsCtx, NlBufInit(&nlBuf, usrParamsCtx->outputBuffer, usrParamsCtx->outputLength); -OvsAcquireCtrlLock(); - /* remove an event entry from the event queue */ status = OvsRemoveEventEntry(usrParamsCtx->ovsInstance, &eventEntry); if (status != STATUS_SUCCESS) { @@ -1532,7 +1518,6 @@ OvsReadEventCmdHandler(POVS_USER_PARAMS_CONTEXT usrParamsCtx, } cleanup: -OvsReleaseCtrlLock(); return status; } diff --git a/datapath-windows/ovsext/Flow.c b/datapath-windows/ovsext/Flow.c index d3de8cc..1f1bd2f 100644 --- a/datapath-windows/ovsext/Flow.c +++ b/datapath-windows/ovsext/Flow.c @@ -31,7 +31,6 @@ #pragma warning( push ) #pragma warning( disable:4127 ) -extern PNDIS_SPIN_LOCK gOvsCtrlLock; extern POVS_SWITCH_CONTEXT gOvsSwitchContext; extern UINT64 ovsTimeIncrementPerTick; @@ -1995,25 +1994,23 @@ OvsDoDumpFlows(OvsFlowDumpInput *dumpInput, BOOLEAN findNextNonEmpty = FALSE; dpNo = dumpInput->dpNo; -NdisAcquireSpinLock(gOvsCtrlLock); if (gOvsSwitchContext->dpNo != dpNo) { status = STATUS_INVALID_PARAMETER; -goto unlock; +goto exit; } rowIndex = dum
Re: [ovs-dev] [PATCH v4] ovn: add ovn-nbctl
On Sun, Mar 22, 2015 at 04:38:50PM -0400, Russell Bryant wrote: > ovn-nbctl is intended to be a utility for manually interacting with > the OVN northbound db. A real consumer of OVN, such as OpenStack, > should be using ovsdb directly. However, a utility like this is > useful during development and testing of both ovn itself as well as > the OpenStack Neutron plugin. It could also be used when debugging > the state of an ovn deployment. > > The commands related to logical switches and ports have been > implemented. You can add, delete, list, and get/set external:ids. > On ports you can also get/set MAC addresses. > > Signed-off-by: Russell Bryant > --- > > > v1 > - first cut of lswitch-* commands > v2 > - fix handling of multiple switches with the same name > v3 > - add -d/--database option > - add lport-* commands > v4 > - add --database option to support a non-default or remote db > - set up and use vlog > - add tls options > - other minor cleanups > - (I think I'm actually done for now this time ...) Thanks a lot! One thing we'll have to think about here is clean transactions. The code as-is does not use any "verify" operations to make sure that the overall transaction is atomic. This might not be a big deal for ovn-nbctl, and it definitely is not yet, but it is worth thinking about (especially since OpenStack will want atomic transactions so if we catch any issues soon then we'll be better prepared for OpenStack's real use case). I noticed that some of the VLOG invocations put a new-line at the end. That's not necessary, although it's harmless, and our convention is to leave it out, so I removed them. I noticed a few long lines, so I wrapped them. The manpage needed some font adjustments, so I did that. I also changed the subject line of the commit message to start with a capital letter and end with a period, because for whatever reason that's my preferred convention in OVS. With those changes I applied this to the ovn branch. Thanks again! diff --git a/ovn/ovn-nbctl.8.xml b/ovn/ovn-nbctl.8.xml index dad5e9a..54f9620 100644 --- a/ovn/ovn-nbctl.8.xml +++ b/ovn/ovn-nbctl.8.xml @@ -4,42 +4,42 @@ ovn-nbctl -- Open Virtual Network northbound db management utility Synopsys -ovn-nbctl [OPTIONS] COMMAND [ARG...] +ovn-nbctl [options] command [arg...] Description This utility can be used to manage the OVN northbound database. Logical Switch Commands -lswitch-add [name] -lswitch-del-lswitch-list -lswitch-set-external-id [value] -lswitch-get-external-id [key] +lswitch-add [name] +lswitch-del lswitch +lswitch-list +lswitch-set-external-id lswitch key [value] +lswitch-get-external-id lswitch [key] Logical Port Commands -lport-add -lport-del -lport-list -lport-set-external-id [value] -lport-get-external-id [key] -lport-set-macs [MAC] [MAC] [...] -lport-get-macs +lport-add name lswitch +lport-del lport +lport-list lswitch +lport-set-external-id lport key [value] +lport-get-external-id lport [key] +lport-set-macs lport [mac] [mac] [...] +lport-get-macs lport Options --d DATABASE | --database DATABASE --h | --help --o | --options --V | --version +-d database | --database database +-h | --help +-o | --options +-V | --version Logging options --vSPEC, --verbose=SPEC --v, --verbose ---log-file[=FILE] ---syslog-target=HOST:PORT +-vspec, --verbose=spec +-v, --verbose +--log-file[=file] + --syslog-target=host:port PKI configuration (required to use SSL) --p, --private-key=FILE file with private key --c, --certificate=FILE file with certificate for private key --C, --ca-cert=FILE file with peer CA certificate +-p, --private-key=file file with private key +-c, --certificate=file file with certificate for private key +-C, --ca-cert=file file with peer CA certificate diff --git a/ovn/ovn-nbctl.c b/ovn/ovn-nbctl.c index 3dc41e9..eade156 100644 --- a/ovn/ovn-nbctl.c +++ b/ovn/ovn-nbctl.c @@ -89,7 +89,8 @@ lswitch_by_name_or_uuid(struct nbctl_context *nb_ctx, const char *id) if (uuid_from_string(&lswitch_uuid, id)) { is_uuid = true; -lswitch = nbrec_logical_switch_get_for_uuid(nb_ctx->idl, &lswitch_uuid); +lswitch = nbrec_logical_switch_get_for_uuid(nb_ctx->idl, +&lswitch_uuid); } else { const struct nbrec_logical_switch *iter; @@ -99,7 +100,7 @@ lswitch_by_name_or_uuid(struct nbctl_context *nb_ctx, const char *id) } if (lswitch) { VLOG_WARN("There is more than one logical switch named '%s'. " -"Use a UUID.\n", id); +"Use a UUID.", i
Re: [ovs-dev] [PATCH] datapath-windows: Updated WFP system provider handling
Sorin, Ok on [1]. Thanks. On [2], I can see a scenario when the provider can added twice from OvsRegisterSystemProvider() as well as from the callback if (FWPM_SERVICE_RUNNING == FwpmBfeStateGet()) { > Callback is called from a different thread context ! OvsTunnelEngineOpen(&handle); How do you prevent it? Also, can you please explain which issue you ran into which triggered this change? Thank you, Eitan -Original Message- From: Sorin Vinturis [mailto:svintu...@cloudbasesolutions.com] Sent: Monday, March 23, 2015 1:37 AM To: Eitan Eliahu Cc: dev@openvswitch.org Subject: RE: [ovs-dev] [PATCH] datapath-windows: Updated WFP system provider handling Hi Eitan, Thanks for reviewing my patch. [1] The OvsTunnelEngineClose(HANDLE *handle) is verifying the handle it receives before closing it. [2] In order to retrieve the current state of the filter engine I am using the FwpmBfeStateGet function. But before calling FwpmBfeStateGet, the callout driver MUST call the FwpmBfeStateSubscribeChanges function. That is the reason I am registering the callback no mater of the state of the filter engine. Only if the engine is running the provider is added by opening the engine handle first and closing it after the provider is added, then unsubscribes the callback. If the engine is not running the callback is called and the provider is added only for the service running notification. Hope this answers your comments. Thanks, Sorin -Original Message- From: Eitan Eliahu [mailto:elia...@vmware.com] Sent: Monday, 23 March, 2015 02:32 To: Sorin Vinturis Cc: dev@openvswitch.org Subject: RE: [ovs-dev] [PATCH] datapath-windows: Updated WFP system provider handling Hi Sorin, Thank you for posting this change. Here are few comments: [1] Please call OvsTunnelEngineClose(&handle) only if the handle is not zero. (you might want to add an assertion if it zero). [2] I'm somewhat confused about the registration for engine state change. As I understand you check the state of the engine and if it is running you open the engine handle (in the context of the foreground thread). However, you subscribe to state change anyway and you open the handle to the engine from the callback function too. Did I miss anything? Thanks, Eitan -Original Message- From: dev [mailto:dev-boun...@openvswitch.org] On Behalf Of Sorin Vinturis Sent: Monday, 16 March, 2015 20:08 To: dev@openvswitch.org Subject: [ovs-dev] [PATCH] datapath-windows: Updated WFP system provider handling If the Base Filtering Engine (BFE) is not started, the WFP system provider failed to be added because no session to the engine could be acquired. The solution for this was to registered a BFE notification callback that is called whenever the BFE's state changes. Only if the BFE's state is running the WFP system provider is added. Signed-off-by: Sorin Vinturis Reported-by: Sorin Vinturis Reported-at: https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_openvswitch_ovs-2Dissues_issues_65&d=AwIGaQ&c=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs&r=CWsgHUxi6ExLXY798tmo3LJ4e3geGYp56lkcH-5cLCY&m=MVriWGIsZGTQ5f_haLS0X5H8R3YdfikbqnWKRpKksKU&s=1iWqFul95i3o_mD41jKjZNdLnK-df_QWRzSY1614RTw&e= --- datapath-windows/ovsext/Datapath.c | 20 ++- datapath-windows/ovsext/TunnelFilter.c | 99 -- datapath-windows/ovsext/TunnelIntf.h | 8 +-- 3 files changed, 100 insertions(+), 27 deletions(-) diff --git a/datapath-windows/ovsext/Datapath.c b/datapath-windows/ovsext/Datapath.c index 8eb13f1..c6fe89e 100644 --- a/datapath-windows/ovsext/Datapath.c +++ b/datapath-windows/ovsext/Datapath.c @@ -353,35 +353,19 @@ PNDIS_SPIN_LOCK gOvsCtrlLock; VOID OvsInit() { -HANDLE handle = NULL; - gOvsCtrlLock = &ovsCtrlLockObj; NdisAllocateSpinLock(gOvsCtrlLock); OvsInitEventQueue(); - -OvsTunnelEngineOpen(&handle); -if (handle) { -OvsTunnelAddSystemProvider(handle); -} -OvsTunnelEngineClose(&handle); } VOID OvsCleanup() { -HANDLE handle = NULL; - OvsCleanupEventQueue(); if (gOvsCtrlLock) { NdisFreeSpinLock(gOvsCtrlLock); gOvsCtrlLock = NULL; } - -OvsTunnelEngineOpen(&handle); -if (handle) { -OvsTunnelRemoveSystemProvider(handle); -} -OvsTunnelEngineClose(&handle); } VOID @@ -448,6 +432,8 @@ OvsCreateDeviceObject(NDIS_HANDLE ovsExtDriverHandle) if (ovsExt) { ovsExt->numberOpenInstance = 0; } +} else { +OvsRegisterSystemProvider((PVOID)gOvsDeviceObject); } OVS_LOG_TRACE("DeviceObject: %p", gOvsDeviceObject); @@ -471,6 +457,8 @@ OvsDeleteDeviceObject() NdisDeregisterDeviceEx(gOvsDeviceHandle); gOvsDeviceHandle = NULL; gOvsDeviceObject = NULL; + +OvsUnregisterSystemProvider(); } } diff --git a/datapath-wi
[ovs-dev] [PATCH] INSTALL.DPDK.md: Terminate code section.
Add a missing terminator for a code section. Without this, the rendering on github at least shows the rest of the file as a code block. Signed-off-by: Russell Bryant --- INSTALL.DPDK.md | 1 + 1 file changed, 1 insertion(+) diff --git a/INSTALL.DPDK.md b/INSTALL.DPDK.md index 5b61272..60889d0 100644 --- a/INSTALL.DPDK.md +++ b/INSTALL.DPDK.md @@ -430,6 +430,7 @@ qemu-wrap.py -cpu host -boot c -hda -m 4096 -smp 4 --enable-kvm -nographic -vnc none -net none -netdev tap,id=net1, script=no,downscript=no,ifname=if1,vhost=on -device virtio-net-pci, netdev=net1,mac=00:00:00:00:00:01 +``` DPDK vhost VM configuration with libvirt: - -- 2.1.0 ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] Cloud Expo Europe 2015
Hi, As a database provider, I was trying to check if you would be interested in owning a list available of companies with virtualized environments that can be used for your marketing campaigns? These lists can be customized based on geography, titles and more. Some of the lists are categorized into: * VMware users * Citrix Users * MS Hyper V users * RedHat users * DELL users * IBM users and more We have some authentic data of other Virtualization, CRM, ERP, PLM, BI software users too; we can help you if you have a similar niche market to target. Let me know if you would be interested in seeing a sample and getting more info on our authentic lists. Do you need anything custom to reach your prospects? We can help you virtually with anything on the data front, be it industry specific, title specific and any such parameters. Reach out with your specific requirement and get a set of free samples. If you are not the right person to discuss this please forward this email to the right person in your organization. I look forward to hearing from you. Kind Regards, Jennifer Hall Business Coordinator Target Biz Solutions 75 Fifth Street NW Suite 1100 Atlanta, GA 30308 We respect your privacy. If you don't wish to receive an email from us, please send a reply with an email subject line as "Leave out". ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH] INSTALL.DPDK.md: Terminate code section.
On Mon, Mar 23, 2015 at 01:49:10PM -0400, Russell Bryant wrote: > Add a missing terminator for a code section. Without this, the > rendering on github at least shows the rest of the file as a code > block. > > Signed-off-by: Russell Bryant Thanks! Applied to master. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH 2/2] datapath: Add Stateless TCP Tunneling protocol.
On Mon, Mar 9, 2015 at 3:12 PM, Pravin B Shelar wrote: > openvswitch_headers = \ > diff --git a/datapath/linux/.gitignore b/datapath/linux/.gitignore > index 69d6658..1d1aae7 100644 > --- a/datapath/linux/.gitignore > +++ b/datapath/linux/.gitignore > @@ -50,6 +50,7 @@ > /vport-lisp.c > /vport-netdev.c > /vport-patch.c > +/vport-stt.c > /vport-vxlan.c > /vport.c > /vxlan.c Should we add stt.c as well? > diff --git a/datapath/linux/compat/stt.c b/datapath/linux/compat/stt.c > new file mode 100644 > index 000..4cb4ea6 > --- /dev/null > +++ b/datapath/linux/compat/stt.c > +static int skb_list_linearize(struct sk_buff *head, gfp_t gfp_mask) > +{ > + struct sk_buff *skb; > + int tlen = 0; > + int err; > + > + err = skb_linearize(head); > + if (err) > + return err; > + > + skb = head->next; > + while (skb) { > + tlen += skb->len; > + skb = skb->next; > + } > + err = pskb_expand_head(head, 0, tlen, gfp_mask); > + if (err) > + return err; Were you going to try to avoid linearizing and then expanding as Alex had mentioned in his comments? > +static int __build_segments(struct sk_buff **headp, bool ipv4, int l4_offset) > +{ > + struct sk_buff *head = *headp; > + struct sk_buff *nskb = NULL; > + struct sk_buff *rskb, *skb; > + struct tcphdr *tcph; > + int seg_len = 0; > + int hdr_len; > + int tcp_len; > + u32 seq; > + > + /* GRO can produce skbs with only the headers, which we've > +* already pulled off. We can just dump them. > +*/ > + while (head->len == 0) { > + nskb = head->next; > + copy_skb_metadata(nskb, head); > + consume_skb(head); > + head = nskb; > + } > + *headp = head; > + tcph = (struct tcphdr *)(head->data + l4_offset); > + tcp_len = tcph->doff * 4; > + hdr_len = l4_offset + tcp_len; I think there is something wrong with the ordering of the calls in stt_rcv(). reassemble() will just produce a linked list of skbs that are expected to be joined here but we've already called functions that do pskb_may_pull(), which won't traverse that list. In particular, it doesn't make sense to both remove zero length skbs above and expect that there is a TCP header available below. > + > + if (unlikely((tcp_len < sizeof(struct tcphdr)) || > +(head->len < hdr_len))) > + return -EINVAL; > + > + if (unlikely(!pskb_may_pull(head, hdr_len))) > + return -ENOMEM; It seems risky to me to combine skb coalescing with TCP header updating as we would need to very carefully check boundary conditions. I feel like there are two halves of the loop anyways, so it seems better to just break them apart. > +static int __segment_skb(struct sk_buff **headp, bool ipv4, bool tcp, bool > csum_partial, int l4_offset) > +{ > + int err; > + > + err = straighten_frag_list(headp); > + if (unlikely(err)) > + return err; > + > + if (__linearize(*headp, ipv4, tcp, csum_partial)) { > + return skb_list_linearize(*headp, GFP_ATOMIC); > + } > + > + if ((*headp)->next) { I think we can push this check up above linearization. If there is no list of skbs, we don't need to linearize either. > +static int segment_skb(struct sk_buff **headp) > +{ Isn't there a check for frag_list for the receive path missing somewhere? It seems like linearization happens somewhat unconditionally. > +static int skb_list_xmit(struct rtable *rt, struct sk_buff *skb, __be32 src, > +__be32 dst, __u8 tos, __u8 ttl, __be16 df) > +{ > + int len = 0; > + > + while (skb) { > + struct sk_buff *next = skb->next; > + > + if (next) > + dst_clone(&rt->dst); > + > + skb->next = NULL; > + len += iptunnel_xmit(NULL, rt, skb, src, dst, IPPROTO_TCP, > +tos, ttl, df, false); I think there may be a problem in our version of ip_local_out() if it thinks that fix_segment is set in the CB, so we need to find a way to make sure that it is cleared. > +int stt_xmit_skb(struct sk_buff *skb, struct rtable *rt, > +__be32 src, __be32 dst, __u8 tos, > +__u8 ttl, __be16 df, __be16 src_port, __be16 dst_port, > +__be64 tun_id) [...] > + while (skb) { > + struct sk_buff *next_skb = skb->next; > + > + skb->next = NULL; > + > + if (next_skb) > + dst_clone(&rt->dst); > + > + /* Push STT and TCP header. */ > + skb = push_stt_header(skb, tun_id, src_port, dst_port, src, > + dst, inner_h_proto, inner_nw_proto, > + dst_mtu(&rt->dst)); > +
Re: [ovs-dev] question about revalidation
On Wed, Mar 18, 2015 at 06:30:17PM +, Gray, Mark D wrote: > In udpif_revalidator(), the leader thread seems to adjust the datapath > flow_limit > in response to duration (which is the time taken to revalidate). It does this > with > some heuristic calculations that I have reproduced below. Not sure exactly > why it > is doing this or how the heuristics work as I would have assumed the > flow_limit would > be independent of revalidation time. Can anyone shed some light on that for > me? > > if (duration > 2000) { > flow_limit /= duration / 1000; > } else if (duration > 1300) { > flow_limit = flow_limit * 3 / 4; > } else if (duration < 1000 && n_flows > 2000 >&& flow_limit < n_flows * 1000 / duration) { > flow_limit += 1000; > } The number of flows is limited so that OVS can be confident that it can revalidate every flow within a bounded amount of time (I think the goal is 1 second). ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH] auto-attach: Support latest version of auto-attach LLDP TLVs
On Wed, Mar 18, 2015 at 02:47:14PM -0400, drfl...@avaya.com wrote: > From: Dennis Flynn > > The following enhancements to the auto-attach feature are provided > > - Support recent modifications to the AA element discovery TLV > - Support recent Avaya Organizationally Unique ID (OUI) change. > (This will change to IEEE assigned OUI once AA standard has been ratified) > - Remove some Avaya specific #defines > > The primary purpose of this commit is to catch up with the latest changes made > to the auto attach TLVs as the Auto Attach feature progresses through the > 802.1Q IEEE standards committee. Most notably this includes some minor rework > of the AA element discovery TLV and a recent change to the Avaya OUI value. > > Signed-off-by: Dennis Flynn Thank you for the patch! I am very happy to see this code being maintained. I applied this to master. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH] netdev-linux: Support for SFQ, FQ_CoDel and CoDel qdiscs.
On Wed, Mar 18, 2015 at 05:13:01PM +0100, Jonathan Vestin wrote: > This patch adds support for SFQ, CoDel and FQ_CoDel classless qdiscs to Open > vSwitch. It also removes the requirement for a QoS to have at least one Queue > (as this makes no sense when using classless qdiscs). I have also not > implemented class_{get,set,delete,get_stats,dump_stats} because they are > meant for qdiscs with classes. > > Signed-off-by: Jonathan Vestin Thanks for the contribution! I applied this to master. I folded in the following incremental changes. Most of them are purely stylistic (including a lot of removal of trailing white space) but you might want to notice the nl_msg_put_u32() and nl_attr_get_u32() functions for next time. Thanks again! diff --git a/AUTHORS b/AUTHORS index db4520f..8fba915 100644 --- a/AUTHORS +++ b/AUTHORS @@ -85,6 +85,7 @@ Jesse Gross je...@nicira.com Jing Ai ji...@google.com Joe Perches j...@perches.com Joe Stringerjoestrin...@nicira.com +Jonathan Vestin jonav...@kau.se Jun Nakajimajun.nakaj...@intel.com Justin Pettit jpet...@nicira.com Keith Amidonke...@nicira.com @@ -268,7 +269,6 @@ Joan Cirer j...@ev0.net John Darrington j...@darrington.wattle.id.au John Galgay j...@galgay.net John Hurley john.hur...@netronome.com -Jonathan Vestin jonav...@kau.se K ???k940...@hotmail.com Kevin Mancuso kevin.manc...@rackspace.com Kiran Shanbhog ki...@vmware.com diff --git a/lib/netdev-linux.c b/lib/netdev-linux.c index 65f2555..8253dfb 100644 --- a/lib/netdev-linux.c +++ b/lib/netdev-linux.c @@ -2856,7 +2856,7 @@ codel_get__(const struct netdev *netdev_) } static void -codel_install__(struct netdev *netdev_, uint32_t target, uint32_t limit, +codel_install__(struct netdev *netdev_, uint32_t target, uint32_t limit, uint32_t interval) { struct netdev_linux *netdev = netdev_linux_cast(netdev_); @@ -2872,7 +2872,7 @@ codel_install__(struct netdev *netdev_, uint32_t target, uint32_t limit, } static int -codel_setup_qdisc__(struct netdev *netdev, uint32_t target, uint32_t limit, +codel_setup_qdisc__(struct netdev *netdev, uint32_t target, uint32_t limit, uint32_t interval) { size_t opt_offset; @@ -2897,18 +2897,17 @@ codel_setup_qdisc__(struct netdev *netdev, uint32_t target, uint32_t limit, nl_msg_put_string(&request, TCA_KIND, "codel"); opt_offset = nl_msg_start_nested(&request, TCA_OPTIONS); -nl_msg_put_unspec(&request, TCA_CODEL_TARGET, &otarget, sizeof otarget); -nl_msg_put_unspec(&request, TCA_CODEL_LIMIT, &olimit, sizeof olimit); -nl_msg_put_unspec(&request, TCA_CODEL_INTERVAL, &ointerval, - sizeof ointerval); +nl_msg_put_u32(&request, TCA_CODEL_TARGET, otarget); +nl_msg_put_u32(&request, TCA_CODEL_LIMIT, olimit); +nl_msg_put_u32(&request, TCA_CODEL_INTERVAL, ointerval); nl_msg_end_nested(&request, opt_offset); error = tc_transact(&request, NULL); if (error) { VLOG_WARN_RL(&rl, "failed to replace %s qdisc, " -"target %u, limit %u, interval %u error %d(%s)", +"target %u, limit %u, interval %u error %d(%s)", netdev_get_name(netdev), -otarget, olimit, ointerval, +otarget, olimit, ointerval, error, ovs_strerror(error)); } return error; @@ -2930,9 +2929,15 @@ codel_parse_qdisc_details__(struct netdev *netdev OVS_UNUSED, codel->limit = limit_s ? strtoull(limit_s, NULL, 10) : 0; codel->interval = interval_s ? strtoull(interval_s, NULL, 10) : 0; -if (!codel->target) codel->target = 5000; -if (!codel->limit) codel->limit = 10240; -if (!codel->interval) codel->interval = 10; +if (!codel->target) { +codel->target = 5000; +} +if (!codel->limit) { +codel->limit = 10240; +} +if (!codel->interval) { +codel->interval = 10; +} } static int @@ -2942,7 +2947,7 @@ codel_tc_install(struct netdev *netdev, const struct smap *details) struct codel codel; codel_parse_qdisc_details__(netdev, details, &codel); -error = codel_setup_qdisc__(netdev, codel.target, codel.limit, +error = codel_setup_qdisc__(netdev, codel.target, codel.limit, codel.interval); if (!error) { codel_install__(netdev, codel.target, codel.limit, codel.interval); @@ -2967,9 +2972,9 @@ codel_parse_tca_options__(struct nlattr *nl_options, struct codel *codel) return EPROTO; } -codel->target = *((uint32_t *)nl_attr_get(attrs[TCA_CODEL_TARGET])); -codel->limit = *((uint32_t *)nl_attr_get(attrs[TCA_CODEL_LIMIT])); -codel->interval = *((uint32_t *)nl_attr_get(attrs[TCA_CODEL_INTERVAL])); +codel->target = nl_attr_get_u32(attrs[TCA_CODEL_TARGET]); +codel->limit = nl_attr_get_
[ovs-dev] OVN - OVN Northbound DB and ovn-nbd question
I apologize if the 'ASCII Draw' does not appear correctly in your email client. Outlook seems to be changing it.. Hello All, I wonder if you could help me understand the OVN architecture and specifically, how we might implement this in OpenStack or any other CMS.. The XML document has the following ASCII-Draw picture: The implication in the dotted box below the CMS is that the CMS Plugin, OVN NorthBound DB, and the ovn-nbd are all part of the same group or module. In other words, for OpenStack, this suggests the solution would contain an OpenStack specific Plugin (of course), OVN Northbound DB and ovn-ndb (perhaps an agent??). CMS | | +---|---+ | | | | OVN/CMS Plugin| | | | | | | | OVN Northbound DB | | | | | | | |ovn-nbd| | | | +---|---+ | | +--+ |OVN DB| +--+ | | +--+--+ | | | HV 1 | |HV n | +---|---+ . +---|---+ | | | . | | | |ovn-controller | . |ovn-controller | | | | | . | | | | | | | | | | | | | ovs-vswitchd ovsdb-server | | ovs-vswitchd ovsdb-server | | | | | +---+ +---+ However, the OVN Northbound DB is not the CMS DB ( Neutron DB in the case of OpenStack), it is a Schema defined by OVN. This suggests that the OVN Northbound DB and ovn-nbd is CMS independent and should be outside the CMS implementation. The OVN Northbound DB and the Central OVN DB could be in the same 'System' (or cluster or systems). CMS | | +---|---+ | | | | OVN/CMS Plugin| +---|---+ | | | +---|---+ | | | | | | | OVN Northbound DB | | | | | | | |ovn-nbd| | | | +---|---+ : | : : | : +--+ |OVN DB| +--+ | | +--+--+ | | | HV 1| |HV n | +---|---+ . +---|---+ | | | . | | | |ovn-controller | . |ovn-controller | | | | | . | | | | | | | | | | | | | ovs-vswitchd ovsdb-server | | ovs-vswitchd ovsdb-server | | | | | +---+ +---+ Above is the split that I am talking about. The OVN DB could still be separate as it will likely have different HA requirements than the OVN Northbound (assuming the OVN Northbound could be recreated from the CMS DB). The above starts to look a bit more like a 'Controller' architecture where something like ODL or another SDN Controller could host the OVN N
[ovs-dev] [PATCH] Updated spec file - fixed RPM build errors: Installed (but unpackaged) file(s) found.
On exec rpmbuild -ba openvswitch-fedora.spec displayed: error: Installed (but unpackaged) file(s) found: /etc/bash_completion.d/ovs-appctl-bashcomp.bash /etc/bash_completion.d/ovs-vsctl-bashcomp.bash RPM build errors: Installed (but unpackaged) file(s) found: /etc/bash_completion.d/ovs-appctl-bashcomp.bash /etc/bash_completion.d/ovs-vsctl-bashcomp.bash /etc/bash_completion.d/ovs-appctl-bashcomp.bash and /etc/bash_completion.d/ovs-vsctl-bashcomp.bash added to files list. Signed-off-by: Oleg Gashev Acked-by: Alex Wang --- rhel/openvswitch-fedora.spec.in | 2 ++ 1 file changed, 2 insertions(+) diff --git a/rhel/openvswitch-fedora.spec.in b/rhel/openvswitch-fedora.spec.in index 5a3af4a..4889321 100644 --- a/rhel/openvswitch-fedora.spec.in +++ b/rhel/openvswitch-fedora.spec.in @@ -200,6 +200,8 @@ rm -rf $RPM_BUILD_ROOT %files %defattr(-,root,root) +%{_sysconfdir}/bash_completion.d/ovs-appctl-bashcomp.bash +%{_sysconfdir}/bash_completion.d/ovs-vsctl-bashcomp.bash %dir %{_sysconfdir}/openvswitch %config %ghost %{_sysconfdir}/openvswitch/conf.db %config %ghost %{_sysconfdir}/openvswitch/system-id.conf -- 1.9.3 ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] OVN - OVN Northbound DB and ovn-nbd question
I snipped the ascii diagrams, as they were pretty badly munged when I tried to reply ... On 03/23/2015 05:22 PM, Millward, Scott T (HP Networking / CEB- Roseville) wrote: > I wonder if you could help me understand the OVN architecture and > specifically, how we might implement this in OpenStack or any other > CMS.. > > > > The XML document has the following ASCII-Draw picture: > > > > The implication in the dotted box below the CMS is that the CMS > Plugin, OVN NorthBound DB, and the ovn-nbd are all part of the same > group or module. In other words, for OpenStack, this suggests the > solution would contain an OpenStack specific Plugin (of course), OVN > Northbound DB and ovn-ndb (perhaps an agent??). > > > However, the OVN Northbound DB is not the CMS DB ( Neutron DB in the > case of OpenStack), it is a Schema defined by OVN. This suggests > that the OVN Northbound DB and ovn-nbd is CMS independent and should > be outside the CMS implementation. > > > > The OVN Northbound DB and the Central OVN DB could be in the same > 'System' (or cluster or systems). > > > Above is the split that I am talking about. The OVN DB could still > be separate as it will likely have different HA requirements than the > OVN Northbound (assuming the OVN Northbound could be recreated from > the CMS DB). > > > > The above starts to look a bit more like a 'Controller' architecture > where something like ODL or another SDN Controller could host the OVN > Northbound DB and the ovn-nbd. This also means that assuming the > interface between the CMS (and its plugin architecture) is OVSDB > protocol to the OVN Northbound DB, only the CMS and its plugin would > need to be CMS Specific. What you suggest here is the intention. The OpenStack plugin only has to implement communication to the ovn northbound DB. The rest is general OVN code. The OVN northbound db is just a db served by an instance of ovsdb-server, and ovn-nbd is a new daemon to be implemented for OVN. The Neutron plugin has been started here: http://git.openstack.org/cgit/stackforge/networking-ovn/tree/ Right now the code has started using the 'ovn-nbctl' utility to talk to the northbound DB. That's only intended to be the "make it work ASAP" approach. We'd like to make it talk to ovsdb natively. Some very similar work was just finished in Neutron, so we'll be taking advantage of it. -- Russell Bryant ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH] datapath-windows: Updated WFP system provider handling
Hi Eitan, In this case the second call will enter this code path: status = FwpmProviderAdd(handle, &provider, NULL); if (!NT_SUCCESS(status)) { -OVS_LOG_ERROR("Fail to add WFP provider, status: %x.", status); -break; +if (STATUS_FWP_ALREADY_EXISTS != status) { +OVS_LOG_ERROR("Failed to add WFP provider, status: %x.", + status); +break; +} } Thanks, Sorin -Original Message- From: Eitan Eliahu [mailto:elia...@vmware.com] Sent: Monday, 23 March, 2015 18:13 To: Sorin Vinturis Cc: dev@openvswitch.org Subject: RE: [ovs-dev] [PATCH] datapath-windows: Updated WFP system provider handling Sorin, Ok on [1]. Thanks. On [2], I can see a scenario when the provider can added twice from OvsRegisterSystemProvider() as well as from the callback if (FWPM_SERVICE_RUNNING == FwpmBfeStateGet()) { > Callback is called from a different thread context ! OvsTunnelEngineOpen(&handle); How do you prevent it? Also, can you please explain which issue you ran into which triggered this change? Thank you, Eitan -Original Message- From: Sorin Vinturis [mailto:svintu...@cloudbasesolutions.com] Sent: Monday, March 23, 2015 1:37 AM To: Eitan Eliahu Cc: dev@openvswitch.org Subject: RE: [ovs-dev] [PATCH] datapath-windows: Updated WFP system provider handling Hi Eitan, Thanks for reviewing my patch. [1] The OvsTunnelEngineClose(HANDLE *handle) is verifying the handle it receives before closing it. [2] In order to retrieve the current state of the filter engine I am using the FwpmBfeStateGet function. But before calling FwpmBfeStateGet, the callout driver MUST call the FwpmBfeStateSubscribeChanges function. That is the reason I am registering the callback no mater of the state of the filter engine. Only if the engine is running the provider is added by opening the engine handle first and closing it after the provider is added, then unsubscribes the callback. If the engine is not running the callback is called and the provider is added only for the service running notification. Hope this answers your comments. Thanks, Sorin -Original Message- From: Eitan Eliahu [mailto:elia...@vmware.com] Sent: Monday, 23 March, 2015 02:32 To: Sorin Vinturis Cc: dev@openvswitch.org Subject: RE: [ovs-dev] [PATCH] datapath-windows: Updated WFP system provider handling Hi Sorin, Thank you for posting this change. Here are few comments: [1] Please call OvsTunnelEngineClose(&handle) only if the handle is not zero. (you might want to add an assertion if it zero). [2] I'm somewhat confused about the registration for engine state change. As I understand you check the state of the engine and if it is running you open the engine handle (in the context of the foreground thread). However, you subscribe to state change anyway and you open the handle to the engine from the callback function too. Did I miss anything? Thanks, Eitan -Original Message- From: dev [mailto:dev-boun...@openvswitch.org] On Behalf Of Sorin Vinturis Sent: Monday, 16 March, 2015 20:08 To: dev@openvswitch.org Subject: [ovs-dev] [PATCH] datapath-windows: Updated WFP system provider handling If the Base Filtering Engine (BFE) is not started, the WFP system provider failed to be added because no session to the engine could be acquired. The solution for this was to registered a BFE notification callback that is called whenever the BFE's state changes. Only if the BFE's state is running the WFP system provider is added. Signed-off-by: Sorin Vinturis Reported-by: Sorin Vinturis Reported-at: https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_openvswitch_ovs-2Dissues_issues_65&d=AwIGaQ&c=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs&r=CWsgHUxi6ExLXY798tmo3LJ4e3geGYp56lkcH-5cLCY&m=MVriWGIsZGTQ5f_haLS0X5H8R3YdfikbqnWKRpKksKU&s=1iWqFul95i3o_mD41jKjZNdLnK-df_QWRzSY1614RTw&e= --- datapath-windows/ovsext/Datapath.c | 20 ++- datapath-windows/ovsext/TunnelFilter.c | 99 -- datapath-windows/ovsext/TunnelIntf.h | 8 +-- 3 files changed, 100 insertions(+), 27 deletions(-) diff --git a/datapath-windows/ovsext/Datapath.c b/datapath-windows/ovsext/Datapath.c index 8eb13f1..c6fe89e 100644 --- a/datapath-windows/ovsext/Datapath.c +++ b/datapath-windows/ovsext/Datapath.c @@ -353,35 +353,19 @@ PNDIS_SPIN_LOCK gOvsCtrlLock; VOID OvsInit() { -HANDLE handle = NULL; - gOvsCtrlLock = &ovsCtrlLockObj; NdisAllocateSpinLock(gOvsCtrlLock); OvsInitEventQueue(); - -OvsTunnelEngineOpen(&handle); -if (handle) { -OvsTunnelAddSystemProvider(handle); -} -OvsTunnelEngineClose(&handle); } VOID OvsCleanup() { -HANDLE handle = NULL; - OvsCleanupEventQueue(); if (gOvsCtrlLock)
Re: [ovs-dev] [PATCH] datapath-windows: Updated WFP system provider handling
As I explained in the patch description, I have added this change because, at driver initialization phase, the system provider failed to be added due to the fact that the Base Filtering Engine (BFE) is not started and no session to the engine could be acquired, i.e. OvsTunnelEngineOpen() fails. -Original Message- From: dev [mailto:dev-boun...@openvswitch.org] On Behalf Of Sorin Vinturis Sent: Tuesday, 24 March, 2015 01:01 To: Eitan Eliahu Cc: dev@openvswitch.org Subject: Re: [ovs-dev] [PATCH] datapath-windows: Updated WFP system provider handling Hi Eitan, In this case the second call will enter this code path: status = FwpmProviderAdd(handle, &provider, NULL); if (!NT_SUCCESS(status)) { -OVS_LOG_ERROR("Fail to add WFP provider, status: %x.", status); -break; +if (STATUS_FWP_ALREADY_EXISTS != status) { +OVS_LOG_ERROR("Failed to add WFP provider, status: %x.", + status); +break; +} } Thanks, Sorin -Original Message- From: Eitan Eliahu [mailto:elia...@vmware.com] Sent: Monday, 23 March, 2015 18:13 To: Sorin Vinturis Cc: dev@openvswitch.org Subject: RE: [ovs-dev] [PATCH] datapath-windows: Updated WFP system provider handling Sorin, Ok on [1]. Thanks. On [2], I can see a scenario when the provider can added twice from OvsRegisterSystemProvider() as well as from the callback if (FWPM_SERVICE_RUNNING == FwpmBfeStateGet()) { > Callback is called from a different thread context ! OvsTunnelEngineOpen(&handle); How do you prevent it? Also, can you please explain which issue you ran into which triggered this change? Thank you, Eitan -Original Message- From: Sorin Vinturis [mailto:svintu...@cloudbasesolutions.com] Sent: Monday, March 23, 2015 1:37 AM To: Eitan Eliahu Cc: dev@openvswitch.org Subject: RE: [ovs-dev] [PATCH] datapath-windows: Updated WFP system provider handling Hi Eitan, Thanks for reviewing my patch. [1] The OvsTunnelEngineClose(HANDLE *handle) is verifying the handle it receives before closing it. [2] In order to retrieve the current state of the filter engine I am using the FwpmBfeStateGet function. But before calling FwpmBfeStateGet, the callout driver MUST call the FwpmBfeStateSubscribeChanges function. That is the reason I am registering the callback no mater of the state of the filter engine. Only if the engine is running the provider is added by opening the engine handle first and closing it after the provider is added, then unsubscribes the callback. If the engine is not running the callback is called and the provider is added only for the service running notification. Hope this answers your comments. Thanks, Sorin -Original Message- From: Eitan Eliahu [mailto:elia...@vmware.com] Sent: Monday, 23 March, 2015 02:32 To: Sorin Vinturis Cc: dev@openvswitch.org Subject: RE: [ovs-dev] [PATCH] datapath-windows: Updated WFP system provider handling Hi Sorin, Thank you for posting this change. Here are few comments: [1] Please call OvsTunnelEngineClose(&handle) only if the handle is not zero. (you might want to add an assertion if it zero). [2] I'm somewhat confused about the registration for engine state change. As I understand you check the state of the engine and if it is running you open the engine handle (in the context of the foreground thread). However, you subscribe to state change anyway and you open the handle to the engine from the callback function too. Did I miss anything? Thanks, Eitan -Original Message- From: dev [mailto:dev-boun...@openvswitch.org] On Behalf Of Sorin Vinturis Sent: Monday, 16 March, 2015 20:08 To: dev@openvswitch.org Subject: [ovs-dev] [PATCH] datapath-windows: Updated WFP system provider handling If the Base Filtering Engine (BFE) is not started, the WFP system provider failed to be added because no session to the engine could be acquired. The solution for this was to registered a BFE notification callback that is called whenever the BFE's state changes. Only if the BFE's state is running the WFP system provider is added. Signed-off-by: Sorin Vinturis Reported-by: Sorin Vinturis Reported-at: https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_openvswitch_ovs-2Dissues_issues_65&d=AwIGaQ&c=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs&r=CWsgHUxi6ExLXY798tmo3LJ4e3geGYp56lkcH-5cLCY&m=MVriWGIsZGTQ5f_haLS0X5H8R3YdfikbqnWKRpKksKU&s=1iWqFul95i3o_mD41jKjZNdLnK-df_QWRzSY1614RTw&e= --- datapath-windows/ovsext/Datapath.c | 20 ++- datapath-windows/ovsext/TunnelFilter.c | 99 -- datapath-windows/ovsext/TunnelIntf.h | 8 +-- 3 files changed, 100 insertions(+), 27 deletions(-) diff --git a/datapath-windows/ovsext/Datapath.c b/datapath-windows/ovsext/Datapath.c index 8eb13f1..c6fe89e 10064
[ovs-dev] [PATCH] datapath-windows Release lock on HVUpdateNIC
Quick fix to release the dispatch lock when the vport could not be found. Signed-off-by: Alin Gabriel Serdean --- datapath-windows/ovsext/Vport.c | 1 + 1 file changed, 1 insertion(+) diff --git a/datapath-windows/ovsext/Vport.c b/datapath-windows/ovsext/Vport.c index c9dfaea..70bef3d 100644 --- a/datapath-windows/ovsext/Vport.c +++ b/datapath-windows/ovsext/Vport.c @@ -405,6 +405,7 @@ HvUpdateNic(POVS_SWITCH_CONTEXT switchContext, nicParam->NicIndex); if (vport == NULL) { OVS_LOG_WARN("Vport search failed."); +NdisReleaseRWLock(switchContext->dispatchLock, &lockState); goto update_nic_done; } switch (nicParam->NicType) { -- 1.9.5.msysgit.0 ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH] datapath-windows: Updated WFP system provider handling
Thanks Sorin, What would be the case where BFE is not started before the driver initializes? When BFE is not started does the driver continue with WFP rule setiings? Eitan -Original Message- From: Sorin Vinturis [mailto:svintu...@cloudbasesolutions.com] Sent: Monday, March 23, 2015 4:40 PM To: Sorin Vinturis; Eitan Eliahu Cc: dev@openvswitch.org Subject: RE: [ovs-dev] [PATCH] datapath-windows: Updated WFP system provider handling As I explained in the patch description, I have added this change because, at driver initialization phase, the system provider failed to be added due to the fact that the Base Filtering Engine (BFE) is not started and no session to the engine could be acquired, i.e. OvsTunnelEngineOpen() fails. -Original Message- From: dev [mailto:dev-boun...@openvswitch.org] On Behalf Of Sorin Vinturis Sent: Tuesday, 24 March, 2015 01:01 To: Eitan Eliahu Cc: dev@openvswitch.org Subject: Re: [ovs-dev] [PATCH] datapath-windows: Updated WFP system provider handling Hi Eitan, In this case the second call will enter this code path: status = FwpmProviderAdd(handle, &provider, NULL); if (!NT_SUCCESS(status)) { -OVS_LOG_ERROR("Fail to add WFP provider, status: %x.", status); -break; +if (STATUS_FWP_ALREADY_EXISTS != status) { +OVS_LOG_ERROR("Failed to add WFP provider, status: %x.", + status); +break; +} } Thanks, Sorin -Original Message- From: Eitan Eliahu [mailto:elia...@vmware.com] Sent: Monday, 23 March, 2015 18:13 To: Sorin Vinturis Cc: dev@openvswitch.org Subject: RE: [ovs-dev] [PATCH] datapath-windows: Updated WFP system provider handling Sorin, Ok on [1]. Thanks. On [2], I can see a scenario when the provider can added twice from OvsRegisterSystemProvider() as well as from the callback if (FWPM_SERVICE_RUNNING == FwpmBfeStateGet()) { > Callback is called from a different thread context ! OvsTunnelEngineOpen(&handle); How do you prevent it? Also, can you please explain which issue you ran into which triggered this change? Thank you, Eitan -Original Message- From: Sorin Vinturis [mailto:svintu...@cloudbasesolutions.com] Sent: Monday, March 23, 2015 1:37 AM To: Eitan Eliahu Cc: dev@openvswitch.org Subject: RE: [ovs-dev] [PATCH] datapath-windows: Updated WFP system provider handling Hi Eitan, Thanks for reviewing my patch. [1] The OvsTunnelEngineClose(HANDLE *handle) is verifying the handle it receives before closing it. [2] In order to retrieve the current state of the filter engine I am using the FwpmBfeStateGet function. But before calling FwpmBfeStateGet, the callout driver MUST call the FwpmBfeStateSubscribeChanges function. That is the reason I am registering the callback no mater of the state of the filter engine. Only if the engine is running the provider is added by opening the engine handle first and closing it after the provider is added, then unsubscribes the callback. If the engine is not running the callback is called and the provider is added only for the service running notification. Hope this answers your comments. Thanks, Sorin -Original Message- From: Eitan Eliahu [mailto:elia...@vmware.com] Sent: Monday, 23 March, 2015 02:32 To: Sorin Vinturis Cc: dev@openvswitch.org Subject: RE: [ovs-dev] [PATCH] datapath-windows: Updated WFP system provider handling Hi Sorin, Thank you for posting this change. Here are few comments: [1] Please call OvsTunnelEngineClose(&handle) only if the handle is not zero. (you might want to add an assertion if it zero). [2] I'm somewhat confused about the registration for engine state change. As I understand you check the state of the engine and if it is running you open the engine handle (in the context of the foreground thread). However, you subscribe to state change anyway and you open the handle to the engine from the callback function too. Did I miss anything? Thanks, Eitan -Original Message- From: dev [mailto:dev-boun...@openvswitch.org] On Behalf Of Sorin Vinturis Sent: Monday, 16 March, 2015 20:08 To: dev@openvswitch.org Subject: [ovs-dev] [PATCH] datapath-windows: Updated WFP system provider handling If the Base Filtering Engine (BFE) is not started, the WFP system provider failed to be added because no session to the engine could be acquired. The solution for this was to registered a BFE notification callback that is called whenever the BFE's state changes. Only if the BFE's state is running the WFP system provider is added. Signed-off-by: Sorin Vinturis Reported-by: Sorin Vinturis Reported-at: https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_openvswitch_ovs-2Dissues_issues_65
[ovs-dev] Returned mail: Data format error
Dear user of openvswitch.org, Your e-mail account was used to send a large amount of junk e-mail messages during this week. Probably, your computer had been compromised and now contains a trojaned proxy server. Please follow our instructions in order to keep your computer safe. Have a nice day, openvswitch.org support team. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev