Re: [ovs-dev] [PATCH v3 1/3] dpif-netdev: Refactor PMD performance into dpif-netdev-perf
Hi Billy, My patches frequently get corrupted by our email system. When I submit the v4 of the series adapted to the reverted dp_netdev_pmd_thread struct changes I will try out another method of sending them. For now, could you please try the attached patch files? For me they apply cleanly on today's master. Haven't tested them yet. I'll fix the white-space errors and line lengths issues in the next version. More answers in-line. Thanks, Jan > -Original Message- > From: O Mahony, Billy [mailto:billy.o.mah...@intel.com] > Sent: Friday, 08 December, 2017 18:25 > To: Jan Scheurich ; 'ovs-dev@openvswitch.org' > > Subject: RE: [ovs-dev] [PATCH v3 1/3] dpif-netdev: Refactor PMD performance > into dpif-netdev-perf > > Hi Jan, > > I had problems applying later patches in this series so just reviewing this > one for now. I tried several revisions to apply them. > > The second patch ([ovs-dev,v3,2/3] dpif-netdev: Detailed performance stats > for PMDs ) fails with > fatal: patch fragment without header at line 708: @@ -1073,6 +1155,12 @@ > dpif_netdev_init(void) > > Overall not only is user-visible output is clearer but the code is also more > consistent and easier to understand. > > I tested this patch by applying it to: 3728b3b Ben Pfaff 2017-11-20 Merge > branch 'dpdk_merge' of https://github.com/istokes/ovs into > HEAD > > These are the issues I did find: > 1. make check #1159 "ofproto-dpif patch ports" consistently fails for me with > this patch applied > > 2. ./utilities/checkpatch.py reports some line length issues: > == Checking > "dpif-netdev-perf/ovs-dev-v3-1-3-dpif-netdev-Refactor-PMD-performance-into-dpif-netdev-perf.patch" > == > ERROR: Too many signoffs; are you missing Co-authored-by lines? I'm still confused by the tagging rule. I guess I need to add a Co-authored-by: Darrell Ball line to comply, right? > WARNING: Line length is >79-characters long > #346 FILE: lib/dpif-netdev.c:544: > struct ovs_refcount ref_cnt;/* Every reference must be > refcount'ed. */ > > WARNING: Line has non-spaces leading whitespace > WARNING: Line has trailing whitespace > #347 FILE: lib/dpif-netdev.c:545: > > > WARNING: Line length is >79-characters long > #349 FILE: lib/dpif-netdev.c:547: > * XPS disabled for this netdev. All static_tx_qid's are unique and > less > > WARNING: Line has non-spaces leading whitespace > WARNING: Line has trailing whitespace > #352 FILE: lib/dpif-netdev.c:550: > > > WARNING: Line length is >79-characters long > WARNING: Line has trailing whitespace > #413 FILE: lib/dpif-netdev.c:610: > OVS_ALIGNED_VAR(CACHE_LINE_SIZE) struct pmd_perf_stats perf_stats; > > Lines checked: 976, Warnings: 8, Errors: 1 > > 3. Does the new 'pmd' arg to pmd-stats-show interfere with the existing [dp] > arg? > > sudo ./utilities/ovs-appctl dpif-netdev/pmd-stats-show -pmd 1 > netdev@dpif-netdev > "dpif-netdev/pmd-stats-show" command takes at most 2 arguments > ovs-appctl: ovs-vswitchd: server returned an error Good question. I'd say the two options should be mutually exclusive. Specifying a PMD should imply datapath netdev@dpif-netdev. I can make this clear in the usage: dpif-netdev/pmd-stats-show", "[-pmd core | dp]. Actually, I have never really understood the dp argument for these commands. Can there ever be more than one dpif-netdev datapath? I know that one must specify netdev@dpif-netdev if there also is a kernel datapath in OVS, but I still don't fully get why. > > Otherwise it looks like a really useful patch. And the remainder of the > series more so. > > Thanks, > Billy. > > > -Original Message- > > From: ovs-dev-boun...@openvswitch.org [mailto:ovs-dev- > > boun...@openvswitch.org] On Behalf Of Jan Scheurich > > Sent: Tuesday, November 21, 2017 12:38 AM > > To: 'ovs-dev@openvswitch.org' > > Subject: [ovs-dev] [PATCH v3 1/3] dpif-netdev: Refactor PMD performance into > > dpif-netdev-perf > > > > Add module dpif-netdev-perf to host all PMD performance-related data > > structures and functions in dpif-netdev. Refactor the PMD stats handling in > > dpif- > > netdev and delegate whatever possible into the new module, using clean > > interfaces to shield dpif-netdev from the implementation details. > > Accordingly, > > the all PMD statistics members are moved from the main struct > > dp_netdev_pmd_thread into a dedicated member of type struct pmd_perf_stats. > > > > Include Darrel's prior refactoring of PMD stats contained in [PATCH v5,2/3] > > dpif- > > netdev: Refactor some pmd stats: > > > > 1. The cycles per packet counts are now based on packets received rather > > than > > packet passes through the datapath. > > > > 2. Packet counters are now kept for packets received and packets > > recirculated. > > These are kept as separate counters for maintainability reasons. The cost of > > incrementing these counters is negligible. These new counters are also > > displayed to the user. > > > > 3. A displa
Re: [ovs-dev] [RFC PATCH V4 0/9] netdev-dpdk: support multi-segment mbufs
Thanks Sugesh,
I'll rebase today and send an updated version that addresses your comments.
Best,
Mark
>-Original Message-
>From: Chandran, Sugesh
>Sent: Friday, December 8, 2017 6:10 PM
>To: Kavanagh, Mark B ; d...@openvswitch.org;
>qiud...@chinac.com
>Cc: Stokes, Ian ; Loftus, Ciara
>; santosh.shu...@caviumnetworks.com
>Subject: RE: [ovs-dev][RFC PATCH V4 0/9] netdev-dpdk: support multi-segment
>mbufs
>
>Thank you Mark & Michael Qiu for this work.
>
>I had looked at the patch series and have some high level comments as inline.
>
>I would like to test these patch on some of the test scenarios.
>so please send a proper patch series once the DPDK17.11 support is added in
>OVS.
>
>Regards
>_Sugesh
>
>
>> -Original Message-
>> From: Kavanagh, Mark B
>> Sent: Friday, December 8, 2017 12:02 PM
>> To: d...@openvswitch.org; qiud...@chinac.com
>> Cc: Stokes, Ian ; Loftus, Ciara
>;
>> santosh.shu...@caviumnetworks.com; Chandran, Sugesh
>> ; Kavanagh, Mark B
>>
>> Subject: [ovs-dev][RFC PATCH V4 0/9] netdev-dpdk: support multi-segment
>> mbufs
>>
>> Overview
>>
>> This patchset introduces support for multi-segment mbufs to OvS-DPDK.
>> Multi-segment mbufs are typically used when the size of an mbuf is
>insufficient
>> to contain the entirety of a packet's data. Instead, the data is split
>across
>> numerous mbufs, each carrying a portion, or 'segment', of the packet data.
>> mbufs are chained via their 'next'
>> attribute (an mbuf pointer).
>>
>> Use Cases
>> =
>> i. Handling oversized (guest-originated) frames, which are marked
>> for hardware accelration/offload (TSO, for example).
>>
>> Packets which originate from a non-DPDK source may be marked for
>> offload; as such, they may be larger than the permitted ingress
>> interface's MTU, and may be stored in an oversized dp-packet. In
>> order to transmit such packets over a DPDK port, their contents
>> must be copied to a DPDK mbuf (via dpdk_do_tx_copy). However, in
>> its current implementation, that function only copies data into
>> a single mbuf; if the space available in the mbuf is exhausted,
>> but not all packet data has been copied, then it is lost.
>> Similarly, when cloning a DPDK mbuf, it must be considered
>> whether that mbuf contains multiple segments. Both issues are
>> resolved within this patchset.
>>
>> ii. Handling jumbo frames.
>>
>> While OvS already supports jumbo frames, it does so by increasing
>> mbuf size, such that the entirety of a jumbo frame may be handled
>> in a single mbuf. This is certainly the preferred, and most
>> performant approach (and remains the default). However, it places
>> high demands on system memory; multi-segment mbufs may be
>> prefereable for systems which are memory-constrained.
>>
>> Enabling multi-segment mbufs
>>
>> Multi-segment and single-segment mbufs are mutually exclusive, and the user
>> must decide on which approach to adopt on init. The introduction of a new
>> OVSDB field, 'dpdk-multi-seg-mbufs', facilitates this.
>>
>> This is a global boolean value, which determines how jumbo frames are
>> represented across all DPDK ports. In the absence of a user-supplied value,
>> 'dpdk-multi-seg-mbufs' defaults to false, i.e. multi-segment mbufs must be
>> explicitly enabled / single-segment mbufs remain the default.
>>
>> Setting the field is identical to setting existing DPDK-specific OVSDB
>> fields:
>>
>> ovs-vsctl set Open_vSwitch . other_config:dpdk-init=true
>> ovs-vsctl set Open_vSwitch . other_config:dpdk-lcore-mask=0x10
>> ovs-vsctl set Open_vSwitch . other_config:dpdk-socket-mem=4096,0
>> ==> ovs-vsctl set Open_vSwitch . other_config:dpdk-multi-seg-mbufs=true
>>
>>
>> Notes
>> =
>> This patchset was generated and built against:
>> - Current HEAD of OvS master[1] + DPDK v17.11 support patchset[2]
>> - DPDK v17.11
>>
>> [1] 159cc1f ("datapath-windows: Correct endianness for deleting zone.") [2]
>> https://mail.openvswitch.org/pipermail/ovs-dev/2017-December/341786.html
>>
>> ---
>>
>> v4: - restructure patchset
>> - account for 128B ARM cacheline when sizing mbufs
>>
>> Mark Kavanagh (5):
>> netdev-dpdk: fix mbuf sizing
>> dp-packet: fix reset_packet for multi-seg mbufs
>> dp-packet: fix put_uninit for multi-seg mbufs
>> dp-packet: init specific mbuf fields to 0
>> netdev-dpdk: support multi-segment jumbo frames
>>
>> Michael Qiu (4):
>> dp-packet: Fix data_len issue with multi-seg mbufs
>> dp-packet: copy mbuf info for packet copy
>> dp-packet: copy data from multi-seg. DPDK mbuf
>> netdev-dpdk: copy large packet to multi-seg. mbufs
>>
>> NEWS | 1 +
>> lib/dp-packet.c | 51 -
>> lib/dp-packet.h | 26 -
>> lib/dpdk.c | 7 +++
>> lib/netdev-dpdk.c| 156 ++---
>> --
>> lib/netdev-dpdk.h| 1 +
>> vswitchd/vswitch
Re: [ovs-dev] [RFC PATCH V4 9/9] netdev-dpdk: support multi-segment jumbo frames
>From: Chandran, Sugesh >Sent: Friday, December 8, 2017 6:05 PM >To: Kavanagh, Mark B ; d...@openvswitch.org; >qiud...@chinac.com >Cc: Stokes, Ian ; Loftus, Ciara >; santosh.shu...@caviumnetworks.com >Subject: RE: [ovs-dev][RFC PATCH V4 9/9] netdev-dpdk: support multi-segment >jumbo frames > >Hi Mark, > >For some reason, I could not apply this patch cleanly. Apologies for this Sugesh, I'll send an updated version soon. -Mark >I couldn't do much of testing on the feature as such. >Can you please send a proper Patch after rebase. > >Regards >_Sugesh > > >> -Original Message- >> From: Kavanagh, Mark B >> Sent: Friday, December 8, 2017 12:02 PM >> To: d...@openvswitch.org; qiud...@chinac.com >> Cc: Stokes, Ian ; Loftus, Ciara >; >> santosh.shu...@caviumnetworks.com; Chandran, Sugesh >> ; Kavanagh, Mark B >> >> Subject: [ovs-dev][RFC PATCH V4 9/9] netdev-dpdk: support multi-segment >> jumbo frames >> > >[Snip] >> 1.9.3 ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH V2] ofproto-dpif-xlate: Incorrect handling of errors in group action processing
As per OpenFlow v1.3 specification, when an action list contains a group
action a copy of the packet is passed to the group for processing by the
group. This means that if there is an error encountered during group
processing, only the copy of packet should be dropped, but subsequent
actions in the action list should be executed on the original packet.
Additionally, if the group type is "ALL", each action bucket of the group
should process a copy of the packet. If there is an error while processing
one bucket other buckets should still be processed.
Example 1:
table=0,in_port=tap0 actions=output:tap1,group:10,output:tap2
Even if any error is encountered while processing the group action, the
packet should still be forwarded to ports tap1 and tap2.
Example 2:
group_id=1,type=all,bucket=actions=output:tap1,bucket=actions=encap(eth)
Even if processing the action in the second bucket fails because the
packet already has an Ethernet header, the other copy of the packet should
still be processed by the first bucket and output to port tap1.
Currently the error handling in OVS does not comply with those rules. When
any group bucket execution fails the error is recorded in the so-called
"translation context" which is global for the processing of the original
packet. Once an error is recorded, OVS skips processing subsequent buckets
and installs a drop action in the datapath even if parts of the action list
were previously processed successfully.
This patch clears the error flag after any bucket of a group is executed.
This way the error encountered in processing any bucket of the group will
not impact other actions of action-list or other buckets of the group.
Errors which are system limits to protect translation from taking too long
time or too much space are not cleared. Instead drop action is installed
for them.
Signed-off-by: Vishal Deep Ajmera
Signed-off-by: Keshav Gupta
---
ofproto/ofproto-dpif-xlate.c | 17 +
1 file changed, 17 insertions(+)
diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index 9b3a2f2..7893c0e 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -4065,6 +4065,23 @@ xlate_group_bucket(struct xlate_ctx *ctx, struct
ofputil_bucket *bucket,
* the group bucket freezes translation, the actions after the group action
* must continue processing with the original, not the frozen packet! */
ctx->exit = false;
+
+/* Context error in a bucket should not impact processing of other buckets
+ * or actions. This is similar to cloning a packet for group buckets.
+ * There is no need to restore the error back to old value due to the fact
+ * that we actually processed group action which can happen only when there
+ * is no previous context error.
+ *
+ * Exception to above is errors which are system limits to protect
translation
+ * from running too long or occupy too much space. These errors should not
be
+ * masked. XLATE_RECURSION_TOO_DEEP, XLATE_TOO_MANY_RESUBMITS and
+ * XLATE_STACK_TOO_DEEP fall in this category.
+ */
+if (ctx->error == XLATE_TOO_MANY_MPLS_LABELS ||
+ctx->error == XLATE_UNSUPPORTED_PACKET_TYPE) {
+/* reset the error and continue processing other buckets */
+ctx->error = XLATE_OK;
+}
}
static void
--
1.9.1
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [RFC PATCH V4 9/9] netdev-dpdk: support multi-segment jumbo frames
>From: Kavanagh, Mark B >Sent: Monday, December 11, 2017 11:49 AM >To: Chandran, Sugesh ; d...@openvswitch.org; >qiud...@chinac.com >Cc: Stokes, Ian ; Loftus, Ciara >; santosh.shu...@caviumnetworks.com >Subject: RE: [ovs-dev][RFC PATCH V4 9/9] netdev-dpdk: support multi-segment >jumbo frames > >>From: Chandran, Sugesh >>Sent: Friday, December 8, 2017 6:05 PM >>To: Kavanagh, Mark B ; d...@openvswitch.org; >>qiud...@chinac.com >>Cc: Stokes, Ian ; Loftus, Ciara >>; santosh.shu...@caviumnetworks.com >>Subject: RE: [ovs-dev][RFC PATCH V4 9/9] netdev-dpdk: support multi-segment >>jumbo frames >> >>Hi Mark, >> >>For some reason, I could not apply this patch cleanly. > >Apologies for this Sugesh, I'll send an updated version soon. >-Mark Addendum: I think I know what happened Sugesh. This patchset was built on the DPDK v17.11 upgrade patchset (this was mentioned in the cover letter!) - did you apply the former in advance of applying the multi-segment patchset? Thanks again, Mark > >>I couldn't do much of testing on the feature as such. >>Can you please send a proper Patch after rebase. >> >>Regards >>_Sugesh >> >> >>> -Original Message- >>> From: Kavanagh, Mark B >>> Sent: Friday, December 8, 2017 12:02 PM >>> To: d...@openvswitch.org; qiud...@chinac.com >>> Cc: Stokes, Ian ; Loftus, Ciara >>; >>> santosh.shu...@caviumnetworks.com; Chandran, Sugesh >>> ; Kavanagh, Mark B >>> >>> Subject: [ovs-dev][RFC PATCH V4 9/9] netdev-dpdk: support multi-segment >>> jumbo frames >>> >> >>[Snip] >>> 1.9.3 ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v3 1/2] vswitchd: Document netdev-dpdk commands.
On 08.12.2017 21:18, Flavio Leitner wrote: > On Fri, Dec 08, 2017 at 06:38:10PM +0300, Ilya Maximets wrote: >> netdev-dpdk appctl commands added to man pages. >> >> Signed-off-by: Ilya Maximets >> Acked-by: Antonio Fischetti >> --- >> NEWS| 2 ++ >> lib/automake.mk | 1 + >> lib/netdev-dpdk-unixctl.man | 9 + >> manpages.mk | 2 ++ >> vswitchd/ovs-vswitchd.8.in | 1 + >> 5 files changed, 15 insertions(+) >> create mode 100644 lib/netdev-dpdk-unixctl.man >> >> diff --git a/NEWS b/NEWS >> index 188a075..e9fb279 100644 >> --- a/NEWS >> +++ b/NEWS >> @@ -15,6 +15,8 @@ Post-v2.8.0 >> * Add support for compiling OVS with the latest Linux 4.13 kernel >> - "flush-conntrack" in ovs-dpctl and ovs-appctl now accept a 5-tuple to >> delete a specific connection tracking entry. >> + - DPDK: >> + * All the netdev-dpdk appctl commands described in ovs-vswitchd man >> page. >> >> v2.8.0 - 31 Aug 2017 >> >> diff --git a/lib/automake.mk b/lib/automake.mk >> index effe5b5..4b38a11 100644 >> --- a/lib/automake.mk >> +++ b/lib/automake.mk >> @@ -465,6 +465,7 @@ MAN_FRAGMENTS += \ >> lib/db-ctl-base.man \ >> lib/dpctl.man \ >> lib/memory-unixctl.man \ >> +lib/netdev-dpdk-unixctl.man \ >> lib/ofp-version.man \ >> lib/ovs.tmac \ >> lib/service.man \ >> diff --git a/lib/netdev-dpdk-unixctl.man b/lib/netdev-dpdk-unixctl.man >> new file mode 100644 >> index 000..a4b7f60 >> --- /dev/null >> +++ b/lib/netdev-dpdk-unixctl.man >> @@ -0,0 +1,9 @@ >> +.SS "NETDEV-DPDK COMMANDS" >> +These commands manage DPDK related ports (\fItype=dpdk*\fR). >> +.IP "\fBnetdev-dpdk/set-admin-state\fR [\fIinterface\fR] \fIstate\fR" >> +Sets admin state for DPDK \fIinterface\fR (or all interfaces if none is >> given) >> +to \fIstate\fR. \fIstate\fR can be "up" or "down". >> +.IP "\fBnetdev-dpdk/detach\fR \fIpci-address\fR" >> +Detaches device with corresponding \fIpci-address\fR from DPDK. This >> command >> +can be used to detach device if it wasn't detached automatically after port >> +deletion. Refer to the documentation for details and instructions. >> diff --git a/manpages.mk b/manpages.mk >> index d610d88..c89bc45 100644 >> --- a/manpages.mk >> +++ b/manpages.mk >> @@ -279,6 +279,7 @@ vswitchd/ovs-vswitchd.8: \ >> lib/daemon.man \ >> lib/dpctl.man \ >> lib/memory-unixctl.man \ >> +lib/netdev-dpdk-unixctl.man \ >> lib/service.man \ >> lib/ssl-bootstrap.man \ >> lib/ssl.man \ >> @@ -296,6 +297,7 @@ lib/coverage-unixctl.man: >> lib/daemon.man: >> lib/dpctl.man: >> lib/memory-unixctl.man: >> +lib/netdev-dpdk-unixctl.man: >> lib/service.man: >> lib/ssl-bootstrap.man: >> lib/ssl.man: >> diff --git a/vswitchd/ovs-vswitchd.8.in b/vswitchd/ovs-vswitchd.8.in >> index c18baf6..76ccfcb 100644 >> --- a/vswitchd/ovs-vswitchd.8.in >> +++ b/vswitchd/ovs-vswitchd.8.in >> @@ -283,6 +283,7 @@ port names, which this thread polls. >> .IP "\fBdpif-netdev/pmd-rxq-rebalance\fR [\fIdp\fR]" >> Reassigns rxqs to pmds in the datapath \fIdp\fR based on their current >> usage. >> . >> +.so lib/netdev-dpdk-unixctl.man >> .so ofproto/ofproto-dpif-unixctl.man >> .so ofproto/ofproto-unixctl.man >> .so lib/vlog-unixctl.man >> -- >> 2.7.4 >> > > I had the same feedback as Mark, and I reworded a bit to remove the > parenthesis. This is just a suggestion, I am fine either way. OK. Since you both want it, I will not argue. I'll use below wording in v4 with a few changes: * changed italic to bold for 'up' and 'down' as they are constant words. * added spaces between them. * removed brackets around 'interface' in description. > > Acked-by: Flavio Leitner > > diff --git a/lib/netdev-dpdk-unixctl.man b/lib/netdev-dpdk-unixctl.man > index 73b2e1065..d31b0135c 100644 > --- a/lib/netdev-dpdk-unixctl.man > +++ b/lib/netdev-dpdk-unixctl.man > @@ -1,8 +1,8 @@ > .SS "NETDEV-DPDK COMMANDS" > These commands manage DPDK related ports (\fItype=dpdk*\fR). > -.IP "\fBnetdev-dpdk/set-admin-state\fR [\fIinterface\fR] \fIstate\fR" > -Sets admin state for DPDK \fIinterface\fR (or all interfaces if none is > given) > -to \fIstate\fR. \fIstate\fR can be "up" or "down". > +.IP "\fBnetdev-dpdk/set-admin-state\fR [\fIinterface\fR] \fIup|down\fR" > +Change the admin state for DPDK \fIinterface\fR to \fIup\fR or \fIdown\fR. > +If [\fIinterface\fR] is not specified, then it applies to all DPDK ports. > .IP "\fBnetdev-dpdk/detach\fR \fIpci-address\fR" > Detaches device with corresponding \fIpci-address\fR from DPDK. This command > can be used to detach device if it wasn't detached automatically after port > ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v3 2/2] netdev-dpdk: Add debug appctl to get mempool information.
On 08.12.2017 19:36, Kavanagh, Mark B wrote:
>> From: Ilya Maximets [mailto:i.maxim...@samsung.com]
>> Sent: Friday, December 8, 2017 3:38 PM
>> To: ovs-dev@openvswitch.org
>> Cc: Heetae Ahn ; Fischetti, Antonio
>> ; Loftus, Ciara ;
>> Kavanagh, Mark B ; Stokes, Ian
>> ; Wojciechowicz, RobertX
>> ; Flavio Leitner ; Ilya
>> Maximets
>> Subject: [PATCH v3 2/2] netdev-dpdk: Add debug appctl to get mempool
>> information.
>>
>> New appctl 'netdev-dpdk/get-mempool-info' implemented to get result
>> of 'rte_mempool_list_dump()' function if no arguments passed and
>> 'rte_mempool_dump()' if DPDK netdev passed as argument.
>>
>> Could be used for debugging mbuf leaks and other mempool related
>> issues. Most useful in pair with `grep -v "cache_count.*=0"`.
>>
>> Signed-off-by: Ilya Maximets
>> Acked-by: Antonio Fischetti
>
>
> Hey Ilya,
>
> This is a really useful patch. When testing, I found the most helpful metrics
> to be the stats that are enabled when DPDK is built with
> CONFIG_RTE_LIBRTE_MEMPOOL_DEBUG=y.
>
> It might be a good idea to mention this somewhere in the documentation -
> thoughts?
I added a note to man page in v4.
>
> Other than that, LGTM:
> Tested-by: Mark Kavanagh
> Acked-by: Mark Kavanagh
>
> Cheers,
> Mark
>
>
>> ---
>> NEWS| 1 +
>> lib/netdev-dpdk-unixctl.man | 4
>> lib/netdev-dpdk.c | 54
>> +
>> 3 files changed, 59 insertions(+)
>>
>> diff --git a/NEWS b/NEWS
>> index e9fb279..5e13038 100644
>> --- a/NEWS
>> +++ b/NEWS
>> @@ -16,6 +16,7 @@ Post-v2.8.0
>>- "flush-conntrack" in ovs-dpctl and ovs-appctl now accept a 5-tuple to
>> delete a specific connection tracking entry.
>>- DPDK:
>> + * New debug appctl command 'netdev-dpdk/get-mempool-info'.
>> * All the netdev-dpdk appctl commands described in ovs-vswitchd man
>> page.
>>
>> v2.8.0 - 31 Aug 2017
>> diff --git a/lib/netdev-dpdk-unixctl.man b/lib/netdev-dpdk-unixctl.man
>> index a4b7f60..73b2e10 100644
>> --- a/lib/netdev-dpdk-unixctl.man
>> +++ b/lib/netdev-dpdk-unixctl.man
>> @@ -7,3 +7,7 @@ to \fIstate\fR. \fIstate\fR can be "up" or "down".
>> Detaches device with corresponding \fIpci-address\fR from DPDK. This command
>> can be used to detach device if it wasn't detached automatically after port
>> deletion. Refer to the documentation for details and instructions.
>> +.IP "\fBnetdev-dpdk/get-mempool-info\fR [\fIinterface\fR]"
>> +Prints the debug information about memory pool used by DPDK \fIinterface\fR.
>> +If called without arguments, information of all the available mempools will
>> +be printed.
>> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
>> index faff842..26c8e94 100644
>> --- a/lib/netdev-dpdk.c
>> +++ b/lib/netdev-dpdk.c
>> @@ -2560,6 +2560,56 @@ error:
>> free(response);
>> }
>>
>> +static void
>> +netdev_dpdk_get_mempool_info(struct unixctl_conn *conn,
>> + int argc, const char *argv[],
>> + void *aux OVS_UNUSED)
>> +{
>> +size_t size;
>> +FILE *stream;
>> +char *response = NULL;
>> +struct netdev *netdev = NULL;
>> +
>> +if (argc == 2) {
>> +netdev = netdev_from_name(argv[1]);
>> +if (!netdev || !is_dpdk_class(netdev->netdev_class)) {
>> +unixctl_command_reply_error(conn, "Not a DPDK Interface");
>> +goto out;
>> +}
>> +}
>> +
>> +stream = open_memstream(&response, &size);
>> +if (!stream) {
>> +response = xasprintf("Unable to open memstream: %s.",
>> + ovs_strerror(errno));
>> +unixctl_command_reply_error(conn, response);
>> +goto out;
>> +}
>> +
>> +if (netdev) {
>> +struct netdev_dpdk *dev = netdev_dpdk_cast(netdev);
>> +
>> +ovs_mutex_lock(&dev->mutex);
>> +ovs_mutex_lock(&dpdk_mp_mutex);
>> +
>> +rte_mempool_dump(stream, dev->mp);
>> +
>> +ovs_mutex_unlock(&dpdk_mp_mutex);
>> +ovs_mutex_unlock(&dev->mutex);
>> +} else {
>> +ovs_mutex_lock(&dpdk_mp_mutex);
>> +rte_mempool_list_dump(stream);
>> +ovs_mutex_unlock(&dpdk_mp_mutex);
>> +}
>> +
>> +fclose(stream);
>> +
>> +unixctl_command_reply(conn, response);
>> +out:
>> +free(response);
>> +netdev_close(netdev);
>> +}
>> +
>> /*
>> * Set virtqueue flags so that we do not receive interrupts.
>> */
>> @@ -2816,6 +2866,10 @@ netdev_dpdk_class_init(void)
>> "pci address of device", 1, 1,
>> netdev_dpdk_detach, NULL);
>>
>> +unixctl_command_register("netdev-dpdk/get-mempool-info",
>> + "[netdev]", 0, 1,
>> + netdev_dpdk_get_mempool_info, NULL);
>> +
>> ovsthread_once_done(&once);
>> }
>>
>> --
>> 2.7.4
>
>
>
>
___
dev mailing list
d...@openvswit
[ovs-dev] [PATCH v4 0/2] netdev-dpdk: Mempool creation failure + Appctl
Second patch adds debug appctl to obtain mempool information from DPDK including names, numbers of available mbufs, object sizes and memory pointers. First patch introduces common place for 'netdev-dpdk' unixctl commands documentation in man pages. Version 4: * Rebased on current istokes/dpdk_merge (3eb8d4f). * Updated man page entry for 'set-admin-state' to be conform with 'ovs-appctl list-commands' output. * Added note about CONFIG_RTE_LIBRTE_MEMPOOL_DEBUG to man page. Version 3: * Dropped patch about mempool failure - Already accepted. * Rebased on current master Version 2: * Oficially added documentation patch. * Documentation patch splitted to move 'get-mempool-info'-related docs to patch, where this appctl introduced. * Patches reordered for consistency. * I kept all the ACKs because there was no actual changes. Antonio, please, let me know if you don't like the re-splitting. Ilya Maximets (2): vswitchd: Document netdev-dpdk commands. netdev-dpdk: Add debug appctl to get mempool information. NEWS| 2 ++ lib/automake.mk | 1 + lib/netdev-dpdk-unixctl.man | 14 lib/netdev-dpdk.c | 54 + manpages.mk | 2 ++ vswitchd/ovs-vswitchd.8.in | 1 + 6 files changed, 74 insertions(+) create mode 100644 lib/netdev-dpdk-unixctl.man -- 2.7.4 ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH v4 1/2] vswitchd: Document netdev-dpdk commands.
netdev-dpdk appctl commands added to man pages. Signed-off-by: Ilya Maximets --- NEWS| 1 + lib/automake.mk | 1 + lib/netdev-dpdk-unixctl.man | 9 + manpages.mk | 2 ++ vswitchd/ovs-vswitchd.8.in | 1 + 5 files changed, 14 insertions(+) create mode 100644 lib/netdev-dpdk-unixctl.man diff --git a/NEWS b/NEWS index d45904e..69d5dab 100644 --- a/NEWS +++ b/NEWS @@ -18,6 +18,7 @@ Post-v2.8.0 - DPDK: * Add support for DPDK v17.11 * Add support for vHost IOMMU + * All the netdev-dpdk appctl commands described in ovs-vswitchd man page. v2.8.0 - 31 Aug 2017 diff --git a/lib/automake.mk b/lib/automake.mk index effe5b5..4b38a11 100644 --- a/lib/automake.mk +++ b/lib/automake.mk @@ -465,6 +465,7 @@ MAN_FRAGMENTS += \ lib/db-ctl-base.man \ lib/dpctl.man \ lib/memory-unixctl.man \ + lib/netdev-dpdk-unixctl.man \ lib/ofp-version.man \ lib/ovs.tmac \ lib/service.man \ diff --git a/lib/netdev-dpdk-unixctl.man b/lib/netdev-dpdk-unixctl.man new file mode 100644 index 000..5af6eca --- /dev/null +++ b/lib/netdev-dpdk-unixctl.man @@ -0,0 +1,9 @@ +.SS "NETDEV-DPDK COMMANDS" +These commands manage DPDK related ports (\fBtype=\fR\fIdpdk*\fR). +.IP "\fBnetdev-dpdk/set-admin-state\fR [\fIinterface\fR] \fBup\fR | \fBdown\fR" +Change the admin state for DPDK \fIinterface\fR to \fBup\fR or \fBdown\fR. +If \fIinterface\fR is not specified, then it applies to all DPDK ports. +.IP "\fBnetdev-dpdk/detach\fR \fIpci-address\fR" +Detaches device with corresponding \fIpci-address\fR from DPDK. This command +can be used to detach device if it wasn't detached automatically after port +deletion. Refer to the documentation for details and instructions. diff --git a/manpages.mk b/manpages.mk index d610d88..c89bc45 100644 --- a/manpages.mk +++ b/manpages.mk @@ -279,6 +279,7 @@ vswitchd/ovs-vswitchd.8: \ lib/daemon.man \ lib/dpctl.man \ lib/memory-unixctl.man \ + lib/netdev-dpdk-unixctl.man \ lib/service.man \ lib/ssl-bootstrap.man \ lib/ssl.man \ @@ -296,6 +297,7 @@ lib/coverage-unixctl.man: lib/daemon.man: lib/dpctl.man: lib/memory-unixctl.man: +lib/netdev-dpdk-unixctl.man: lib/service.man: lib/ssl-bootstrap.man: lib/ssl.man: diff --git a/vswitchd/ovs-vswitchd.8.in b/vswitchd/ovs-vswitchd.8.in index c18baf6..76ccfcb 100644 --- a/vswitchd/ovs-vswitchd.8.in +++ b/vswitchd/ovs-vswitchd.8.in @@ -283,6 +283,7 @@ port names, which this thread polls. .IP "\fBdpif-netdev/pmd-rxq-rebalance\fR [\fIdp\fR]" Reassigns rxqs to pmds in the datapath \fIdp\fR based on their current usage. . +.so lib/netdev-dpdk-unixctl.man .so ofproto/ofproto-dpif-unixctl.man .so ofproto/ofproto-unixctl.man .so lib/vlog-unixctl.man -- 2.7.4 ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH v4 2/2] netdev-dpdk: Add debug appctl to get mempool information.
New appctl 'netdev-dpdk/get-mempool-info' implemented to get result
of 'rte_mempool_list_dump()' function if no arguments passed and
'rte_mempool_dump()' if DPDK netdev passed as argument.
Could be used for debugging mbuf leaks and other mempool related
issues. Most useful in pair with `grep -v "cache_count.*=0"`.
Signed-off-by: Ilya Maximets
---
NEWS| 1 +
lib/netdev-dpdk-unixctl.man | 5 +
lib/netdev-dpdk.c | 54 +
3 files changed, 60 insertions(+)
diff --git a/NEWS b/NEWS
index 69d5dab..e60514e 100644
--- a/NEWS
+++ b/NEWS
@@ -18,6 +18,7 @@ Post-v2.8.0
- DPDK:
* Add support for DPDK v17.11
* Add support for vHost IOMMU
+ * New debug appctl command 'netdev-dpdk/get-mempool-info'.
* All the netdev-dpdk appctl commands described in ovs-vswitchd man page.
v2.8.0 - 31 Aug 2017
diff --git a/lib/netdev-dpdk-unixctl.man b/lib/netdev-dpdk-unixctl.man
index 5af6eca..ac274cd 100644
--- a/lib/netdev-dpdk-unixctl.man
+++ b/lib/netdev-dpdk-unixctl.man
@@ -7,3 +7,8 @@ If \fIinterface\fR is not specified, then it applies to all
DPDK ports.
Detaches device with corresponding \fIpci-address\fR from DPDK. This command
can be used to detach device if it wasn't detached automatically after port
deletion. Refer to the documentation for details and instructions.
+.IP "\fBnetdev-dpdk/get-mempool-info\fR [\fIinterface\fR]"
+Prints the debug information about memory pool used by DPDK \fIinterface\fR.
+If called without arguments, information of all the available mempools will
+be printed. For additional mempool statistics enable
+\fBCONFIG_RTE_LIBRTE_MEMPOOL_DEBUG\fR while building DPDK.
diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index 8f22264..3bf461b 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -2586,6 +2586,56 @@ error:
free(response);
}
+static void
+netdev_dpdk_get_mempool_info(struct unixctl_conn *conn,
+ int argc, const char *argv[],
+ void *aux OVS_UNUSED)
+{
+size_t size;
+FILE *stream;
+char *response = NULL;
+struct netdev *netdev = NULL;
+
+if (argc == 2) {
+netdev = netdev_from_name(argv[1]);
+if (!netdev || !is_dpdk_class(netdev->netdev_class)) {
+unixctl_command_reply_error(conn, "Not a DPDK Interface");
+goto out;
+}
+}
+
+stream = open_memstream(&response, &size);
+if (!stream) {
+response = xasprintf("Unable to open memstream: %s.",
+ ovs_strerror(errno));
+unixctl_command_reply_error(conn, response);
+goto out;
+}
+
+if (netdev) {
+struct netdev_dpdk *dev = netdev_dpdk_cast(netdev);
+
+ovs_mutex_lock(&dev->mutex);
+ovs_mutex_lock(&dpdk_mp_mutex);
+
+rte_mempool_dump(stream, dev->mp);
+
+ovs_mutex_unlock(&dpdk_mp_mutex);
+ovs_mutex_unlock(&dev->mutex);
+} else {
+ovs_mutex_lock(&dpdk_mp_mutex);
+rte_mempool_list_dump(stream);
+ovs_mutex_unlock(&dpdk_mp_mutex);
+}
+
+fclose(stream);
+
+unixctl_command_reply(conn, response);
+out:
+free(response);
+netdev_close(netdev);
+}
+
/*
* Set virtqueue flags so that we do not receive interrupts.
*/
@@ -2842,6 +2892,10 @@ netdev_dpdk_class_init(void)
"pci address of device", 1, 1,
netdev_dpdk_detach, NULL);
+unixctl_command_register("netdev-dpdk/get-mempool-info",
+ "[netdev]", 0, 1,
+ netdev_dpdk_get_mempool_info, NULL);
+
ovsthread_once_done(&once);
}
--
2.7.4
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] travis: Use pip2 instead of pip for OSX build.
> -Original Message- > From: ovs-dev-boun...@openvswitch.org [mailto:ovs-dev- > boun...@openvswitch.org] On Behalf Of Ilya Maximets > Sent: Friday, December 8, 2017 2:53 PM > To: ovs-dev@openvswitch.org > Cc: Ilya Maximets ; Lance Richardson > ; Heetae Ahn > Subject: [ovs-dev] [PATCH] travis: Use pip2 instead of pip for OSX build. > > xcode8.3 is a new default image for OS X on Travis-CI, but it does not have > 'pip': > > pip install --user six > ./.travis/osx-prepare.sh: line 3: pip: command not found > > 'pip2' or 'pip3' should be used explicitly instead: > > https://github.com/travis-ci/travis-ci/issues/8829 > > Signed-off-by: Ilya Maximets I haven't tested it, but I saw other people complaining about the same issue on travis-ci. Acked-by: Alin Gabriel Serdean ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH] daemon-unix: include missing help information
These options have existed for a while, but were not expressed in the help information. Inform the user that these options exist, and give some basic help. Reported-by: Saravanan KR Signed-off-by: Aaron Conole --- lib/daemon-unix.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lib/daemon-unix.c b/lib/daemon-unix.c index 967a28432..adb549c98 100644 --- a/lib/daemon-unix.c +++ b/lib/daemon-unix.c @@ -537,6 +537,8 @@ daemon_usage(void) printf( "\nDaemon options:\n" " --detachrun in background as daemon\n" +" --monitor creates a process to monitor this daemon\n" +" --user=username[:group] changes the effective daemon user:group\n" " --no-chdir do not chdir to '/'\n" " --pidfile[=FILE]create pidfile (default: %s/%s.pid)\n" " --overwrite-pidfile with --pidfile, start even if already " -- 2.14.3 ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] daemon-unix: include missing help information
On 11/12/17 15:07, Aaron Conole wrote: > These options have existed for a while, but were not expressed in the > help information. Inform the user that these options exist, and give > some basic help. > > Reported-by: Saravanan KR > Signed-off-by: Aaron Conole > --- > lib/daemon-unix.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/lib/daemon-unix.c b/lib/daemon-unix.c > index 967a28432..adb549c98 100644 > --- a/lib/daemon-unix.c > +++ b/lib/daemon-unix.c > @@ -537,6 +537,8 @@ daemon_usage(void) > printf( > "\nDaemon options:\n" > " --detachrun in background as daemon\n" > +" --monitor creates a process to monitor this > daemon\n" > +" --user=username[:group] changes the effective daemon user:group\n" > " --no-chdir do not chdir to '/'\n" > " --pidfile[=FILE]create pidfile (default: %s/%s.pid)\n" > " --overwrite-pidfile with --pidfile, start even if already " > Good catch Reviewed-by: Markos Chandras -- markos SUSE LINUX GmbH | GF: Felix Imendörffer, Jane Smithard, Graham Norton HRB 21284 (AG Nürnberg) Maxfeldstr. 5, D-90409, Nürnberg ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] tunnel: fix tnl_find() after packet_type changed
Hi, I've been working on the solution proposed by Jan. I faced a new issue with patch ports. Let's have a config like in unit test 1025: "1025: ofproto-dpif - balance-tcp bonding, different recirc flow " +-+ | br-int| p5 (OF 5) | o--<--- +---o-+ | br1- (patch port: OF 101) | +--+ | | br1+ (patch port: OF 100) +--o--+ | br1 | | | +---o-o---+ p3 | | p4 (OF 3)| | (OF 4) |bond1| +--+--+ | In the unit test, a packet is received on port p5. The packet is then output to 'br1-' which is a patch port, its pair is 'br1+' on br1. Ports p3 and p4 are in bond0, the packet is recirculated in br1, before it is sent out on bond1. So, when it's recirculated, ofproto xlate will end up in xlate_lookup_ofproto_() at some point in time. The vanilla code does xport_lookup() based on the flow which is built up based on dp_packet (due to miniflow_extract/expand) which has in port 5 (p5). So the flow has in port 5 as well. Furthermore, the returned ofproto_dpif will be xport->xbridge->ofproto: br-int. If we do use the recirc ID extracted from the flow (or from dp_packet) which is 2, then the state's uuid will refer to the bridge br1. This is the bridge where the recirc action was added to odp_actions. On the other hand the in port stored in the state is 0x (undefined). It's neither 5 (p5) nor 100 (br1+). So, If we want to build our code based on the recirc ID extracted from flow (or dp_packet) we won't get the same result as the vanilla code in case of using patch ports and recirculate in the second bridge. Does anyone have a idea how to resolve this? Maybe the vanilla code or patch port concept is broken? If we would use the original value of packet_type in tnl_find() we could still workaround this. Best regards, Zoltan > -Original Message- > From: Jan Scheurich > Sent: Friday, December 08, 2017 5:18 PM > To: Zoltán Balogh ; d...@openvswitch.org > Cc: Ben Pfaff > Subject: RE: [PATCH] tunnel: fix tnl_find() after packet_type changed > > Hi Zoltan, > Please find answers below. > Regards, Jan > > > -Original Message- > > From: Zoltán Balogh > > Sent: Friday, 08 December, 2017 12:56 > > To: Jan Scheurich ; d...@openvswitch.org > > Cc: Ben Pfaff > > Subject: RE: [PATCH] tunnel: fix tnl_find() after packet_type changed > > > > Hello Jan, > > > > Thank you for the review. > > The recirc_id_node, and thus the frozen_state with the the ofproto_uuid can > > be retrieved from recirc ID via > recirc_id_node_find(). So I > > think, it would be feasible to get the ofproto from the recirc ID without > > calling tnl_find(). In addition we would > need to store in_port in the > > frozen_state. > > Yes, my thinking was to lookup the recirculation context in > xlate_lookup_ofproto_() if flow->recirc_id != 0 and get > bridge and in_port from the frozen data in that case. But I was looking for a > way to store the recirc context > pointer so that it need not be looked up once more later during > xlate_actions(). > > BTW: in_port is already stored in frozen_state.metdata.in_port > > > > > However, if we do not fix this in tnl_find(), then we need to make sure, > > that neither xlate_lookup_ofproto() nor > xlate_lookup() are called > > after flow->packet_type has changed during xlate, don't we? For instance, > > calling revalidate() can lead to call > xlate_lookup(). > > revalidate() is called from revalidator threads to pass an artificial packet > representing the megaflow through the > pipeline. The flow constructed from the ukey will contain the recirc_id and > it should still hit the same > recirculation context with the frozen_state as a MISS upcall for the > originally recirculated packet. > > > > > Btw, what about the case, when we have a L2 and L3 port with the same local > > IP. A packet is received on one of > them, but the packet_type has changed during xlate and later revalidate leads > to calling xlate_lookup()? It will > provide the wrong tunnel port as in_port. Is this a valid case? > > I think xlate_lookup() is only executed once at the beginning of each upcall > or ukey revalidation to initiatilize > some data. It is never done again later during the xlation process. > > > > > I was thinking about storing the original value of packet_type of received > > packet in dp_packet or in the flow > structure. Then use the > > original value in tnl_find(). > > > > Best regards, > > Zoltan > > > > > -Original Message- > > > From: Jan Scheurich > > > Sent: Friday, December 08, 2017 11:30 AM > > > To: Zoltán Balogh ; d...@openvswitch.org > > > Cc: Ben Pfaff > > > Subject: RE: [PATCH] tunnel: fix tnl_find() after packet_type changed > > > > > > Hi Zoltan, > > > > > > My feeling here is that it is conceptually wrong to do another tunnel > > > lookup if the packet is reci
[ovs-dev] [patch v2] conntrack: Some style improvements.
Fix up some instances where variable declarations were not close
enough to their use, as these were missed before. This is the
preferred art in OVS code and flagged heavily in code reviews.
This is highly desirable due to code clarity reasons.
There are also some cases where newlines were not needed by prior art
and some cases where they were needed but missed.
There was one case where there was a missing space after "}".
There were a few cases where for loop index decalrations could be
folded into the loop.
One function was missing some const qualifiers.
Signed-off-by: Darrell Ball
---
This patch depends on the series here:
https://patchwork.ozlabs.org/project/openvswitch/list/?series=16626
and the folowing patch.
https://patchwork.ozlabs.org/patch/846490/
lib/conntrack.c | 145 +++-
1 file changed, 59 insertions(+), 86 deletions(-)
diff --git a/lib/conntrack.c b/lib/conntrack.c
index be0752d..03f6fce 100644
--- a/lib/conntrack.c
+++ b/lib/conntrack.c
@@ -248,8 +248,8 @@ conn_key_cmp(const struct conn_key *key1, const struct
conn_key *key2)
}
static void
-ct_print_conn_info(struct conn *c, char *log_msg, enum vlog_level vll,
- bool force, bool rl_on)
+ct_print_conn_info(const struct conn *c, const char *log_msg,
+ enum vlog_level vll, bool force, bool rl_on)
{
#define CT_VLOG(RL_ON, LEVEL, ...) \
do {\
@@ -307,7 +307,6 @@ ct_print_conn_info(struct conn *c, char *log_msg, enum
vlog_level vll,
void
conntrack_init(struct conntrack *ct)
{
-unsigned i, j;
long long now = time_msec();
ct_rwlock_init(&ct->resources_lock);
@@ -318,13 +317,13 @@ conntrack_init(struct conntrack *ct)
ovs_list_init(&ct->alg_exp_list);
ct_rwlock_unlock(&ct->resources_lock);
-for (i = 0; i < CONNTRACK_BUCKETS; i++) {
+for (unsigned i = 0; i < CONNTRACK_BUCKETS; i++) {
struct conntrack_bucket *ctb = &ct->buckets[i];
-
ct_lock_init(&ctb->lock);
ct_lock_lock(&ctb->lock);
hmap_init(&ctb->connections);
-for (j = 0; j < ARRAY_SIZE(ctb->exp_lists); j++) {
+
+for (unsigned j = 0; j < ARRAY_SIZE(ctb->exp_lists); j++) {
ovs_list_init(&ctb->exp_lists[j]);
}
ct_lock_unlock(&ctb->lock);
@@ -344,12 +343,10 @@ conntrack_init(struct conntrack *ct)
void
conntrack_destroy(struct conntrack *ct)
{
-unsigned i;
-
latch_set(&ct->clean_thread_exit);
pthread_join(ct->clean_thread, NULL);
latch_destroy(&ct->clean_thread_exit);
-for (i = 0; i < CONNTRACK_BUCKETS; i++) {
+for (unsigned i = 0; i < CONNTRACK_BUCKETS; i++) {
struct conntrack_bucket *ctb = &ct->buckets[i];
struct conn *conn;
@@ -422,7 +419,6 @@ write_ct_md(struct dp_packet *pkt, uint16_t zone, const
struct conn *conn,
pkt->md.ct_orig_tuple_ipv6 = false;
if (key) {
if (key->dl_type == htons(ETH_TYPE_IP)) {
-
pkt->md.ct_orig_tuple.ipv4 = (struct ovs_key_ct_tuple_ipv4) {
key->src.addr.ipv4_aligned,
key->dst.addr.ipv4_aligned,
@@ -645,7 +641,6 @@ reverse_nat_packet(struct dp_packet *pkt, const struct conn
*conn)
struct ip_header *inner_l3 = (struct ip_header *) (icmp + 1);
extract_l3_ipv4(&inner_key, inner_l3, tail - ((char *)inner_l3) - pad,
&inner_l4, false);
-
pkt->l3_ofs += (char *) inner_l3 - (char *) nh;
pkt->l4_ofs += inner_l4 - (char *) icmp;
@@ -656,6 +651,7 @@ reverse_nat_packet(struct dp_packet *pkt, const struct conn
*conn)
packet_set_ipv4_addr(pkt, &inner_l3->ip_dst,
conn->key.dst.addr.ipv4_aligned);
}
+
reverse_pat_packet(pkt, conn);
icmp->icmp_csum = 0;
icmp->icmp_csum = csum(icmp, tail - (char *) icmp - pad);
@@ -770,7 +766,6 @@ nat_clean(struct conntrack *ct, struct conn *conn,
struct conntrack_bucket *ctb)
OVS_REQUIRES(ctb->lock)
{
-long long now = time_msec();
ct_rwlock_wrlock(&ct->resources_lock);
nat_conn_keys_remove(&ct->nat_conn_keys, &conn->rev_key, ct->hash_basis);
ct_rwlock_unlock(&ct->resources_lock);
@@ -782,8 +777,8 @@ nat_clean(struct conntrack *ct, struct conn *conn,
ct_lock_lock(&ct->buckets[bucket_rev_conn].lock);
ct_rwlock_wrlock(&ct->resources_lock);
+long long now = time_msec();
struct conn *rev_conn = conn_lookup(ct, &conn->rev_key, now);
-
struct nat_conn_key_node *nat_conn_key_node =
nat_conn_keys_lookup(&ct->nat_conn_keys, &conn->rev_key,
ct->hash_basis);
@@ -797,8 +792,8 @@ nat_clean(struct conntrack *ct, struct conn *conn,
&rev_conn->node);
free(rev_conn);
}
-delete_conn(conn);
+delete_conn(conn);
ct_rwlock_unlock(&
Re: [ovs-dev] [PATCH v1] datapath-windows: Add support for deleting conntrack entry by 5-tuple.
Thanks all! I applied this on master. > -Original Message- > From: ovs-dev-boun...@openvswitch.org [mailto:ovs-dev- > boun...@openvswitch.org] On Behalf Of Yi-Hung Wei > Sent: Wednesday, November 29, 2017 9:28 PM > To: aserd...@ovn.org; Anand Kumar > Cc: ovs dev > Subject: Re: [ovs-dev] [PATCH v1] datapath-windows: Add support for > deleting conntrack entry by 5-tuple. > > Hi Anand and Alin, > > I have some updates about supporting delete conntrack entry by 5-tuple on > my v2 patch series ( https://mail.openvswitch.org/pipermail/ovs-dev/2017- > November/341140.html ). > > In the v2 series, I made some changes on the dpif-netlink implementation ( > https://mail.openvswitch.org/pipermail/ovs-dev/2017- > November/341142.html ), and I am not sure if it will affect the windows > datapath implementation. > > Given that the dpif-netlink implementation is still under review, if it will > affect the windows datapath implementation, do you mind if we wait a bit till > the dpif-netlink implementation is upstream? > > Thanks, > > -Yi-Hung > > On Wed, Nov 29, 2017 at 10:19 AM, wrote: > > > I wanted to give it a few days to see if another review pops up. > > > > Mind if I wait until tomorrow? > > > > Thanks, > > Alin. > > > ___ > dev mailing list > d...@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] datapath-windows: Correct endianness for deleting zone.
I was concerned a bit about binary compatibility between the future userspace binaries and older kernel versions. But Windows users usually just use the MSI (because of the signing issues). Thanks both! Alin. > -Original Message- > From: ovs-dev-boun...@openvswitch.org [mailto:ovs-dev- > boun...@openvswitch.org] On Behalf Of Sairam Venugopal > Sent: Friday, December 8, 2017 1:26 AM > To: aserd...@ovn.org; 'Justin Pettit' ; > d...@openvswitch.org > Subject: Re: [ovs-dev] [PATCH] datapath-windows: Correct endianness for > deleting zone. > > We can hold off on back-porting this. This change mainly affects the delete > by 5-tuple API. > > https://github.com/openvswitch/ovs/commit/817a76577fec3f03310d7d3a5a > 10df01340ee8ad > > Unless we plan to backport that patch, we should hold off on backporting this > as well. > > Thanks, > Sairam > > > > On 12/6/17, 9:41 AM, "aserd...@ovn.org" wrote: > > >> -Original Message- > >> From: ovs-dev-boun...@openvswitch.org [mailto:ovs-dev- > >> boun...@openvswitch.org] On Behalf Of Justin Pettit > >> Sent: Tuesday, December 5, 2017 9:30 AM > >> To: d...@openvswitch.org; Alin Gabriel Serdean ; > >> Sairam Venugopal > >> Subject: [ovs-dev] [PATCH] datapath-windows: Correct endianness for > >> deleting zone. > >> > >> The zone Netlink attribute is supposed to be in network-byte order, > >> but > >the > >> Windows code for deleting conntrack entries was treating it as > >> host-byte order. > >> > >> Found by inspection. > >> > >> Signed-off-by: Justin Pettit > > > >Thanks for the patch, Justin. I tested the patch, and everything looks > >good. > >I want to hold on the acknowledge until all of us are on the same page. > > > >This will make the windows datapath consistent with future patches from > >the userspace. > >For consistency I would like to backport the patch all the way to > >branch-2.6. > > > >What do you think Sai? > > > >Thanks, > >Alin. > > > ___ > dev mailing list > d...@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH v7 0/6] Output packet batching.
This patch-set inspired by [1] from Bhanuprakash Bodireddy. Implementation of [1] looks very complex and introduces many pitfalls [2] for later code modifications like possible packet stucks. This version targeted to make simple and flexible output packet batching on higher level without introducing and even simplifying netdev layer. [1] [PATCH v4 0/5] netdev-dpdk: Use intermediate queue during packet transmission. https://mail.openvswitch.org/pipermail/ovs-dev/2017-August/337019.html [2] For example: https://mail.openvswitch.org/pipermail/ovs-dev/2017-August/337133.html Testing of 'PVP with OVS bonding on phy ports' scenario shows significant performance improvement up to +25%: https://mail.openvswitch.org/pipermail/ovs-dev/2017-December/341700.html Other testing results for v6: https://mail.openvswitch.org/pipermail/ovs-dev/2017-December/341605.html https://mail.openvswitch.org/pipermail/ovs-dev/2017-December/341628.html Version 7: * Rebased on current istokes/dpdk_merge (3eb8d4f) * Dropped dp_netdev_pmd_thread structure refactoring patch. Not needed since revert applied. Version 6: * Rebased on current master: - Added new patch to refactor dp_netdev_pmd_thread structure according to following suggestion: https://mail.openvswitch.org/pipermail/ovs-dev/2017-November/341230.html NOTE: I still prefer reverting of the padding related patch. Rebase done to not block acepting of this series. Revert patch and discussion here: https://mail.openvswitch.org/pipermail/ovs-dev/2017-November/341153.html * Added comment about pmd_thread_ctx_time_update() usage. Version 5: * pmd_thread_ctx_time_update() calls moved to different places to call them only from dp_netdev_process_rxq_port() and main polling functions: pmd_thread_main, dpif_netdev_run and dpif_netdev_execute. All other functions should use cached time from pmd->ctx.now. It's guaranteed to be updated at least once per polling cycle. * 'may_steal' patch returned to version from v3 because 'may_steal' in qos is a completely different variable. This patch only removes 'may_steal' from netdev API. * 2 more usec functions added to timeval to have complete public API. * Checking of 'output_cnt' turned to assertion. Version 4: * Rebased on current master. * Rebased on top of "Keep latest measured time for PMD thread." (Jan Scheurich) * Microsecond resolution related patches integrated. * Time-based batching without RFC tag. * 'output_time' renamed to 'flush_time'. (Jan Scheurich) * 'flush_time' update moved to 'dp_netdev_pmd_flush_output_on_port'. (Jan Scheurich) * 'output-max-latency' renamed to 'tx-flush-interval'. * Added patch for output batching statistics. Version 3: * Rebased on current master. * Time based RFC: fixed assert on n_output_batches <= 0. Version 2: * Rebased on current master. * Added time based batching RFC patch. * Fixed mixing packets with different sources in same batch. Ilya Maximets (6): dpif-netdev: Keep latest measured time for PMD thread. dpif-netdev: Output packet batching. netdev: Remove unused may_steal. netdev: Remove useless cutlen. dpif-netdev: Time based output batching. dpif-netdev: Count sent packets and batches. lib/dpif-netdev.c | 349 ++ lib/netdev-bsd.c | 6 +- lib/netdev-dpdk.c | 30 ++--- lib/netdev-dummy.c| 6 +- lib/netdev-linux.c| 8 +- lib/netdev-provider.h | 7 +- lib/netdev.c | 12 +- lib/netdev.h | 2 +- vswitchd/vswitch.xml | 16 +++ 9 files changed, 311 insertions(+), 125 deletions(-) -- 2.7.4 ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH v7 1/6] dpif-netdev: Keep latest measured time for PMD thread.
In current implementation 'now' variable updated once on each
receive cycle and passed through the whole datapath via function
arguments. It'll be better to keep this variable inside PMD
thread structure to be able to get it at any time. Such solution
will save the stack memory and simplify possible modifications
in current logic.
This patch introduces new structure 'dp_netdev_pmd_thread_ctx'
contained by 'struct dp_netdev_pmd_thread' to store any processing
context of this PMD thread. For now, only time and cycles moved to
that structure. Can be extended in the future.
Acked-by: Eelco Chaudron
Signed-off-by: Ilya Maximets
---
lib/dpif-netdev.c | 144 +-
1 file changed, 88 insertions(+), 56 deletions(-)
diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index 43f6a78..48d54f5 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -528,6 +528,17 @@ struct tx_port {
struct hmap_node node;
};
+/* A set of properties for the current processing loop that is not directly
+ * associated with the pmd thread itself, but with the packets being
+ * processed or the short-term system configuration (for example, time).
+ * Contained by struct dp_netdev_pmd_thread's 'ctx' member. */
+struct dp_netdev_pmd_thread_ctx {
+/* Latest measured time. See 'pmd_thread_ctx_time_update()'. */
+long long now;
+/* Used to count cycles. See 'cycles_count_end()' */
+unsigned long long last_cycles;
+};
+
/* PMD: Poll modes drivers. PMD accesses devices via polling to eliminate
* the performance overhead of interrupt processing. Therefore netdev can
* not implement rx-wait for these devices. dpif-netdev needs to poll
@@ -583,8 +594,8 @@ struct dp_netdev_pmd_thread {
/* Cycles counters */
struct dp_netdev_pmd_cycles cycles;
-/* Used to count cicles. See 'cycles_counter_end()' */
-unsigned long long last_cycles;
+/* Current context of the PMD thread. */
+struct dp_netdev_pmd_thread_ctx ctx;
struct latch exit_latch;/* For terminating the pmd thread. */
struct seq *reload_seq;
@@ -657,8 +668,7 @@ static void dp_netdev_execute_actions(struct
dp_netdev_pmd_thread *pmd,
struct dp_packet_batch *,
bool may_steal, const struct flow *flow,
const struct nlattr *actions,
- size_t actions_len,
- long long now);
+ size_t actions_len);
static void dp_netdev_input(struct dp_netdev_pmd_thread *,
struct dp_packet_batch *, odp_port_t port_no);
static void dp_netdev_recirculate(struct dp_netdev_pmd_thread *,
@@ -718,9 +728,9 @@ static uint64_t
dp_netdev_rxq_get_intrvl_cycles(struct dp_netdev_rxq *rx, unsigned idx);
static void
dpif_netdev_xps_revalidate_pmd(const struct dp_netdev_pmd_thread *pmd,
- long long now, bool purge);
+ bool purge);
static int dpif_netdev_xps_get_tx_qid(const struct dp_netdev_pmd_thread *pmd,
- struct tx_port *tx, long long now);
+ struct tx_port *tx);
static inline bool emc_entry_alive(struct emc_entry *ce);
static void emc_clear_entry(struct emc_entry *ce);
@@ -764,6 +774,28 @@ emc_cache_slow_sweep(struct emc_cache *flow_cache)
flow_cache->sweep_idx = (flow_cache->sweep_idx + 1) & EM_FLOW_HASH_MASK;
}
+/* Updates the time in PMD threads context and should be called in three cases:
+ *
+ * 1. PMD structure initialization:
+ * - dp_netdev_configure_pmd()
+ *
+ * 2. Before processing of the new packet batch:
+ * - dpif_netdev_execute()
+ * - dp_netdev_input__()
+ *
+ * 3. At least once per polling iteration in main polling threads if no
+ *packets received on current iteration:
+ * - dpif_netdev_run()
+ * - pmd_thread_main()
+ *
+ * 'pmd->ctx.now' should be used without update in all other cases if possible.
+ */
+static inline void
+pmd_thread_ctx_time_update(struct dp_netdev_pmd_thread *pmd)
+{
+pmd->ctx.now = time_msec();
+}
+
/* Returns true if 'dpif' is a netdev or dummy dpif, false otherwise. */
bool
dpif_is_netdev(const struct dpif *dpif)
@@ -2915,6 +2947,9 @@ dpif_netdev_execute(struct dpif *dpif, struct
dpif_execute *execute)
ovs_mutex_lock(&dp->non_pmd_mutex);
}
+/* Update current time in PMD context. */
+pmd_thread_ctx_time_update(pmd);
+
/* The action processing expects the RSS hash to be valid, because
* it's always initialized at the beginning of datapath processing.
* In this case, though, 'execute->packet' may not have gone through
@@ -2927,8 +2962,7 @@ dpif_netdev_execute(struct dpif *dpif, struct
dpif_execute *execute)
dp_packet_batch_init_packet(&pp, execute->p
[ovs-dev] [PATCH v7 2/6] dpif-netdev: Output packet batching.
While processing incoming batch of packets they are scattered
across many per-flow batches and sent separately.
This becomes an issue while using more than a few flows.
For example if we have balanced-tcp OvS bonding with 2 ports
there will be 256 datapath internal flows for each dp_hash
pattern. This will lead to scattering of a single recieved
batch across all of that 256 per-flow batches and invoking
send for each packet separately. This behaviour greatly degrades
overall performance of netdev_send because of inability to use
advantages of vectorized transmit functions.
But the half (if 2 ports in bonding) of datapath flows will
have the same output actions. This means that we can collect
them in a single place back and send at once using single call
to netdev_send. This patch introduces per-port packet batch
for output packets for that purpose.
'output_pkts' batch is thread local and located in send port cache.
Acked-by: Eelco Chaudron
Signed-off-by: Ilya Maximets
---
lib/dpif-netdev.c | 77 ++-
1 file changed, 65 insertions(+), 12 deletions(-)
diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index 48d54f5..e504ddb 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -526,6 +526,7 @@ struct tx_port {
int qid;
long long last_used;
struct hmap_node node;
+struct dp_packet_batch output_pkts;
};
/* A set of properties for the current processing loop that is not directly
@@ -704,6 +705,9 @@ static void dp_netdev_add_rxq_to_pmd(struct
dp_netdev_pmd_thread *pmd,
static void dp_netdev_del_rxq_from_pmd(struct dp_netdev_pmd_thread *pmd,
struct rxq_poll *poll)
OVS_REQUIRES(pmd->port_mutex);
+static void
+dp_netdev_pmd_flush_output_packets(struct dp_netdev_pmd_thread *pmd);
+
static void reconfigure_datapath(struct dp_netdev *dp)
OVS_REQUIRES(dp->port_mutex);
static bool dp_netdev_pmd_try_ref(struct dp_netdev_pmd_thread *pmd);
@@ -781,7 +785,7 @@ emc_cache_slow_sweep(struct emc_cache *flow_cache)
*
* 2. Before processing of the new packet batch:
* - dpif_netdev_execute()
- * - dp_netdev_input__()
+ * - dp_netdev_process_rxq_port()
*
* 3. At least once per polling iteration in main polling threads if no
*packets received on current iteration:
@@ -2963,6 +2967,7 @@ dpif_netdev_execute(struct dpif *dpif, struct
dpif_execute *execute)
dp_packet_batch_init_packet(&pp, execute->packet);
dp_netdev_execute_actions(pmd, &pp, false, execute->flow,
execute->actions, execute->actions_len);
+dp_netdev_pmd_flush_output_packets(pmd);
if (pmd->core_id == NON_PMD_CORE_ID) {
ovs_mutex_unlock(&dp->non_pmd_mutex);
@@ -3249,6 +3254,36 @@ dp_netdev_rxq_get_intrvl_cycles(struct dp_netdev_rxq
*rx, unsigned idx)
return processing_cycles;
}
+static void
+dp_netdev_pmd_flush_output_on_port(struct dp_netdev_pmd_thread *pmd,
+ struct tx_port *p)
+{
+int tx_qid;
+bool dynamic_txqs;
+
+dynamic_txqs = p->port->dynamic_txqs;
+if (dynamic_txqs) {
+tx_qid = dpif_netdev_xps_get_tx_qid(pmd, p);
+} else {
+tx_qid = pmd->static_tx_qid;
+}
+
+netdev_send(p->port->netdev, tx_qid, &p->output_pkts, true, dynamic_txqs);
+dp_packet_batch_init(&p->output_pkts);
+}
+
+static void
+dp_netdev_pmd_flush_output_packets(struct dp_netdev_pmd_thread *pmd)
+{
+struct tx_port *p;
+
+HMAP_FOR_EACH (p, node, &pmd->send_port_cache) {
+if (!dp_packet_batch_is_empty(&p->output_pkts)) {
+dp_netdev_pmd_flush_output_on_port(pmd, p);
+}
+}
+}
+
static int
dp_netdev_process_rxq_port(struct dp_netdev_pmd_thread *pmd,
struct netdev_rxq *rx,
@@ -3262,9 +3297,11 @@ dp_netdev_process_rxq_port(struct dp_netdev_pmd_thread
*pmd,
error = netdev_rxq_recv(rx, &batch);
if (!error) {
*recirc_depth_get() = 0;
+pmd_thread_ctx_time_update(pmd);
batch_cnt = batch.count;
dp_netdev_input(pmd, &batch, port_no);
+dp_netdev_pmd_flush_output_packets(pmd);
} else if (error != EAGAIN && error != EOPNOTSUPP) {
static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
@@ -4740,6 +4777,7 @@ dp_netdev_add_port_tx_to_pmd(struct dp_netdev_pmd_thread
*pmd,
tx->port = port;
tx->qid = -1;
+dp_packet_batch_init(&tx->output_pkts);
hmap_insert(&pmd->tx_ports, &tx->node, hash_port_no(tx->port->port_no));
pmd->need_reload = true;
@@ -5197,8 +5235,6 @@ dp_netdev_input__(struct dp_netdev_pmd_thread *pmd,
size_t n_batches;
odp_port_t in_port;
-pmd_thread_ctx_time_update(pmd);
-
n_batches = 0;
emc_processing(pmd, packets, keys, batches, &n_batches,
md_is_valid, port_no);
@@ -5412,18 +5448,35 @@ dp_execute_cb(void *aux_, struct dp_packet_batch
*packets
[ovs-dev] [PATCH v7 3/6] netdev: Remove unused may_steal.
Not needed anymore because 'may_steal' already handled on
dpif-netdev layer and always true.
Acked-by: Eelco Chaudron
Signed-off-by: Ilya Maximets
---
lib/dpif-netdev.c | 2 +-
lib/netdev-bsd.c | 4 ++--
lib/netdev-dpdk.c | 25 +++--
lib/netdev-dummy.c| 4 ++--
lib/netdev-linux.c| 4 ++--
lib/netdev-provider.h | 7 +++
lib/netdev.c | 12
lib/netdev.h | 2 +-
8 files changed, 26 insertions(+), 34 deletions(-)
diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index e504ddb..aafaaf8 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -3268,7 +3268,7 @@ dp_netdev_pmd_flush_output_on_port(struct
dp_netdev_pmd_thread *pmd,
tx_qid = pmd->static_tx_qid;
}
-netdev_send(p->port->netdev, tx_qid, &p->output_pkts, true, dynamic_txqs);
+netdev_send(p->port->netdev, tx_qid, &p->output_pkts, dynamic_txqs);
dp_packet_batch_init(&p->output_pkts);
}
diff --git a/lib/netdev-bsd.c b/lib/netdev-bsd.c
index 65c5674..658a5bc 100644
--- a/lib/netdev-bsd.c
+++ b/lib/netdev-bsd.c
@@ -680,7 +680,7 @@ netdev_bsd_rxq_drain(struct netdev_rxq *rxq_)
*/
static int
netdev_bsd_send(struct netdev *netdev_, int qid OVS_UNUSED,
-struct dp_packet_batch *batch, bool may_steal,
+struct dp_packet_batch *batch,
bool concurrent_txq OVS_UNUSED)
{
struct netdev_bsd *dev = netdev_bsd_cast(netdev_);
@@ -728,7 +728,7 @@ netdev_bsd_send(struct netdev *netdev_, int qid OVS_UNUSED,
}
ovs_mutex_unlock(&dev->mutex);
-dp_packet_delete_batch(batch, may_steal);
+dp_packet_delete_batch(batch, true);
return error;
}
diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index 8f22264..c2aebdf 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -1892,12 +1892,12 @@ dpdk_do_tx_copy(struct netdev *netdev, int qid, struct
dp_packet_batch *batch)
static int
netdev_dpdk_vhost_send(struct netdev *netdev, int qid,
struct dp_packet_batch *batch,
- bool may_steal, bool concurrent_txq OVS_UNUSED)
+ bool concurrent_txq OVS_UNUSED)
{
-if (OVS_UNLIKELY(!may_steal || batch->packets[0]->source != DPBUF_DPDK)) {
+if (OVS_UNLIKELY(batch->packets[0]->source != DPBUF_DPDK)) {
dpdk_do_tx_copy(netdev, qid, batch);
-dp_packet_delete_batch(batch, may_steal);
+dp_packet_delete_batch(batch, true);
} else {
dp_packet_batch_apply_cutlen(batch);
__netdev_dpdk_vhost_send(netdev, qid, batch->packets, batch->count);
@@ -1907,11 +1907,11 @@ netdev_dpdk_vhost_send(struct netdev *netdev, int qid,
static inline void
netdev_dpdk_send__(struct netdev_dpdk *dev, int qid,
- struct dp_packet_batch *batch, bool may_steal,
+ struct dp_packet_batch *batch,
bool concurrent_txq)
{
if (OVS_UNLIKELY(!(dev->flags & NETDEV_UP))) {
-dp_packet_delete_batch(batch, may_steal);
+dp_packet_delete_batch(batch, true);
return;
}
@@ -1920,12 +1920,11 @@ netdev_dpdk_send__(struct netdev_dpdk *dev, int qid,
rte_spinlock_lock(&dev->tx_q[qid].tx_lock);
}
-if (OVS_UNLIKELY(!may_steal ||
- batch->packets[0]->source != DPBUF_DPDK)) {
+if (OVS_UNLIKELY(batch->packets[0]->source != DPBUF_DPDK)) {
struct netdev *netdev = &dev->up;
dpdk_do_tx_copy(netdev, qid, batch);
-dp_packet_delete_batch(batch, may_steal);
+dp_packet_delete_batch(batch, true);
} else {
int tx_cnt, dropped;
int batch_cnt = dp_packet_batch_size(batch);
@@ -1953,12 +1952,11 @@ netdev_dpdk_send__(struct netdev_dpdk *dev, int qid,
static int
netdev_dpdk_eth_send(struct netdev *netdev, int qid,
- struct dp_packet_batch *batch, bool may_steal,
- bool concurrent_txq)
+ struct dp_packet_batch *batch, bool concurrent_txq)
{
struct netdev_dpdk *dev = netdev_dpdk_cast(netdev);
-netdev_dpdk_send__(dev, qid, batch, may_steal, concurrent_txq);
+netdev_dpdk_send__(dev, qid, batch, concurrent_txq);
return 0;
}
@@ -2933,8 +2931,7 @@ dpdk_ring_open(const char dev_name[], dpdk_port_t
*eth_port_id)
static int
netdev_dpdk_ring_send(struct netdev *netdev, int qid,
- struct dp_packet_batch *batch, bool may_steal,
- bool concurrent_txq)
+ struct dp_packet_batch *batch, bool concurrent_txq)
{
struct netdev_dpdk *dev = netdev_dpdk_cast(netdev);
struct dp_packet *packet;
@@ -2947,7 +2944,7 @@ netdev_dpdk_ring_send(struct netdev *netdev, int qid,
dp_packet_mbuf_rss_flag_reset(packet);
}
-netdev_dpdk_send__(dev, qid, batch, may_steal, concurrent_txq);
+netdev_dpdk_send__(dev, qid, batch, concurrent_txq);
return 0;
}
diff --git a/lib/netdev-dummy.c b/lib/netdev-
[ovs-dev] [PATCH v7 4/6] netdev: Remove useless cutlen.
Cutlen already applied while processing OVS_ACTION_ATTR_OUTPUT.
Acked-by: Eelco Chaudron
Signed-off-by: Ilya Maximets
---
lib/netdev-bsd.c | 2 +-
lib/netdev-dpdk.c | 5 -
lib/netdev-dummy.c | 2 +-
lib/netdev-linux.c | 4 ++--
4 files changed, 4 insertions(+), 9 deletions(-)
diff --git a/lib/netdev-bsd.c b/lib/netdev-bsd.c
index 658a5bc..05974c1 100644
--- a/lib/netdev-bsd.c
+++ b/lib/netdev-bsd.c
@@ -697,7 +697,7 @@ netdev_bsd_send(struct netdev *netdev_, int qid OVS_UNUSED,
for (i = 0; i < batch->count; i++) {
const void *data = dp_packet_data(batch->packets[i]);
-size_t size = dp_packet_get_send_len(batch->packets[i]);
+size_t size = dp_packet_size(batch->packets[i]);
while (!error) {
ssize_t retval;
diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index c2aebdf..2325f0e 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -1843,8 +1843,6 @@ dpdk_do_tx_copy(struct netdev *netdev, int qid, struct
dp_packet_batch *batch)
dropped += batch_cnt - cnt;
}
-dp_packet_batch_apply_cutlen(batch);
-
uint32_t txcnt = 0;
for (uint32_t i = 0; i < cnt; i++) {
@@ -1899,7 +1897,6 @@ netdev_dpdk_vhost_send(struct netdev *netdev, int qid,
dpdk_do_tx_copy(netdev, qid, batch);
dp_packet_delete_batch(batch, true);
} else {
-dp_packet_batch_apply_cutlen(batch);
__netdev_dpdk_vhost_send(netdev, qid, batch->packets, batch->count);
}
return 0;
@@ -1930,8 +1927,6 @@ netdev_dpdk_send__(struct netdev_dpdk *dev, int qid,
int batch_cnt = dp_packet_batch_size(batch);
struct rte_mbuf **pkts = (struct rte_mbuf **) batch->packets;
-dp_packet_batch_apply_cutlen(batch);
-
tx_cnt = netdev_dpdk_filter_packet_len(dev, pkts, batch_cnt);
tx_cnt = netdev_dpdk_qos_run(dev, pkts, tx_cnt, true);
dropped = batch_cnt - tx_cnt;
diff --git a/lib/netdev-dummy.c b/lib/netdev-dummy.c
index 140e08d..40086a3 100644
--- a/lib/netdev-dummy.c
+++ b/lib/netdev-dummy.c
@@ -1071,7 +1071,7 @@ netdev_dummy_send(struct netdev *netdev, int qid
OVS_UNUSED,
struct dp_packet *packet;
DP_PACKET_BATCH_FOR_EACH(packet, batch) {
const void *buffer = dp_packet_data(packet);
-size_t size = dp_packet_get_send_len(packet);
+size_t size = dp_packet_size(packet);
if (batch->packets[i]->packet_type != htonl(PT_ETH)) {
error = EPFNOSUPPORT;
diff --git a/lib/netdev-linux.c b/lib/netdev-linux.c
index 634ff6f..e767567 100644
--- a/lib/netdev-linux.c
+++ b/lib/netdev-linux.c
@@ -1198,7 +1198,7 @@ netdev_linux_sock_batch_send(int sock, int ifindex,
struct dp_packet *packet;
DP_PACKET_BATCH_FOR_EACH (packet, batch) {
iov[i].iov_base = dp_packet_data(packet);
-iov[i].iov_len = dp_packet_get_send_len(packet);
+iov[i].iov_len = dp_packet_size(packet);
mmsg[i].msg_hdr = (struct msghdr) { .msg_name = &sll,
.msg_namelen = sizeof sll,
.msg_iov = &iov[i],
@@ -1235,7 +1235,7 @@ netdev_linux_tap_batch_send(struct netdev *netdev_,
struct netdev_linux *netdev = netdev_linux_cast(netdev_);
struct dp_packet *packet;
DP_PACKET_BATCH_FOR_EACH (packet, batch) {
-size_t size = dp_packet_get_send_len(packet);
+size_t size = dp_packet_size(packet);
ssize_t retval;
int error;
--
2.7.4
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH v7 5/6] dpif-netdev: Time based output batching.
This allows to collect packets from more than one RX burst
and send them together with a configurable intervals.
'other_config:tx-flush-interval' can be used to configure
time that a packet can wait in output batch for sending.
dpif-netdev turned to microsecond resolution for time
measuring to ensure desired resolution of 'tx-flush-interval'.
Acked-by: Eelco Chaudron
Signed-off-by: Ilya Maximets
---
lib/dpif-netdev.c| 149 +++
vswitchd/vswitch.xml | 16 ++
2 files changed, 131 insertions(+), 34 deletions(-)
diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index aafaaf8..edd6f12 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -85,6 +85,9 @@ VLOG_DEFINE_THIS_MODULE(dpif_netdev);
#define MAX_RECIRC_DEPTH 6
DEFINE_STATIC_PER_THREAD_DATA(uint32_t, recirc_depth, 0)
+/* Use instant packet send by default. */
+#define DEFAULT_TX_FLUSH_INTERVAL 0
+
/* Configuration parameters. */
enum { MAX_FLOWS = 65536 }; /* Maximum number of flows in flow table. */
enum { MAX_METERS = 65536 };/* Maximum number of meters. */
@@ -178,12 +181,13 @@ struct emc_cache {
/* Simple non-wildcarding single-priority classifier. */
-/* Time in ms between successive optimizations of the dpcls subtable vector */
-#define DPCLS_OPTIMIZATION_INTERVAL 1000
+/* Time in microseconds between successive optimizations of the dpcls
+ * subtable vector */
+#define DPCLS_OPTIMIZATION_INTERVAL 100LL
-/* Time in ms of the interval in which rxq processing cycles used in
- * rxq to pmd assignments is measured and stored. */
-#define PMD_RXQ_INTERVAL_LEN 1
+/* Time in microseconds of the interval in which rxq processing cycles used
+ * in rxq to pmd assignments is measured and stored. */
+#define PMD_RXQ_INTERVAL_LEN 1000LL
/* Number of intervals for which cycles are stored
* and used during rxq to pmd assignment. */
@@ -270,6 +274,9 @@ struct dp_netdev {
struct hmap ports;
struct seq *port_seq; /* Incremented whenever a port changes. */
+/* The time that a packet can wait in output batch for sending. */
+atomic_uint32_t tx_flush_interval;
+
/* Meters. */
struct ovs_mutex meter_locks[N_METER_LOCKS];
struct dp_meter *meters[MAX_METERS]; /* Meter bands. */
@@ -356,7 +363,7 @@ enum rxq_cycles_counter_type {
RXQ_N_CYCLES
};
-#define XPS_TIMEOUT_MS 500LL
+#define XPS_TIMEOUT 50LL/* In microseconds. */
/* Contained by struct dp_netdev_port's 'rxqs' member. */
struct dp_netdev_rxq {
@@ -526,6 +533,7 @@ struct tx_port {
int qid;
long long last_used;
struct hmap_node node;
+long long flush_time;
struct dp_packet_batch output_pkts;
};
@@ -612,6 +620,9 @@ struct dp_netdev_pmd_thread {
* than 'cmap_count(dp->poll_threads)'. */
uint32_t static_tx_qid;
+/* Number of filled output batches. */
+int n_output_batches;
+
struct ovs_mutex port_mutex;/* Mutex for 'poll_list' and 'tx_ports'. */
/* List of rx queues to poll. */
struct hmap poll_list OVS_GUARDED;
@@ -705,8 +716,9 @@ static void dp_netdev_add_rxq_to_pmd(struct
dp_netdev_pmd_thread *pmd,
static void dp_netdev_del_rxq_from_pmd(struct dp_netdev_pmd_thread *pmd,
struct rxq_poll *poll)
OVS_REQUIRES(pmd->port_mutex);
-static void
-dp_netdev_pmd_flush_output_packets(struct dp_netdev_pmd_thread *pmd);
+static int
+dp_netdev_pmd_flush_output_packets(struct dp_netdev_pmd_thread *pmd,
+ bool force);
static void reconfigure_datapath(struct dp_netdev *dp)
OVS_REQUIRES(dp->port_mutex);
@@ -797,7 +809,7 @@ emc_cache_slow_sweep(struct emc_cache *flow_cache)
static inline void
pmd_thread_ctx_time_update(struct dp_netdev_pmd_thread *pmd)
{
-pmd->ctx.now = time_msec();
+pmd->ctx.now = time_usec();
}
/* Returns true if 'dpif' is a netdev or dummy dpif, false otherwise. */
@@ -1297,6 +1309,7 @@ create_dp_netdev(const char *name, const struct
dpif_class *class,
conntrack_init(&dp->conntrack);
atomic_init(&dp->emc_insert_min, DEFAULT_EM_FLOW_INSERT_MIN);
+atomic_init(&dp->tx_flush_interval, DEFAULT_TX_FLUSH_INTERVAL);
cmap_init(&dp->poll_threads);
@@ -2967,7 +2980,7 @@ dpif_netdev_execute(struct dpif *dpif, struct
dpif_execute *execute)
dp_packet_batch_init_packet(&pp, execute->packet);
dp_netdev_execute_actions(pmd, &pp, false, execute->flow,
execute->actions, execute->actions_len);
-dp_netdev_pmd_flush_output_packets(pmd);
+dp_netdev_pmd_flush_output_packets(pmd, true);
if (pmd->core_id == NON_PMD_CORE_ID) {
ovs_mutex_unlock(&dp->non_pmd_mutex);
@@ -3016,6 +3029,16 @@ dpif_netdev_set_config(struct dpif *dpif, const struct
smap *other_config)
smap_get_ullong(other_config, "emc-insert-inv-prob",
DEFAULT_EM_FLOW_INSERT_INV_PROB);
uint32_t insert_min, cur_min;
+uin
[ovs-dev] [PATCH v7 6/6] dpif-netdev: Count sent packets and batches.
New statistics for 'pmd-stats-show' command:
average number of packets per output batch.
Acked-by: Eelco Chaudron
Signed-off-by: Ilya Maximets
---
lib/dpif-netdev.c | 27 ++-
1 file changed, 22 insertions(+), 5 deletions(-)
diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index edd6f12..5fa1db4 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -344,6 +344,8 @@ enum dp_stat_type {
DP_STAT_LOST, /* Packets not passed up to the client. */
DP_STAT_LOOKUP_HIT, /* Number of subtable lookups for flow table
hits */
+DP_STAT_SENT_PKTS, /* Packets that has been sent. */
+DP_STAT_SENT_BATCHES, /* Number of batches sent. */
DP_N_STATS
};
@@ -515,6 +517,9 @@ struct dp_netdev_pmd_cycles {
atomic_ullong n[PMD_N_CYCLES];
};
+static void dp_netdev_count_packet(struct dp_netdev_pmd_thread *,
+ enum dp_stat_type type, int cnt);
+
struct polled_queue {
struct dp_netdev_rxq *rxq;
odp_port_t port_no;
@@ -846,6 +851,7 @@ pmd_info_show_stats(struct ds *reply,
{
unsigned long long total_packets;
uint64_t total_cycles = 0;
+double lookups_per_hit = 0, packets_per_batch = 0;
int i;
/* These loops subtracts reference values ('*_zero') from the counters.
@@ -887,15 +893,23 @@ pmd_info_show_stats(struct ds *reply,
}
ds_put_cstr(reply, ":\n");
+if (stats[DP_STAT_MASKED_HIT] > 0) {
+lookups_per_hit = stats[DP_STAT_LOOKUP_HIT]
+ / (double) stats[DP_STAT_MASKED_HIT];
+}
+if (stats[DP_STAT_SENT_BATCHES] > 0) {
+packets_per_batch = stats[DP_STAT_SENT_PKTS]
+/ (double) stats[DP_STAT_SENT_BATCHES];
+}
+
ds_put_format(reply,
"\temc hits:%llu\n\tmegaflow hits:%llu\n"
"\tavg. subtable lookups per hit:%.2f\n"
- "\tmiss:%llu\n\tlost:%llu\n",
+ "\tmiss:%llu\n\tlost:%llu\n"
+ "\tavg. packets per output batch: %.2f\n",
stats[DP_STAT_EXACT_HIT], stats[DP_STAT_MASKED_HIT],
- stats[DP_STAT_MASKED_HIT] > 0
- ? (1.0*stats[DP_STAT_LOOKUP_HIT])/stats[DP_STAT_MASKED_HIT]
- : 0,
- stats[DP_STAT_MISS], stats[DP_STAT_LOST]);
+ lookups_per_hit, stats[DP_STAT_MISS], stats[DP_STAT_LOST],
+ packets_per_batch);
if (total_cycles == 0) {
return;
@@ -3306,6 +3320,9 @@ dp_netdev_pmd_flush_output_on_port(struct
dp_netdev_pmd_thread *pmd,
ovs_assert(pmd->n_output_batches > 0);
pmd->n_output_batches--;
+dp_netdev_count_packet(pmd, DP_STAT_SENT_PKTS, output_cnt);
+dp_netdev_count_packet(pmd, DP_STAT_SENT_BATCHES, 1);
+
return output_cnt;
}
--
2.7.4
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] dpif-netdev: Refactor datapath flow cache
Hi Jan, This is a very interesting patch with compelling results. I had a few questions after reading the commit message for this patch: How did you decide on 1M as the proposed size for the DFC? Will the size of this DFC be configurable (i.e. map a different number of hash bits)? I was mostly interested in how the performance of the very basic phy2phy testcase would be affected by this change. Below are my results from 1-1 flows, 64B packets, unidirectional with 1 PMD. One OpenFlow rule is installed for each stream of traffic being sent: Flows master DFC+EMC Gain [Mpps] [Mpps] -- 1 12.34 13.126.3% 10 7.838.19 4.5% 100 4.714.78 1.3% 10003.773.83 1.5% 1 3.273.51 7.1% This shows a performance improvement for this testcase also. Tested-by: Cian Ferriter Let me know your thoughts on the above questions. Thanks, Cian > -Original Message- > From: ovs-dev-boun...@openvswitch.org [mailto:ovs-dev- > boun...@openvswitch.org] On Behalf Of Jan Scheurich > Sent: 20 November 2017 17:33 > To: d...@openvswitch.org > Subject: [ovs-dev] [PATCH] dpif-netdev: Refactor datapath flow cache > > So far the netdev datapath uses an 8K EMC to speed up the lookup of > frequently used flows by comparing the parsed packet headers against the > miniflow of a cached flow, using 13 bits of the packet RSS hash as index. The > EMC is too small for many applications with 100K or more parallel packet > flows so that EMC threshing actually degrades performance. > Furthermore, the size of struct miniflow and the flow copying cost prevents > us from making it much larger. > > At the same time the lookup cost of the megaflow classifier (DPCLS) is > increasing as the number of frequently hit subtables grows with the > complexity of pipeline and the number of recirculations. > > To close the performance gap for many parallel flows, this patch introduces > the datapath flow cache (DFC) with 1M entries as lookup stage between EMC > and DPCLS. It directly maps 20 bits of the RSS hash to a pointer to the last > hit > megaflow entry and performs a masked comparison of the packet flow with > the megaflow key to confirm the hit. This avoids the costly DPCLS lookup > even for very large number of parallel flows with a small memory overhead. > > Due the large size of the DFC and the low risk of DFC thrashing, any DPCLS hit > immediately inserts an entry in the DFC so that subsequent packets get > speeded up. The DFC, thus, accelerate also short-lived flows. > > To further accelerate the lookup of few elephant flows, every DFC hit > triggers a probabilistic EMC insertion of the flow. As the DFC entry is > already > in place the default EMC insertion probability can be reduced to > 1/1000 to minimize EMC thrashing should there still be many fat flows. > The inverse EMC insertion probability remains configurable. > > The EMC implementation is simplified by removing the possibility to store a > flow in two slots, as there is no particular reason why two flows should > systematically collide (the RSS hash is not symmetric). > The maximum size of the EMC flow key is limited to 256 bytes to reduce the > memory footprint. This should be sufficient to hold most real life packet flow > keys. Larger flows are not installed in the EMC. > > The pmd-stats-show command is enhanced to show both EMC and DFC hits > separately. > > The sweep speed for cleaning up obsolete EMC and DFC flow entries and > freeing dead megaflow entries is increased. With a typical PMD cycle > duration of 100us under load and checking one DFC entry per cycle, the DFC > sweep should normally complete within in 100s. > > In PVP performance tests with an L3 pipeline over VXLAN we determined the > optimal EMC size to be 16K entries to obtain a uniform speedup compared to > the master branch over the full range of parallel flows. The measurement > below is for 64 byte packets and the average number of subtable lookups per > DPCLS hit in this pipeline is 1.0, i.e. the acceleration already starts for a > single > busy mask. Tests with many visited subtables should show a strong increase > of the gain through DFC. > > Flows master DFC+EMC Gain > [Mpps] [Mpps] > -- > 8 4.454.62 3.8% > 100 4.174.47 7.2% > 10003.884.3412.0% > 20003.544.1717.8% > 50003.013.8227.0% > 1 2.753.6331.9% > 2 2.643.5032.8% > 5 2.603.3328.1% > 10 2.593.2324.7% > 50 2.593.1621.9% > > > Signed-off-by: Jan Scheurich > --- > lib/dpif-netdev.c | 349 --- > --- > 1 file changed, 235 insertions(+), 114 deletions(-) > > diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c index db78318..efcf2e9 > 100644 > --- a/lib/dpif-netdev.c > +++ b/lib/dpif-netdev.c > @@ -127,19 +127,19 @@ struct
[ovs-dev] [RFC] docs: Describe output packet batching in DPDK guide.
Added information about output packet batching and a way to configure 'tx-flush-interval'. Signed-off-by: Ilya Maximets --- This patch is made on top of v7 of Output Batching patch-set: https://mail.openvswitch.org/pipermail/ovs-dev/2017-December/341936.html This patch sent as RFC separately from the rest of the series because it's new and I don't want this to block accepting of the main series. It'll be much easier to fix/rebase just this patch without re-sending of the whole patch-set. Documentation/intro/install/dpdk.rst | 24 1 file changed, 24 insertions(+) diff --git a/Documentation/intro/install/dpdk.rst b/Documentation/intro/install/dpdk.rst index 3fecb5c..b0bb76e 100644 --- a/Documentation/intro/install/dpdk.rst +++ b/Documentation/intro/install/dpdk.rst @@ -568,6 +568,30 @@ not needed i.e. jumbo frames are not needed, it can be forced off by adding chains of descriptors it will make more individual virtio descriptors available for rx to the guest using dpdkvhost ports and this can improve performance. +Output Packet Batching +~~ + +To get advantages of batched send functions OVS collects packets in +intermediate queues before sending. This allows to use single send even for +packets matched by different flows but having same output action. Furthermore, +OVS is able to collect packets for some reasonable amount of time before +sending to use batched send in case where input batches are small. + +``tx-flush-interval`` config could be used to specify the time in microseconds +that a packet can wait in an output queue for sending (default is ``0``):: + +$ ovs-vsctl set Open_vSwitch . other_config:tx-flush-interval=50 + +Lower values decreases latency while higher values may be useful to achieve +higher performance. For example, increasing of ``tx-flush-interval`` can be +used to decrease number of interrupts for interrupt based guest drivers. +This may significantly affect the performance. Zero value means immediate +send at the end of processing of a single input batch. + +Average number of packets per output batch could be checked in PMD stats:: + +$ ovs-appctl dpif-netdev/pmd-stats-show + Limitations -- 2.7.4 ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v3 2/5] conntrack: add commands to r/w CT parameters.
Thanks Antonio for doing all this and pushing it forward.
Regarding patches 2-4:
I understand you want to save some code for various possible set and get
operations.
The prior art for these commands is however not generic set and get commands.
Sometimes, we have specific commands that can take different numbers of
arguments but
those are specific and the context is clear.
If we take some examples, we might see there is an issue with the approach here.
Take per zone limits as one example, which is an internal requirement that we
are working on
across datapaths.
This is a set operation with 2 arguments plus the datapath = 3 arguments.
This won’t work with the generic set functions here.
The corresponding get will not work either.
Trying to make this work generically will get pretty messy and the parameter
validation error prone.
I would like to propose an alternative (a simple one; also with code
consolidation and more error checking) below:
Please let me know what you think.
Thanks Darrell
///
diff --git a/lib/conntrack.c b/lib/conntrack.c
index f5a3aa9..c8ad548 100644
--- a/lib/conntrack.c
+++ b/lib/conntrack.c
@@ -2485,6 +2485,27 @@ conntrack_flush(struct conntrack *ct, const uint16_t
*zone)
return 0;
}
+int
+conntrack_set_maxconns(struct conntrack *ct, uint32_t maxconns)
+{
+atomic_init(&ct->n_conn_limit, maxconns);
+return 0;
+}
+
+int
+conntrack_get_maxconns(struct conntrack *ct, uint32_t *maxconns)
+{
+atomic_read_relaxed(&ct->n_conn_limit, maxconns);
+return 0;
+}
+
+int
+conntrack_get_nconns(struct conntrack *ct, uint32_t *nconns)
+{
+*nconns = atomic_count_get(&ct->n_conn);
+return 0;
+}
+
/* This function must be called with the ct->resources read lock taken. */
static struct alg_exp_node *
expectation_lookup(struct hmap *alg_expectations,
diff --git a/lib/conntrack.h b/lib/conntrack.h
index fbeef1c..8652724 100644
--- a/lib/conntrack.h
+++ b/lib/conntrack.h
@@ -114,6 +114,9 @@ int conntrack_dump_next(struct conntrack_dump *, struct
ct_dpif_entry *);
int conntrack_dump_done(struct conntrack_dump *);
int conntrack_flush(struct conntrack *, const uint16_t *zone);
+int conntrack_set_maxconns(struct conntrack *ct, uint32_t maxconns);
+int conntrack_get_maxconns(struct conntrack *ct, uint32_t *maxconns);
+int conntrack_get_nconns(struct conntrack *ct, uint32_t *nconns);
^L
/* 'struct ct_lock' is a wrapper for an adaptive mutex. It's useful to try
* different types of locks (e.g. spinlocks) */
diff --git a/lib/ct-dpif.c b/lib/ct-dpif.c
index 239c848..5fa3a97 100644
--- a/lib/ct-dpif.c
+++ b/lib/ct-dpif.c
@@ -140,6 +140,30 @@ ct_dpif_flush(struct dpif *dpif, const uint16_t *zone,
: EOPNOTSUPP);
}
+int
+ct_dpif_set_maxconns(struct dpif *dpif, uint32_t maxconns)
+{
+return (dpif->dpif_class->ct_set_maxconns
+? dpif->dpif_class->ct_set_maxconns(dpif, maxconns)
+: EOPNOTSUPP);
+}
+
+int
+ct_dpif_get_maxconns(struct dpif *dpif, uint32_t *maxconns)
+{
+return (dpif->dpif_class->ct_get_maxconns
+? dpif->dpif_class->ct_get_maxconns(dpif, maxconns)
+: EOPNOTSUPP);
+}
+
+int
+ct_dpif_get_nconns(struct dpif *dpif, uint32_t *nconns)
+{
+return (dpif->dpif_class->ct_get_nconns
+? dpif->dpif_class->ct_get_nconns(dpif, nconns)
+: EOPNOTSUPP);
+}
+
void
ct_dpif_entry_uninit(struct ct_dpif_entry *entry)
{
diff --git a/lib/ct-dpif.h b/lib/ct-dpif.h
index 5e2de53..09e7698 100644
--- a/lib/ct-dpif.h
+++ b/lib/ct-dpif.h
@@ -197,6 +197,9 @@ int ct_dpif_dump_next(struct ct_dpif_dump_state *, struct
ct_dpif_entry *);
int ct_dpif_dump_done(struct ct_dpif_dump_state *);
int ct_dpif_flush(struct dpif *, const uint16_t *zone,
const struct ct_dpif_tuple *);
+int ct_dpif_set_maxconns(struct dpif *dpif, uint32_t maxconns);
+int ct_dpif_get_maxconns(struct dpif *dpif, uint32_t *maxconns);
+int ct_dpif_get_nconns(struct dpif *dpif, uint32_t *nconns);
void ct_dpif_entry_uninit(struct ct_dpif_entry *);
void ct_dpif_format_entry(const struct ct_dpif_entry *, struct ds *,
bool verbose, bool print_stats);
diff --git a/lib/dpctl.c b/lib/dpctl.c
index a28ded9..215d1c3 100644
--- a/lib/dpctl.c
+++ b/lib/dpctl.c
@@ -1654,6 +1654,96 @@ dpctl_ct_bkts(int argc, const char *argv[],
return error;
}
^L
+static int
+dpctl_ct_open_dp(int argc, const char *argv[],
+ struct dpctl_params *dpctl_p, struct dpif **dpif)
+{
+int error = 0;
+/* The datapath name is not a mandatory parameter for this command.
+ * If it is not specified - so argc < 3 - we retrieve it from the
+ * current setup, assuming only one exists. */
+char *dpname = argc == 3 ? xstrdup(argv[1]) : get_one_dp(dpctl_p);
+if (!dpname) {
+error = EINVAL;
+dpctl_error(dpctl_p, error, "datapath not found");
+} else {
+error = parsed_dpif_open(dpname, false,
Re: [ovs-dev] [PATCH v4 2/2] netdev-dpdk: Add debug appctl to get mempool information.
>From: Ilya Maximets [mailto:i.maxim...@samsung.com]
>Sent: Monday, December 11, 2017 1:19 PM
>To: ovs-dev@openvswitch.org
>Cc: Heetae Ahn ; Fischetti, Antonio
>; Loftus, Ciara ;
>Kavanagh, Mark B ; Stokes, Ian
>; Wojciechowicz, RobertX
>; Flavio Leitner ; Ilya
>Maximets
>Subject: [PATCH v4 2/2] netdev-dpdk: Add debug appctl to get mempool
>information.
>
>New appctl 'netdev-dpdk/get-mempool-info' implemented to get result
>of 'rte_mempool_list_dump()' function if no arguments passed and
>'rte_mempool_dump()' if DPDK netdev passed as argument.
>
>Could be used for debugging mbuf leaks and other mempool related
>issues. Most useful in pair with `grep -v "cache_count.*=0"`.
>
>Signed-off-by: Ilya Maximets
>---
> NEWS| 1 +
> lib/netdev-dpdk-unixctl.man | 5 +
> lib/netdev-dpdk.c | 54
>+
> 3 files changed, 60 insertions(+)
>
>diff --git a/NEWS b/NEWS
>index 69d5dab..e60514e 100644
>--- a/NEWS
>+++ b/NEWS
>@@ -18,6 +18,7 @@ Post-v2.8.0
>- DPDK:
> * Add support for DPDK v17.11
> * Add support for vHost IOMMU
>+ * New debug appctl command 'netdev-dpdk/get-mempool-info'.
> * All the netdev-dpdk appctl commands described in ovs-vswitchd man
>page.
>
> v2.8.0 - 31 Aug 2017
>diff --git a/lib/netdev-dpdk-unixctl.man b/lib/netdev-dpdk-unixctl.man
>index 5af6eca..ac274cd 100644
>--- a/lib/netdev-dpdk-unixctl.man
>+++ b/lib/netdev-dpdk-unixctl.man
>@@ -7,3 +7,8 @@ If \fIinterface\fR is not specified, then it applies to all
>DPDK ports.
> Detaches device with corresponding \fIpci-address\fR from DPDK. This command
> can be used to detach device if it wasn't detached automatically after port
> deletion. Refer to the documentation for details and instructions.
Hi Ilya,
I would still prefer if the pointer to documentation were more specific;
however, I won't block on that basis alone.
Acked-by: Mark Kavanagh
Tested-by: Mark Kavanagh
Thanks for the series,
Mark
>+.IP "\fBnetdev-dpdk/get-mempool-info\fR [\fIinterface\fR]"
>+Prints the debug information about memory pool used by DPDK \fIinterface\fR.
>+If called without arguments, information of all the available mempools will
>+be printed. For additional mempool statistics enable
>+\fBCONFIG_RTE_LIBRTE_MEMPOOL_DEBUG\fR while building DPDK.
>diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
>index 8f22264..3bf461b 100644
>--- a/lib/netdev-dpdk.c
>+++ b/lib/netdev-dpdk.c
>@@ -2586,6 +2586,56 @@ error:
> free(response);
> }
>
>+static void
>+netdev_dpdk_get_mempool_info(struct unixctl_conn *conn,
>+ int argc, const char *argv[],
>+ void *aux OVS_UNUSED)
>+{
>+size_t size;
>+FILE *stream;
>+char *response = NULL;
>+struct netdev *netdev = NULL;
>+
>+if (argc == 2) {
>+netdev = netdev_from_name(argv[1]);
>+if (!netdev || !is_dpdk_class(netdev->netdev_class)) {
>+unixctl_command_reply_error(conn, "Not a DPDK Interface");
>+goto out;
>+}
>+}
>+
>+stream = open_memstream(&response, &size);
>+if (!stream) {
>+response = xasprintf("Unable to open memstream: %s.",
>+ ovs_strerror(errno));
>+unixctl_command_reply_error(conn, response);
>+goto out;
>+}
>+
>+if (netdev) {
>+struct netdev_dpdk *dev = netdev_dpdk_cast(netdev);
>+
>+ovs_mutex_lock(&dev->mutex);
>+ovs_mutex_lock(&dpdk_mp_mutex);
>+
>+rte_mempool_dump(stream, dev->mp);
>+
>+ovs_mutex_unlock(&dpdk_mp_mutex);
>+ovs_mutex_unlock(&dev->mutex);
>+} else {
>+ovs_mutex_lock(&dpdk_mp_mutex);
>+rte_mempool_list_dump(stream);
>+ovs_mutex_unlock(&dpdk_mp_mutex);
>+}
>+
>+fclose(stream);
>+
>+unixctl_command_reply(conn, response);
>+out:
>+free(response);
>+netdev_close(netdev);
>+}
>+
> /*
> * Set virtqueue flags so that we do not receive interrupts.
> */
>@@ -2842,6 +2892,10 @@ netdev_dpdk_class_init(void)
> "pci address of device", 1, 1,
> netdev_dpdk_detach, NULL);
>
>+unixctl_command_register("netdev-dpdk/get-mempool-info",
>+ "[netdev]", 0, 1,
>+ netdev_dpdk_get_mempool_info, NULL);
>+
> ovsthread_once_done(&once);
> }
>
>--
>2.7.4
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v4 1/2] vswitchd: Document netdev-dpdk commands.
>From: Ilya Maximets [mailto:i.maxim...@samsung.com] >Sent: Monday, December 11, 2017 1:19 PM >To: ovs-dev@openvswitch.org >Cc: Heetae Ahn ; Fischetti, Antonio >; Loftus, Ciara ; >Kavanagh, Mark B ; Stokes, Ian >; Wojciechowicz, RobertX >; Flavio Leitner ; Ilya >Maximets >Subject: [PATCH v4 1/2] vswitchd: Document netdev-dpdk commands. > >netdev-dpdk appctl commands added to man pages. > >Signed-off-by: Ilya Maximets LGTM. Acked-by: Mark Kavanagh >--- > NEWS| 1 + > lib/automake.mk | 1 + > lib/netdev-dpdk-unixctl.man | 9 + > manpages.mk | 2 ++ > vswitchd/ovs-vswitchd.8.in | 1 + > 5 files changed, 14 insertions(+) > create mode 100644 lib/netdev-dpdk-unixctl.man > >diff --git a/NEWS b/NEWS >index d45904e..69d5dab 100644 >--- a/NEWS >+++ b/NEWS >@@ -18,6 +18,7 @@ Post-v2.8.0 >- DPDK: > * Add support for DPDK v17.11 > * Add support for vHost IOMMU >+ * All the netdev-dpdk appctl commands described in ovs-vswitchd man >page. > > v2.8.0 - 31 Aug 2017 > >diff --git a/lib/automake.mk b/lib/automake.mk >index effe5b5..4b38a11 100644 >--- a/lib/automake.mk >+++ b/lib/automake.mk >@@ -465,6 +465,7 @@ MAN_FRAGMENTS += \ > lib/db-ctl-base.man \ > lib/dpctl.man \ > lib/memory-unixctl.man \ >+ lib/netdev-dpdk-unixctl.man \ > lib/ofp-version.man \ > lib/ovs.tmac \ > lib/service.man \ >diff --git a/lib/netdev-dpdk-unixctl.man b/lib/netdev-dpdk-unixctl.man >new file mode 100644 >index 000..5af6eca >--- /dev/null >+++ b/lib/netdev-dpdk-unixctl.man >@@ -0,0 +1,9 @@ >+.SS "NETDEV-DPDK COMMANDS" >+These commands manage DPDK related ports (\fBtype=\fR\fIdpdk*\fR). >+.IP "\fBnetdev-dpdk/set-admin-state\fR [\fIinterface\fR] \fBup\fR | >\fBdown\fR" >+Change the admin state for DPDK \fIinterface\fR to \fBup\fR or \fBdown\fR. >+If \fIinterface\fR is not specified, then it applies to all DPDK ports. >+.IP "\fBnetdev-dpdk/detach\fR \fIpci-address\fR" >+Detaches device with corresponding \fIpci-address\fR from DPDK. This command >+can be used to detach device if it wasn't detached automatically after port >+deletion. Refer to the documentation for details and instructions. >diff --git a/manpages.mk b/manpages.mk >index d610d88..c89bc45 100644 >--- a/manpages.mk >+++ b/manpages.mk >@@ -279,6 +279,7 @@ vswitchd/ovs-vswitchd.8: \ > lib/daemon.man \ > lib/dpctl.man \ > lib/memory-unixctl.man \ >+ lib/netdev-dpdk-unixctl.man \ > lib/service.man \ > lib/ssl-bootstrap.man \ > lib/ssl.man \ >@@ -296,6 +297,7 @@ lib/coverage-unixctl.man: > lib/daemon.man: > lib/dpctl.man: > lib/memory-unixctl.man: >+lib/netdev-dpdk-unixctl.man: > lib/service.man: > lib/ssl-bootstrap.man: > lib/ssl.man: >diff --git a/vswitchd/ovs-vswitchd.8.in b/vswitchd/ovs-vswitchd.8.in >index c18baf6..76ccfcb 100644 >--- a/vswitchd/ovs-vswitchd.8.in >+++ b/vswitchd/ovs-vswitchd.8.in >@@ -283,6 +283,7 @@ port names, which this thread polls. > .IP "\fBdpif-netdev/pmd-rxq-rebalance\fR [\fIdp\fR]" > Reassigns rxqs to pmds in the datapath \fIdp\fR based on their current usage. > . >+.so lib/netdev-dpdk-unixctl.man > .so ofproto/ofproto-dpif-unixctl.man > .so ofproto/ofproto-unixctl.man > .so lib/vlog-unixctl.man >-- >2.7.4 ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] dpif-netdev: Refactor datapath flow cache
Hi Cian, Thank you for your feedback and the interesting measurement results. As first I was a bit surprised that the gain is smaller in your P2P test case. I guess that is because your traffic passes the datapath (and hence EMC) only once, while in our PVP test with VXLAN each packet causes three passes and EMC lookups, so the effective number of occupied EMC entries is three times as high. Could you test your scenario with significantly more flows to confirm the larger gain with growing number of flows? I set the DFC size by macro to 1M entries mainly because it gave good acceleration all across the entire range of parallel flows we typically test in our NFVI setup (up to 500K flows). The static memory overhead of a 1M DFC is not very high (8 MB per PMD). From that perspective there seemed little need to offer a knob to tune the size for certain use cases. However, if there are sufficient parallel flows to completely fill the DFC, the 8MB L3 cache footprint per PMD is significant and may limit the performance gain across all PMDs and cause additional L3 cache contention with guest applications (VNFs) running on the compute host. So, the main motivation for making the DFC size configurable (or to disable it completely) might be to limit the L3 cache footprint of a PMD. There might be possibly more effective tools on CPU level (e.g. "cache QoS") to achieve the same effect. I am open for discussions on this topic and would welcome multi-PMD measurements. Regards, Jan > -Original Message- > From: Ferriter, Cian [mailto:cian.ferri...@intel.com] > Sent: Monday, 11 December, 2017 17:29 > To: Jan Scheurich ; d...@openvswitch.org > Subject: RE: [PATCH] dpif-netdev: Refactor datapath flow cache > > Hi Jan, > > This is a very interesting patch with compelling results. > > I had a few questions after reading the commit message for this patch: > How did you decide on 1M as the proposed size for the DFC? > Will the size of this DFC be configurable (i.e. map a different number of > hash bits)? > > I was mostly interested in how the performance of the very basic phy2phy > testcase would be affected by this change. > Below are my results from 1-1 flows, 64B packets, unidirectional with 1 > PMD. One OpenFlow rule is installed for each stream of > traffic being sent: > Flows master DFC+EMC Gain > [Mpps] [Mpps] > -- > 1 12.34 13.126.3% > 10 7.838.19 4.5% > 100 4.714.78 1.3% > 10003.773.83 1.5% > 1 3.273.51 7.1% > > This shows a performance improvement for this testcase also. > > Tested-by: Cian Ferriter > > Let me know your thoughts on the above questions. > Thanks, > Cian > > > -Original Message- > > From: ovs-dev-boun...@openvswitch.org [mailto:ovs-dev- > > boun...@openvswitch.org] On Behalf Of Jan Scheurich > > Sent: 20 November 2017 17:33 > > To: d...@openvswitch.org > > Subject: [ovs-dev] [PATCH] dpif-netdev: Refactor datapath flow cache > > > > So far the netdev datapath uses an 8K EMC to speed up the lookup of > > frequently used flows by comparing the parsed packet headers against the > > miniflow of a cached flow, using 13 bits of the packet RSS hash as index. > > The > > EMC is too small for many applications with 100K or more parallel packet > > flows so that EMC threshing actually degrades performance. > > Furthermore, the size of struct miniflow and the flow copying cost prevents > > us from making it much larger. > > > > At the same time the lookup cost of the megaflow classifier (DPCLS) is > > increasing as the number of frequently hit subtables grows with the > > complexity of pipeline and the number of recirculations. > > > > To close the performance gap for many parallel flows, this patch introduces > > the datapath flow cache (DFC) with 1M entries as lookup stage between EMC > > and DPCLS. It directly maps 20 bits of the RSS hash to a pointer to the > > last hit > > megaflow entry and performs a masked comparison of the packet flow with > > the megaflow key to confirm the hit. This avoids the costly DPCLS lookup > > even for very large number of parallel flows with a small memory overhead. > > > > Due the large size of the DFC and the low risk of DFC thrashing, any DPCLS > > hit > > immediately inserts an entry in the DFC so that subsequent packets get > > speeded up. The DFC, thus, accelerate also short-lived flows. > > > > To further accelerate the lookup of few elephant flows, every DFC hit > > triggers a probabilistic EMC insertion of the flow. As the DFC entry is > > already > > in place the default EMC insertion probability can be reduced to > > 1/1000 to minimize EMC thrashing should there still be many fat flows. > > The inverse EMC insertion probability remains configurable. > > > > The EMC implementation is simplified by removing the possibility to store a > > flow in two slots, as there is no particular reason why two flows should >
Re: [ovs-dev] [PATCH v3 5/5] doc: ConnTracker cfg parameters.
Thanks Antonio for doing this. 1/ Given the comments on patches 2-4, I think the documentation would change in dpctl.man to be attribute specific, if we go that route. I did not write it up yet, but most of it would be obvious. One exception is how a case where setting a limit is handled when the limit is already exceeded – this needs documentation. I think the simple and robust approach is to set the attribute regardless without affecting existing connections. When existing connections time out, the limit would be enforced. This is what the proposed code does. 2/ I also think the userspace connection tracker documentation does not belong in dpdk documentation. Part of the content in intro/install/dpdk.rst could be moved to dpctl.man. dpctl.man is pulled into ovs-vswitchd.8.pdf. 3/ The documentation in dpctl.man would mention that support is presently only in the userspace connection tracker. Thanks Darrell On 10/13/17, 1:28 AM, "ovs-dev-boun...@openvswitch.org on behalf of antonio.fische...@intel.com" wrote: Update documentation with the new commands to Read/Write ConnTracker configuration parameters. CC: Kevin Traynor CC: Darrell Ball Signed-off-by: Antonio Fischetti --- Documentation/intro/install/dpdk.rst | 25 + lib/dpctl.man| 10 ++ 2 files changed, 35 insertions(+) diff --git a/Documentation/intro/install/dpdk.rst b/Documentation/intro/install/dpdk.rst index bb69ae5..a1f259c 100644 --- a/Documentation/intro/install/dpdk.rst +++ b/Documentation/intro/install/dpdk.rst @@ -568,6 +568,31 @@ not needed i.e. jumbo frames are not needed, it can be forced off by adding chains of descriptors it will make more individual virtio descriptors available for rx to the guest using dpdkvhost ports and this can improve performance. +Connection Tracker +~~ + +When the Connection Tracker is enabled the overall performance can be deeply +affected, even with simple firewall rules and with stateless protocols like +UDP. In order to find a better tuning, commands like + +:: + +$ ovs-appctl dpctl/ct-get-glbl-cfg +$ ovs-appctl dpctl/ct-set-glbl-cfg = + +allow respectively to read the current value, or set a new value to a +configuration parameter. +For example, to reduce the impact of the Connection Tracker load on the +system performance, the maximum number of tracked connections can be +reduced. + +The available configuration parameters are: + +- maxconn: Maximum number of connections managed by the Connection Tracker + module. It's both readable and writeable. +- totconn: Total number of connections currently managed by the Connection + Tracker module. Readable only. + Limitations diff --git a/lib/dpctl.man b/lib/dpctl.man index 675fe5a..64ad105 100644 --- a/lib/dpctl.man +++ b/lib/dpctl.man @@ -235,3 +235,13 @@ For each ConnTracker bucket, displays the number of connections used by \fIdp\fR. If \fBgt=\fIThreshold\fR is specified, bucket numbers are displayed when the number of connections in a bucket is greater than \fIThreshold\fR. +. +.TP +\*(DX\fBct\-get\-glbl\-cfg\fR [\fIdp\fR] \fBparam\fR +Read the current value of the specified ConnTracker parameter used +by \fIdp\fR. +. +.TP +\*(DX\fBct\-set\-glbl\-cfg\fR [\fIdp\fR] \fBparam=\fI..\fR +Set a value to the specified ConnTracker parameter used +by \fIdp\fR. -- 2.4.11 ___ dev mailing list d...@openvswitch.org https://urldefense.proofpoint.com/v2/url?u=https-3A__mail.openvswitch.org_mailman_listinfo_ovs-2Ddev&d=DwICAg&c=uilaK90D4TOVoH58JNXRgQ&r=BVhFA09CGX7JQ5Ih-uZnsw&m=vXZ1YIrzm8yx9y_G6RlRqBJPOyEO6liY9bXSHzA0uAE&s=PHKAZck2m0ZlG-WVDIVcLP56XP-S94YZ2m0pGqDmjPc&e= ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v4 2/2] netdev-dpdk: Add debug appctl to get mempool information.
Still LGTM, please add
Acked-by: Antonio Fischetti
> -Original Message-
> From: Kavanagh, Mark B
> Sent: Monday, December 11, 2017 4:37 PM
> To: Ilya Maximets ; ovs-dev@openvswitch.org
> Cc: Heetae Ahn ; Fischetti, Antonio
> ; Loftus, Ciara ;
> Stokes, Ian ; Wojciechowicz, RobertX
> ; Flavio Leitner
> Subject: RE: [PATCH v4 2/2] netdev-dpdk: Add debug appctl to get mempool
> information.
>
> >From: Ilya Maximets [mailto:i.maxim...@samsung.com]
> >Sent: Monday, December 11, 2017 1:19 PM
> >To: ovs-dev@openvswitch.org
> >Cc: Heetae Ahn ; Fischetti, Antonio
> >; Loftus, Ciara ;
> >Kavanagh, Mark B ; Stokes, Ian
> >; Wojciechowicz, RobertX
> >; Flavio Leitner ;
> Ilya
> >Maximets
> >Subject: [PATCH v4 2/2] netdev-dpdk: Add debug appctl to get mempool
> >information.
> >
> >New appctl 'netdev-dpdk/get-mempool-info' implemented to get result
> >of 'rte_mempool_list_dump()' function if no arguments passed and
> >'rte_mempool_dump()' if DPDK netdev passed as argument.
> >
> >Could be used for debugging mbuf leaks and other mempool related
> >issues. Most useful in pair with `grep -v "cache_count.*=0"`.
> >
> >Signed-off-by: Ilya Maximets
> >---
> > NEWS| 1 +
> > lib/netdev-dpdk-unixctl.man | 5 +
> > lib/netdev-dpdk.c | 54
> >+
> > 3 files changed, 60 insertions(+)
> >
> >diff --git a/NEWS b/NEWS
> >index 69d5dab..e60514e 100644
> >--- a/NEWS
> >+++ b/NEWS
> >@@ -18,6 +18,7 @@ Post-v2.8.0
> >- DPDK:
> > * Add support for DPDK v17.11
> > * Add support for vHost IOMMU
> >+ * New debug appctl command 'netdev-dpdk/get-mempool-info'.
> > * All the netdev-dpdk appctl commands described in ovs-vswitchd
> man
> >page.
> >
> > v2.8.0 - 31 Aug 2017
> >diff --git a/lib/netdev-dpdk-unixctl.man b/lib/netdev-dpdk-unixctl.man
> >index 5af6eca..ac274cd 100644
> >--- a/lib/netdev-dpdk-unixctl.man
> >+++ b/lib/netdev-dpdk-unixctl.man
> >@@ -7,3 +7,8 @@ If \fIinterface\fR is not specified, then it applies to
> all
> >DPDK ports.
> > Detaches device with corresponding \fIpci-address\fR from DPDK. This
> command
> > can be used to detach device if it wasn't detached automatically after
> port
> > deletion. Refer to the documentation for details and instructions.
>
> Hi Ilya,
>
> I would still prefer if the pointer to documentation were more specific;
> however, I won't block on that basis alone.
>
> Acked-by: Mark Kavanagh
> Tested-by: Mark Kavanagh
>
> Thanks for the series,
> Mark
>
> >+.IP "\fBnetdev-dpdk/get-mempool-info\fR [\fIinterface\fR]"
> >+Prints the debug information about memory pool used by DPDK
> \fIinterface\fR.
> >+If called without arguments, information of all the available mempools
> will
> >+be printed. For additional mempool statistics enable
> >+\fBCONFIG_RTE_LIBRTE_MEMPOOL_DEBUG\fR while building DPDK.
> >diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
> >index 8f22264..3bf461b 100644
> >--- a/lib/netdev-dpdk.c
> >+++ b/lib/netdev-dpdk.c
> >@@ -2586,6 +2586,56 @@ error:
> > free(response);
> > }
> >
> >+static void
> >+netdev_dpdk_get_mempool_info(struct unixctl_conn *conn,
> >+ int argc, const char *argv[],
> >+ void *aux OVS_UNUSED)
> >+{
> >+size_t size;
> >+FILE *stream;
> >+char *response = NULL;
> >+struct netdev *netdev = NULL;
> >+
> >+if (argc == 2) {
> >+netdev = netdev_from_name(argv[1]);
> >+if (!netdev || !is_dpdk_class(netdev->netdev_class)) {
> >+unixctl_command_reply_error(conn, "Not a DPDK Interface");
> >+goto out;
> >+}
> >+}
> >+
> >+stream = open_memstream(&response, &size);
> >+if (!stream) {
> >+response = xasprintf("Unable to open memstream: %s.",
> >+ ovs_strerror(errno));
> >+unixctl_command_reply_error(conn, response);
> >+goto out;
> >+}
> >+
> >+if (netdev) {
> >+struct netdev_dpdk *dev = netdev_dpdk_cast(netdev);
> >+
> >+ovs_mutex_lock(&dev->mutex);
> >+ovs_mutex_lock(&dpdk_mp_mutex);
> >+
> >+rte_mempool_dump(stream, dev->mp);
> >+
> >+ovs_mutex_unlock(&dpdk_mp_mutex);
> >+ovs_mutex_unlock(&dev->mutex);
> >+} else {
> >+ovs_mutex_lock(&dpdk_mp_mutex);
> >+rte_mempool_list_dump(stream);
> >+ovs_mutex_unlock(&dpdk_mp_mutex);
> >+}
> >+
> >+fclose(stream);
> >+
> >+unixctl_command_reply(conn, response);
> >+out:
> >+free(response);
> >+netdev_close(netdev);
> >+}
> >+
> > /*
> > * Set virtqueue flags so that we do not receive interrupts.
> > */
> >@@ -2842,6 +2892,10 @@ netdev_dpdk_class_init(void)
> > "pci address of device", 1, 1,
> > netdev_dpdk_detach, NULL);
> >
> >+unixctl_command_register("netdev-dpdk/get-mempool-info",
> >+ "[netdev]", 0, 1,
> >+
[ovs-dev] [PATCH] MSI: Use platform specific netcfg location
We use the command `netcfg` to install the Windows datapath. Since we have both 32 and 64 bit installers available point it to the platform specific binary. Found while testing. Signed-off-by: Alin Gabriel Serdean --- windows/ovs-windows-installer/CustomActions.wxs | 9 +++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/windows/ovs-windows-installer/CustomActions.wxs b/windows/ovs-windows-installer/CustomActions.wxs index 422f951..cc1b80e 100644 --- a/windows/ovs-windows-installer/CustomActions.wxs +++ b/windows/ovs-windows-installer/CustomActions.wxs @@ -16,6 +16,11 @@ under the License. --> + + + + + http://schemas.microsoft.com/wix/2006/wi";> @@ -35,14 +40,14 @@ JScriptCall="runCommandAction" Execute="deferred" Return="check" Impersonate="no" /> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v6 0/4] nsh: add new nsh key ttl and action dec_nsh_ttl
On 12/10/2017 4:39 PM, Yang, Yi wrote: On Sat, Dec 09, 2017 at 12:42:32AM +0800, Gregory Rose wrote: On 12/8/2017 6:04 AM, Yi Yang wrote: v5->v6 - Rebase v5 to master - Refactor netlink message format to align to NSH kernel implementation - Add dec_nsh_ttl unit test into tests/nsh.at - Fix unit test unstable issue I don't see any backport of the upstream kernel datapath changes? I'm working on catching our out of tree datapath code with the upstream Linux kernel datapath and your patch (commit b2d0f5d5dc53532e6f07bc546a476a55ebdfe0f3 " openvswitch: enable NSH support") needs the backport as well as compat layer changes. Do you plan on doing that work? Thanks, - Greg Greg, yes, I'll backport kernel datapath patches once this patch series is merged. BTW this doesn't depend on the kernel datapath patches. Understood that this series of patches you sent does not require the kernel datapath backport but I found that the kernel datapath patches do modify the openvswitch uapi header and adds new switch cases. These need to be handled eventually which can be done when you do the backport. Thanks for coordinating! - Greg ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH] dpif-netdev: Avoid "sparse" warning.
"sparse" warns when odp_port_t is used directly in an inequality
comparison. This avoids the warning.
CC: Kevin Traynor
Fixes: a130f1a89bd8 ("dpif-netdev: Add port/queue tiebreaker to
rxq_cycle_sort.")
Signed-off-by: Ben Pfaff
---
lib/dpif-netdev.c | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index 43f6a7857412..68fb3925bcf1 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -3444,8 +3444,10 @@ compare_rxq_cycles(const void *a, const void *b)
/* Cycles are the same so tiebreak on port/queue id.
* Tiebreaking (as opposed to return 0) ensures consistent
* sort results across multiple OS's. */
-if (qa->port->port_no != qb->port->port_no) {
-return (qa->port->port_no > qb->port->port_no) ? 1 : -1;
+uint32_t port_qa = odp_to_u32(qa->port->port_no);
+uint32_t port_qb = odp_to_u32(qb->port->port_no);
+if (port_qa != port_qb) {
+return port_qa > port_qb ? 1 : -1;
} else {
return netdev_rxq_get_queue_id(qa->rx)
- netdev_rxq_get_queue_id(qb->rx);
--
2.10.2
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] OVS DPDK: dpdk_merge pull request
On Fri, Dec 08, 2017 at 10:14:49PM +, Stokes, Ian wrote: > The following changes since commit 65d9759c4fc433dbda89ad3d7225c1f5eac274ca: > > ovsdb-data: Add OVS_WARN_UNUSED_RESULT annotations to function definitions. > (2017-12-08 13:39:29 -0800) > > are available in the git repository at: > > https://github.com/istokes/ovs dpdk_merge > > for you to fetch changes up to 3eb8d4fa0db3159a8ffc8f52223417b3417263b3: Thanks a lot. I merged this to master. This merge added one new "sparse" warning. I sent a fix for review: https://patchwork.ozlabs.org/patch/847165/ ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v3 2/5] conntrack: add commands to r/w CT parameters.
One extra note inline
Thanks Darrell
On 12/11/17, 8:35 AM, "Darrell Ball" wrote:
Thanks Antonio for doing all this and pushing it forward.
Regarding patches 2-4:
I understand you want to save some code for various possible set and get
operations.
The prior art for these commands is however not generic set and get
commands.
Sometimes, we have specific commands that can take different numbers of
arguments but
those are specific and the context is clear.
If we take some examples, we might see there is an issue with the approach
here.
Take per zone limits as one example, which is an internal requirement that
we are working on
across datapaths.
This is a set operation with 2 arguments plus the datapath = 3 arguments.
This won’t work with the generic set functions here.
The corresponding get will not work either.
Trying to make this work generically will get pretty messy and the
parameter validation error prone.
I would like to propose an alternative (a simple one; also with code
consolidation and more error checking) below:
Please let me know what you think.
Thanks Darrell
///
diff --git a/lib/conntrack.c b/lib/conntrack.c
index f5a3aa9..c8ad548 100644
--- a/lib/conntrack.c
+++ b/lib/conntrack.c
@@ -2485,6 +2485,27 @@ conntrack_flush(struct conntrack *ct, const uint16_t
*zone)
return 0;
}
+int
+conntrack_set_maxconns(struct conntrack *ct, uint32_t maxconns)
+{
+atomic_init(&ct->n_conn_limit, maxconns);
+return 0;
+}
+
+int
+conntrack_get_maxconns(struct conntrack *ct, uint32_t *maxconns)
+{
+atomic_read_relaxed(&ct->n_conn_limit, maxconns);
+return 0;
+}
+
+int
+conntrack_get_nconns(struct conntrack *ct, uint32_t *nconns)
+{
+*nconns = atomic_count_get(&ct->n_conn);
+return 0;
+}
+
/* This function must be called with the ct->resources read lock taken. */
static struct alg_exp_node *
expectation_lookup(struct hmap *alg_expectations,
diff --git a/lib/conntrack.h b/lib/conntrack.h
index fbeef1c..8652724 100644
--- a/lib/conntrack.h
+++ b/lib/conntrack.h
@@ -114,6 +114,9 @@ int conntrack_dump_next(struct conntrack_dump *, struct
ct_dpif_entry *);
int conntrack_dump_done(struct conntrack_dump *);
int conntrack_flush(struct conntrack *, const uint16_t *zone);
+int conntrack_set_maxconns(struct conntrack *ct, uint32_t maxconns);
+int conntrack_get_maxconns(struct conntrack *ct, uint32_t *maxconns);
+int conntrack_get_nconns(struct conntrack *ct, uint32_t *nconns);
^L
/* 'struct ct_lock' is a wrapper for an adaptive mutex. It's useful to try
* different types of locks (e.g. spinlocks) */
diff --git a/lib/ct-dpif.c b/lib/ct-dpif.c
index 239c848..5fa3a97 100644
--- a/lib/ct-dpif.c
+++ b/lib/ct-dpif.c
@@ -140,6 +140,30 @@ ct_dpif_flush(struct dpif *dpif, const uint16_t *zone,
: EOPNOTSUPP);
}
+int
+ct_dpif_set_maxconns(struct dpif *dpif, uint32_t maxconns)
+{
+return (dpif->dpif_class->ct_set_maxconns
+? dpif->dpif_class->ct_set_maxconns(dpif, maxconns)
+: EOPNOTSUPP);
+}
+
+int
+ct_dpif_get_maxconns(struct dpif *dpif, uint32_t *maxconns)
+{
+return (dpif->dpif_class->ct_get_maxconns
+? dpif->dpif_class->ct_get_maxconns(dpif, maxconns)
+: EOPNOTSUPP);
+}
+
+int
+ct_dpif_get_nconns(struct dpif *dpif, uint32_t *nconns)
+{
+return (dpif->dpif_class->ct_get_nconns
+? dpif->dpif_class->ct_get_nconns(dpif, nconns)
+: EOPNOTSUPP);
+}
+
void
ct_dpif_entry_uninit(struct ct_dpif_entry *entry)
{
diff --git a/lib/ct-dpif.h b/lib/ct-dpif.h
index 5e2de53..09e7698 100644
--- a/lib/ct-dpif.h
+++ b/lib/ct-dpif.h
@@ -197,6 +197,9 @@ int ct_dpif_dump_next(struct ct_dpif_dump_state *,
struct ct_dpif_entry *);
int ct_dpif_dump_done(struct ct_dpif_dump_state *);
int ct_dpif_flush(struct dpif *, const uint16_t *zone,
const struct ct_dpif_tuple *);
+int ct_dpif_set_maxconns(struct dpif *dpif, uint32_t maxconns);
+int ct_dpif_get_maxconns(struct dpif *dpif, uint32_t *maxconns);
+int ct_dpif_get_nconns(struct dpif *dpif, uint32_t *nconns);
void ct_dpif_entry_uninit(struct ct_dpif_entry *);
void ct_dpif_format_entry(const struct ct_dpif_entry *, struct ds *,
bool verbose, bool print_stats);
diff --git a/lib/dpctl.c b/lib/dpctl.c
index a28ded9..215d1c3 100644
--- a/lib/dpctl.c
+++ b/lib/dpctl.c
@@ -1654,6 +1654,96 @@ dpctl_ct_bkts(int argc, const char *argv[],
return error;
}
[ovs-dev] [PATCH 1/2] valgrind: Add support to run kernel datapath testsuite under valgrind
With this patch, kernel datapath testsuite can be run under valgrind by using the "check-kernel-valgrind" target and the results can be found under directory "tests/system-kmod-testsuite.dir/". Signed-off-by: Yifeng Sun --- Documentation/topics/testing.rst | 4 tests/automake.mk| 7 +++ 2 files changed, 11 insertions(+) diff --git a/Documentation/topics/testing.rst b/Documentation/topics/testing.rst index 85aa6a1fb495..6645b794a07f 100644 --- a/Documentation/topics/testing.rst +++ b/Documentation/topics/testing.rst @@ -118,6 +118,10 @@ valgrind by using the ``check-valgrind`` target:: When you do this, the "valgrind" results for test are reported in files named ``tests/testsuite.dir//valgrind.*``. +To test the testsuite of kernel datapath under valgrind, you can use the +``check-kernel-valgrind`` target and find the "valgrind" results under +directory ``tests/system-kmod-testsuite.dir/``. + All the same options are available via TESTSUITEFLAGS. .. hint:: diff --git a/tests/automake.mk b/tests/automake.mk index 7eed1064e82b..0acafca100d3 100644 --- a/tests/automake.mk +++ b/tests/automake.mk @@ -214,6 +214,13 @@ check-valgrind: all $(valgrind_wrappers) $(check_DATA) @echo '--' @echo 'Valgrind output can be found in tests/testsuite.dir/*/valgrind.*' @echo '--' +check-kernel-valgrind: all $(valgrind_wrappers) $(check_DATA) + set $(SHELL) '$(SYSTEM_KMOD_TESTSUITE)' -C tests VALGRIND='$(VALGRIND)' AUTOTEST_PATH='tests/valgrind:$(AUTOTEST_PATH)' -d $(TESTSUITEFLAGS) -j1; \ + "$$@" || (test X'$(RECHECK)' = Xyes && "$$@" --recheck) + @echo + @echo '--' + @echo 'Valgrind output can be found in tests/system-kmod-testsuite.dir/*/valgrind.*' + @echo '--' check-helgrind: all $(valgrind_wrappers) $(check_DATA) -$(SHELL) '$(TESTSUITE)' -C tests CHECK_VALGRIND=true VALGRIND='$(HELGRIND)' AUTOTEST_PATH='tests/valgrind:$(AUTOTEST_PATH)' -d $(TESTSUITEFLAGS) -- 2.7.4 ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH 2/2] bond: Fix bug that writes to freed memory
pr_op->pr_rule is pointing to memory in bond->hash. It shouldn't be written
if bond->hash is already freed.
This bug is reported by running kernel path testsuite under valgrind:
Invalid write of size 8
at 0x413D16: update_recirc_rules__ (bond.c:392)
by 0x414CA0: bond_unref (bond.c:290)
by 0x427E3C: bundle_destroy (ofproto-dpif.c:3002)
by 0x429EF4: bundle_set (ofproto-dpif.c:3023)
by 0x40858B: port_destroy (bridge.c:4087)
by 0x40BD04: bridge_destroy (bridge.c:3266)
by 0x410528: bridge_exit (bridge.c:506)
by 0x4072EE: main (ovs-vswitchd.c:135)
Address 0xb5a85f0 is 5,360 bytes inside a block of size 12,288 free'd
at 0x4C2EDEB: free (/usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
by 0x414C8D: bond_unref (bond.c:288)
by 0x427E3C: bundle_destroy (ofproto-dpif.c:3002)
by 0x429EF4: bundle_set (ofproto-dpif.c:3023)
by 0x40858B: port_destroy (bridge.c:4087)
by 0x40BD04: bridge_destroy (bridge.c:3266)
by 0x410528: bridge_exit (bridge.c:506)
by 0x4072EE: main (ovs-vswitchd.c:135)
Block was alloc'd at
at 0x4C2DB8F: malloc (/usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
by 0x516C04: xmalloc (util.c:120)
by 0x414FD1: bond_entry_reset (bond.c:1651)
by 0x414FD1: bond_reconfigure (bond.c:470)
by 0x41507D: bond_create (bond.c:245)
by 0x429D5D: bundle_set (ofproto-dpif.c:3194)
by 0x408AC8: port_configure (bridge.c:1052)
by 0x40CD87: bridge_reconfigure (bridge.c:682)
by 0x410775: bridge_run (bridge.c:2998)
by 0x407244: main (ovs-vswitchd.c:119)
Signed-off-by: Yifeng Sun
---
ofproto/bond.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/ofproto/bond.c b/ofproto/bond.c
index 8ecd22c7d5d3..6f3d7b5b3817 100644
--- a/ofproto/bond.c
+++ b/ofproto/bond.c
@@ -389,7 +389,9 @@ update_recirc_rules__(struct bond *bond)
}
hmap_remove(&bond->pr_rule_ops, &pr_op->hmap_node);
-*pr_op->pr_rule = NULL;
+if (bond->hash) {
+*pr_op->pr_rule = NULL;
+}
free(pr_op);
break;
}
--
2.7.4
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v6 0/4] nsh: add new nsh key ttl and action dec_nsh_ttl
On Mon, Dec 11, 2017 at 10:25:54AM -0800, Gregory Rose wrote: > On 12/10/2017 4:39 PM, Yang, Yi wrote: > >On Sat, Dec 09, 2017 at 12:42:32AM +0800, Gregory Rose wrote: > >>On 12/8/2017 6:04 AM, Yi Yang wrote: > >>>v5->v6 > >>>- Rebase v5 to master > >>>- Refactor netlink message format to align to NSH kernel > >>> implementation > >>>- Add dec_nsh_ttl unit test into tests/nsh.at > >>>- Fix unit test unstable issue > >>> > >>I don't see any backport of the upstream kernel datapath changes? I'm > >>working on catching our out of tree datapath code with the upstream > >>Linux kernel datapath and your patch (commit > >>b2d0f5d5dc53532e6f07bc546a476a55ebdfe0f3 " openvswitch: enable NSH > >>support") needs the backport as well as compat layer changes. > >> > >>Do you plan on doing that work? > >> > >>Thanks, > >> > >>- Greg > >Greg, yes, I'll backport kernel datapath patches once this patch series > >is merged. BTW this doesn't depend on the kernel datapath patches. > > Understood that this series of patches you sent does not require the kernel > datapath backport > but I found that the kernel datapath patches do modify the openvswitch uapi > header and adds > new switch cases. These need to be handled eventually which can be done > when you do the > backport. Greg and Yi, can you help me understand the compatibility and patch workflow implications of accepting this series now instead of after the kernel datapath backport? In your opinions, is it important to apply them in a particular order? Thanks, Ben. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] OVS DPDK: dpdk_merge pull request
> On Fri, Dec 08, 2017 at 10:14:49PM +, Stokes, Ian wrote: > > The following changes since commit > 65d9759c4fc433dbda89ad3d7225c1f5eac274ca: > > > > ovsdb-data: Add OVS_WARN_UNUSED_RESULT annotations to function > definitions. (2017-12-08 13:39:29 -0800) > > > > are available in the git repository at: > > > > https://github.com/istokes/ovs dpdk_merge > > > > for you to fetch changes up to 3eb8d4fa0db3159a8ffc8f52223417b3417263b3: > > Thanks a lot. I merged this to master. Thanks for this Ben. > > This merge added one new "sparse" warning. I sent a fix for review: > https://patchwork.ozlabs.org/patch/847165/ I recompiled with sparse to see if I could reproduce the issue but I don't see that warning. I'm concerned that there could be a problem with sparse on my build system at this stage as this is the second time an issue picked up by your build but not on mine. I obviously need to look into this, out of interest what version of sparse do you use? Thanks Ian ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v6 0/4] nsh: add new nsh key ttl and action dec_nsh_ttl
On 12/11/2017 10:55 AM, Ben Pfaff wrote: On Mon, Dec 11, 2017 at 10:25:54AM -0800, Gregory Rose wrote: On 12/10/2017 4:39 PM, Yang, Yi wrote: On Sat, Dec 09, 2017 at 12:42:32AM +0800, Gregory Rose wrote: On 12/8/2017 6:04 AM, Yi Yang wrote: v5->v6 - Rebase v5 to master - Refactor netlink message format to align to NSH kernel implementation - Add dec_nsh_ttl unit test into tests/nsh.at - Fix unit test unstable issue I don't see any backport of the upstream kernel datapath changes? I'm working on catching our out of tree datapath code with the upstream Linux kernel datapath and your patch (commit b2d0f5d5dc53532e6f07bc546a476a55ebdfe0f3 " openvswitch: enable NSH support") needs the backport as well as compat layer changes. Do you plan on doing that work? Thanks, - Greg Greg, yes, I'll backport kernel datapath patches once this patch series is merged. BTW this doesn't depend on the kernel datapath patches. Understood that this series of patches you sent does not require the kernel datapath backport but I found that the kernel datapath patches do modify the openvswitch uapi header and adds new switch cases. These need to be handled eventually which can be done when you do the backport. Greg and Yi, can you help me understand the compatibility and patch workflow implications of accepting this series now instead of after the kernel datapath backport? In your opinions, is it important to apply them in a particular order? Ben, I started backporting the upstream NSH patch from Yi and found that I had to make some related user space changes to make it compile correctly. So it would appear that the upstream kernel patch would require this set of patches first. That said, let's get Yi's response since he is far more familiar with the NSH work than I am. Thanks, - Greg ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] daemon-unix: include missing help information
On Mon, Dec 11, 2017 at 03:11:25PM +, Markos Chandras wrote: > On 11/12/17 15:07, Aaron Conole wrote: > > These options have existed for a while, but were not expressed in the > > help information. Inform the user that these options exist, and give > > some basic help. > > > > Reported-by: Saravanan KR > > Signed-off-by: Aaron Conole > > --- > > lib/daemon-unix.c | 2 ++ > > 1 file changed, 2 insertions(+) > > > > diff --git a/lib/daemon-unix.c b/lib/daemon-unix.c > > index 967a28432..adb549c98 100644 > > --- a/lib/daemon-unix.c > > +++ b/lib/daemon-unix.c > > @@ -537,6 +537,8 @@ daemon_usage(void) > > printf( > > "\nDaemon options:\n" > > " --detachrun in background as daemon\n" > > +" --monitor creates a process to monitor this > > daemon\n" > > +" --user=username[:group] changes the effective daemon > > user:group\n" > > " --no-chdir do not chdir to '/'\n" > > " --pidfile[=FILE]create pidfile (default: %s/%s.pid)\n" > > " --overwrite-pidfile with --pidfile, start even if already " > > > > Good catch > > Reviewed-by: Markos Chandras Thanks, Aaron and Markos. I applied this to master. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] ofproto: Keep inserting buckets into a group from changing group type.
Thank you for testing. I applied this to master and backported it as
far as OVS 2.6.
On Mon, Dec 11, 2017 at 11:04:37AM +0530, shivani dommeti wrote:
> Thank you Ben for suggesting the fix.
> OVS is working as expected after adding the changes.
>
>
> Regards,
> Shivani.
>
> On Fri, Dec 8, 2017 at 3:33 AM, Ben Pfaff wrote:
>
> > The "insert buckets" and "delete buckets" operations on a group should not
> > change the group's type or properties, but the implementation did this by
> > mistake. This fixes the problem.
> >
> > Reported-by: shivani dommeti
> > Reported-at: https://mail.openvswitch.org/pipermail/ovs-discuss/2017-
> > December/045830.html
> > Signed-off-by: Ben Pfaff
> > ---
> > AUTHORS.rst | 1 +
> > ofproto/ofproto.c | 17 +
> > tests/ofproto.at | 25 +
> > 3 files changed, 43 insertions(+)
> >
> > diff --git a/AUTHORS.rst b/AUTHORS.rst
> > index 075b2f72d47d..979cfd7bce41 100644
> > --- a/AUTHORS.rst
> > +++ b/AUTHORS.rst
> > @@ -589,6 +589,7 @@ likunyunkunyu...@hotmail.com
> > meishengxin meisheng...@huawei.com
> > neeraj mehtamehtaneera...@gmail.com
> > rahim entezari rahim.entez...@gmail.com
> > +shivani dommeti shivani.domm...@gmail.com
> > weizj 34965...@qq.com
> > 俊 赵 zhaoju...@outlook.com
> > 冯全树(Crab)fqs...@126.com
> > diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
> > index 82c2bb27d348..84eb18e901ed 100644
> > --- a/ofproto/ofproto.c
> > +++ b/ofproto/ofproto.c
> > @@ -6905,6 +6905,13 @@ handle_queue_get_config_request(struct ofconn
> > *ofconn,
> > return error;
> > }
> >
> > +/* Allocates, initializes, and constructs a new group in 'ofproto',
> > obtaining
> > + * all the attributes for it from 'gm', and stores a pointer to it in
> > + * '*ofgroup'. Makes the new group visible from the flow table starting
> > from
> > + * 'version'.
> > + *
> > + * Returns 0 if successful, otherwise an error code. If there is an
> > error then
> > + * '*ofgroup' is indeterminate upon return. */
> > static enum ofperr
> > init_group(struct ofproto *ofproto, const struct ofputil_group_mod *gm,
> > ovs_version_t version, struct ofgroup **ofgroup)
> > @@ -7113,6 +7120,16 @@ modify_group_start(struct ofproto *ofproto, struct
> > ofproto_group_mod *ogm)
> > return OFPERR_OFPGMFC_UNKNOWN_GROUP;
> > }
> >
> > +/* Inserting or deleting a bucket should not change the group's type
> > or
> > + * properties, so change the group mod so that these aspects match
> > the old
> > + * group. (See EXT-570.) */
> > +if (ogm->gm.command == OFPGC15_INSERT_BUCKET ||
> > +ogm->gm.command == OFPGC15_REMOVE_BUCKET) {
> > +ogm->gm.type = old_group->type;
> > +ofputil_group_properties_destroy(&ogm->gm.props);
> > +ofputil_group_properties_copy(&ogm->gm.props, &old_group->props);
> > +}
> > +
> > if (old_group->type != ogm->gm.type
> > && (ofproto->n_groups[ogm->gm.type]
> > >= ofproto->ogf.max_groups[ogm->gm.type])) {
> > diff --git a/tests/ofproto.at b/tests/ofproto.at
> > index c19a3d10daed..74a30cbefe3b 100644
> > --- a/tests/ofproto.at
> > +++ b/tests/ofproto.at
> > @@ -621,7 +621,32 @@ OFPST_GROUP_DESC reply (OF1.5):
> > group_id=1234,type=all,bucket=bucket_id:0,actions=output:0,
> > bucket=bucket_id:1,actions=output:1,bucket=bucket_id:10,
> > actions=output:10,bucket=bucket_id:11,actions=output:
> > 11,bucket=bucket_id:12,actions=output:12,bucket=
> > bucket_id:13,actions=output:13,bucket=bucket_id:14,
> > actions=output:14,bucket=bucket_id:15,actions=output:
> > 15,bucket=bucket_id:20,actions=output:20,bucket=
> > bucket_id:21,actions=output:21
> > ])
> >
> > +# Delete groups.
> > +AT_CHECK([ovs-ofctl -O OpenFlow15 del-groups br0])
> > +
> > +# Add "fast_failover" group, then insert a bucket into it and make
> > +# sure that the type of the group doesn't change. (There was a bug
> > +# that caused this to happen.)
> > +AT_CHECK([ovs-ofctl -O OpenFlow15 -vwarn add-group br0
> > group_id=1234,type=ff])
> > +AT_CHECK([ovs-ofctl -O OpenFlow15 -vwarn dump-groups br0], [0], [stdout])
> > +AT_CHECK([strip_xids < stdout], [0], [dnl
> > +OFPST_GROUP_DESC reply (OF1.5):
> > + group_id=1234,type=ff
> > +])
> > +AT_CHECK([ovs-ofctl -O OpenFlow15 -vwarn insert-buckets br0
> > group_id=1234,command_bucket_id=first,bucket=bucket_id:20,
> > actions=output:20,bucket=bucket_id:21,actions=output:21])
> > +AT_CHECK([ovs-ofctl -O OpenFlow15 -vwarn dump-groups br0], [0], [stdout])
> > +AT_CHECK([strip_xids < stdout], [0], [dnl
> > +OFPST_GROUP_DESC reply (OF1.5):
> > + group_id=1234,type=ff,bucket=bucket_id:20,actions=output:
> > 20,bucket=bucket_id:21,actions=output:21
> > +])
> > +
> > # Negative tests.
> > +AT_CHECK([ovs-ofctl -O OpenFlow15 -vwarn insert-buckets br0
> > group_id=1234,c
Re: [ovs-dev] [PATCH] dpif-netdev: Avoid "sparse" warning.
> "sparse" warns when odp_port_t is used directly in an inequality
> comparison. This avoids the warning.
>
> CC: Kevin Traynor
> Fixes: a130f1a89bd8 ("dpif-netdev: Add port/queue tiebreaker to
> rxq_cycle_sort.")
> Signed-off-by: Ben Pfaff
> ---
> lib/dpif-netdev.c | 6 --
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c index
> 43f6a7857412..68fb3925bcf1 100644
> --- a/lib/dpif-netdev.c
> +++ b/lib/dpif-netdev.c
> @@ -3444,8 +3444,10 @@ compare_rxq_cycles(const void *a, const void *b)
> /* Cycles are the same so tiebreak on port/queue id.
> * Tiebreaking (as opposed to return 0) ensures consistent
> * sort results across multiple OS's. */
> -if (qa->port->port_no != qb->port->port_no) {
> -return (qa->port->port_no > qb->port->port_no) ? 1 : -1;
> +uint32_t port_qa = odp_to_u32(qa->port->port_no);
> +uint32_t port_qb = odp_to_u32(qb->port->port_no);
> +if (port_qa != port_qb) {
> +return port_qa > port_qb ? 1 : -1;
> } else {
> return netdev_rxq_get_queue_id(qa->rx)
> - netdev_rxq_get_queue_id(qb->rx);
Thanks Ben,
Passed functional validation on this side (couldn't test for the sparse warning
itself unfortunately) and LGTM.
Acked-by: Ian Stokes
> --
> 2.10.2
>
> ___
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH 3/5] ovsdb-idl: Fix assertion failure on error path parsing server reply.
> -Original Message- > From: ovs-dev-boun...@openvswitch.org [mailto:ovs-dev- > boun...@openvswitch.org] On Behalf Of Ben Pfaff > Sent: Friday, December 8, 2017 11:24 PM > To: d...@openvswitch.org > Cc: Ben Pfaff > Subject: [ovs-dev] [PATCH 3/5] ovsdb-idl: Fix assertion failure on error path > parsing server reply. > > If the database server sent an error reply to a monitor_cond request, and > the error was not a JSON string, then passing the error to json_string() > caused an assertion failure. > > Found by inspection. > > Signed-off-by: Ben Pfaff > --- Acked-by: Alin Gabriel Serdean ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH 2/5] ovsdb-idl: Fix indentation in a couple of places.
> -Original Message- > From: ovs-dev-boun...@openvswitch.org [mailto:ovs-dev- > boun...@openvswitch.org] On Behalf Of Ben Pfaff > Sent: Friday, December 8, 2017 11:24 PM > To: d...@openvswitch.org > Cc: Ben Pfaff > Subject: [ovs-dev] [PATCH 2/5] ovsdb-idl: Fix indentation in a couple of > places. > > White space changes only. > > Signed-off-by: Ben Pfaff > --- Acked-by: Alin Gabriel Serdean ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] dpif-netdev: Avoid "sparse" warning.
On 12/11/2017 06:34 PM, Ben Pfaff wrote:
> "sparse" warns when odp_port_t is used directly in an inequality
> comparison. This avoids the warning.
>
> CC: Kevin Traynor
> Fixes: a130f1a89bd8 ("dpif-netdev: Add port/queue tiebreaker to
> rxq_cycle_sort.")
> Signed-off-by: Ben Pfaff
> ---
Thanks for fixing this :/ I'm not seeing that warning, so I need to
figure that part out on my side.
Acked-by: Kevin Traynor
> lib/dpif-netdev.c | 6 --
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
> index 43f6a7857412..68fb3925bcf1 100644
> --- a/lib/dpif-netdev.c
> +++ b/lib/dpif-netdev.c
> @@ -3444,8 +3444,10 @@ compare_rxq_cycles(const void *a, const void *b)
> /* Cycles are the same so tiebreak on port/queue id.
> * Tiebreaking (as opposed to return 0) ensures consistent
> * sort results across multiple OS's. */
> -if (qa->port->port_no != qb->port->port_no) {
> -return (qa->port->port_no > qb->port->port_no) ? 1 : -1;
> +uint32_t port_qa = odp_to_u32(qa->port->port_no);
> +uint32_t port_qb = odp_to_u32(qb->port->port_no);
> +if (port_qa != port_qb) {
> +return port_qa > port_qb ? 1 : -1;
> } else {
> return netdev_rxq_get_queue_id(qa->rx)
> - netdev_rxq_get_queue_id(qb->rx);
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH 1/5] ovsdb-idl: Improve comments.
LGTM. Just two nits.
> -Original Message-
> From: ovs-dev-boun...@openvswitch.org [mailto:ovs-dev-
> boun...@openvswitch.org] On Behalf Of Ben Pfaff
> Sent: Friday, December 8, 2017 11:24 PM
> To: d...@openvswitch.org
> Cc: Ben Pfaff
> Subject: [ovs-dev] [PATCH 1/5] ovsdb-idl: Improve comments.
>
> This change documents the IDL state machine, adds other comments, and
> fixes a spelling error in a comment.
>
> Signed-off-by: Ben Pfaff
> ---
> lib/ovsdb-idl.c | 57
> ++---
> 1 file changed, 46 insertions(+), 11 deletions(-)
>
> diff --git a/lib/ovsdb-idl.c b/lib/ovsdb-idl.c index
> 26a19fe8d5e6..7e3abdee8e62 100644
> --- a/lib/ovsdb-idl.c
> +++ b/lib/ovsdb-idl.c
> @@ -79,29 +79,63 @@ struct ovsdb_idl_arc {
> struct ovsdb_idl_row *dst; /* Destination row. */ };
>
> +/* Connection state machine.
> + *
> + * When a JSON-RPC session connects, the IDL sends a "get_schema"
> +request and
> + * transitions to IDL_S_SCHEMA_REQUESTED. If the session drops and
> +reconnects,
> + * the IDL starts over again in the same way. */
> enum ovsdb_idl_state {
> +/* Waits for "get_schema" reply, then sends a "monitor_cond" request
> whose
> + * details are informed by the schema and transitions to
> + * IDL_S_MONITOR_COND_REQUESTED. */
> IDL_S_SCHEMA_REQUESTED,
> -IDL_S_MONITOR_REQUESTED,
> -IDL_S_MONITORING,
> +
> +/* Waits for "monitor_cond" reply:
> + *
> + *- If the reply indicates success, replaces the IDL contents by the
> + * data carried in the reply and transitions to
> IDL_S_MONITORING_COND.
> + *
> + *- If the reply indicates failure because the database is too old to
> + * support monitor_cond, sends a "monitor" request and transitions
> to
> + * IDl_S_MONITOR_REQUESTED. */
> IDL_S_MONITOR_COND_REQUESTED,
> +
> +/* Waits for "monitor" reply, then replaces the IDL contents by the data
> + * carried in the reply and transitions to IDL_S_MONITORING. */
> +IDL_S_MONITOR_REQUESTED,
> +
> +/* Terminal states that process "update2" (IDL_S_MONITORING_COND)
> or
> + * "update" (IDL_S_MONITORING) notifications. */
> IDL_S_MONITORING_COND,
> +IDL_S_MONITORING,
> +
> +/* Terminal error state that indicates that nothing useful can be done.
> + * The most likely reason is that the database server doesn't actually
> have
[Alin Serdean] doesn't have, maybe?
> + * the desired database. We maintain the session with the database
> server
> + * anyway. If it starts serving the database that we want, then it will
> + * kill the session and we will automatically reconnect and try
> + again. */
> IDL_S_NO_SCHEMA
> };
>
> struct ovsdb_idl {
> +/* Data. */
> const struct ovsdb_idl_class *class_;
> -struct jsonrpc_session *session;
> -struct uuid uuid;
> struct shash table_by_name; /* Contains "struct ovsdb_idl_table *"s.*/
> struct ovsdb_idl_table *tables; /* Array of ->class_->n_tables elements.
> */
> unsigned int change_seqno;
> -bool verify_write_only;
>
> -/* Session state. */
> -unsigned int state_seqno;
> -enum ovsdb_idl_state state;
> -struct json *request_id;
> -struct json *schema;
> +/* Session state.
> + *
> + *'state_seqno' is a snapshot of the session's sequence number as
> returned
> + * jsonrpc_session_get_seqno(session), so if it differs from the value
> that
> + * function currently returns then the session has reconnected and the
> + * state machine must restart. */
> +struct jsonrpc_session *session; /* Connection to the server. */
> +enum ovsdb_idl_state state; /* Current session state. */
> +unsigned int state_seqno;/* See above. */
> +struct json *request_id; /* JSON ID for request awaiting reply.
> */
> +struct json *schema; /* Temporary copy of database schema. */
> +struct uuid uuid;/* Identifier for monitor. */
>
> /* Database locking. */
> char *lock_name;/* Name of lock we need, NULL if none. */
> @@ -112,6 +146,7 @@ struct ovsdb_idl {
> /* Transaction support. */
> struct ovsdb_idl_txn *txn;
> struct hmap outstanding_txns;
> +bool verify_write_only;
>
> /* Conditional monitoring. */
> bool cond_changed;
> @@ -1118,7 +1153,7 @@ ovsdb_idl_condition_clone(struct
> ovsdb_idl_condition *dst,
> * arranges to send the new condition to the database server.
> *
> * Return the next conditional update sequence number. When this
> - * value and ovsdb_idl_get_condition_seqno() matchs, the 'idl'
> + * value and ovsdb_idl_get_condition_seqno() matches, the 'idl'
> * contains rows that match the 'condition'.
> */
> unsigned int
> --
s/. *\//. *\//
Acked-by: Alin Gabriel Serdean
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitc
[ovs-dev] Auditoría para la adquisición de empresas
Porque antes de arriesgar, hay que investigar Auditoría para la adquisición de empresas: Due Diligence 18 de Diciembre - CP. y MAN. Fernando García Zárate9am-6pm Due diligence es una herramienta que le ayudará a prevenir cualquier situación que agravie sus intereses con la realización de una evaluación exhaustiva de las áreas fiscal, financiera, legal y laboral. Reduzca el riesgo de la transacción identificando posibles contingencias, pero también valore las potenciales mejoras que pueden maximizar el éxito de su inversión. BENEFICIOS DE ASISTIR: - Aprenderá el concepto, objetivo y clases de due diligence. - Conocerá las fases del procedimiento. - Identificará los riesgos para el adquiriente - Sabrá cuál debe ser la estructura del informe y la redacción de contratos ¿Requiere la información a la Brevedad? responda este email con la palabra: Due + nombre - teléfono - correo. centro telefónico:018002120744 ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] dpif-netdev: Avoid "sparse" warning.
On Mon, Dec 11, 2017 at 08:00:07PM +, Stokes, Ian wrote:
> > "sparse" warns when odp_port_t is used directly in an inequality
> > comparison. This avoids the warning.
> >
> > CC: Kevin Traynor
> > Fixes: a130f1a89bd8 ("dpif-netdev: Add port/queue tiebreaker to
> > rxq_cycle_sort.")
> > Signed-off-by: Ben Pfaff
> > ---
> > lib/dpif-netdev.c | 6 --
> > 1 file changed, 4 insertions(+), 2 deletions(-)
> >
> > diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c index
> > 43f6a7857412..68fb3925bcf1 100644
> > --- a/lib/dpif-netdev.c
> > +++ b/lib/dpif-netdev.c
> > @@ -3444,8 +3444,10 @@ compare_rxq_cycles(const void *a, const void *b)
> > /* Cycles are the same so tiebreak on port/queue id.
> > * Tiebreaking (as opposed to return 0) ensures consistent
> > * sort results across multiple OS's. */
> > -if (qa->port->port_no != qb->port->port_no) {
> > -return (qa->port->port_no > qb->port->port_no) ? 1 : -1;
> > +uint32_t port_qa = odp_to_u32(qa->port->port_no);
> > +uint32_t port_qb = odp_to_u32(qb->port->port_no);
> > +if (port_qa != port_qb) {
> > +return port_qa > port_qb ? 1 : -1;
> > } else {
> > return netdev_rxq_get_queue_id(qa->rx)
> > - netdev_rxq_get_queue_id(qb->rx);
>
> Thanks Ben,
>
> Passed functional validation on this side (couldn't test for the sparse
> warning itself unfortunately) and LGTM.
>
> Acked-by: Ian Stokes
Thanks Ian, I applied this to master.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] dpif-netdev: Avoid "sparse" warning.
On Mon, Dec 11, 2017 at 08:18:35PM +, Kevin Traynor wrote:
> On 12/11/2017 06:34 PM, Ben Pfaff wrote:
> > "sparse" warns when odp_port_t is used directly in an inequality
> > comparison. This avoids the warning.
> >
> > CC: Kevin Traynor
> > Fixes: a130f1a89bd8 ("dpif-netdev: Add port/queue tiebreaker to
> > rxq_cycle_sort.")
> > Signed-off-by: Ben Pfaff
> > ---
>
> Thanks for fixing this :/ I'm not seeing that warning, so I need to
> figure that part out on my side.
>
> Acked-by: Kevin Traynor
Thanks, applied to master.
(Usually, if "sparse" warnings don't appear, it means that sparse isn't
installed or you aren't building with C=1.)
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH 00/12] Backport upstream Linux OVS patches
The following patches are available in the current Linux upstream git repository: 183dea5 openvswitch: do not propagate headroom updates to internal port 311af51 openvswitch: use ktime_get_ts64() instead of ktime_get_ts() 67c8d22 openvswitch: fix the incorrect flow action alloc size 2734166 net: openvswitch: datapath: fix data type in queue_gso_packets 0c19f846 net: accept UFO datagrams from tuntap and packet b74912a openvswitch: meter: fix NULL pointer dereference in ovs_meter_cmd_reply_start 6dc14dc openvswitch: Using kfree_rcu() to simplify the code 06c2351 openvswitch: Make local function ovs_nsh_key_attr_size() static 8a860c2 openvswitch: Fix return value check in ovs_meter_cmd_features() cd8a6c3 openvswitch: Add meter action support 96fbc13 openvswitch: Add meter infrastructure 9602c01 openvswitch: export get_dp() API. b2d0f5d openvswitch: enable NSH support 9354d45 openvswitch: reliable interface indentification in port dumps 2a17178 Merge git://git.kernel.org/pub/scm/linux/kernel/git/davem/net b244131 License cleanup: add SPDX GPL-2.0 license identifier to files with no license 279badc openvswitch: conntrack: mark expected switch fall-through b822696 openvswitch: add ct_clear action ceaa001 openvswitch: Add erspan tunnel support. 42ab19e net: Add extack to upper device linking 5829e62 openvswitch: Fix an error handling path in 'ovs_nla_init_match_and_action()' This patch series backports all of those patches except these four: 279badc openvswitch: conntrack: mark expected switch fall-through b822696 openvswitch: add ct_clear action ceaa001 openvswitch: Add erspan tunnel support. b2d0f5d openvswitch: enable NSH support Upstream patch 279badc isn't necessary since a patch for it was recently independently added. Upstream patches b2d0f5d, b822696 and ceaa001 require user space changes to allow OVS to build. I will work with the authors of those patches to get backports and required user space changes posted separately. Andy Zhou has sent me additional patches for the user space side of the meter patches. In this case the kernel datapath meter patches do not require the user space code to compile correctly so we can separate the application of the kernel datapath patches and the user space patches. I will update and post Andy's user space side meter patches in the near future. The remaining patches are addressed in this patch series as indicated below. Andy Zhou (3): datapath: export get_dp() API datapath: Add meter netlink definitions datapath: Add meter infrastructure Arnd Bergmann (1): datapath: use ktime_get_ts64() instead of ktime_get_ts() Christophe JAILLET (1): datapath: Fix an error handling path in 'ovs_nla_init_match_and_action() Gustavo A. R. Silva (2): datapath: meter: fix NULL pointer dereference in ovs_meter_cmd_reply_start datapath: fix data type in queue_gso_packets Jiri Benc (1): datapath: reliable interface indentification in port dumps Paolo Abeni (1): datapath: do not propagate headroom updates to internal port Wei Yongjun (2): datapath: Fix return value check in ovs_meter_cmd_features() datapath: Using kfree_rcu() to simplify the code zhangliping (1): datapath: fix the incorrect flow action alloc size acinclude.m4 | 4 +- datapath/Modules.mk | 6 +- datapath/datapath.c | 97 ++-- datapath/datapath.h | 38 +- datapath/dp_notify.c | 3 +- datapath/flow.c | 6 +- datapath/flow_netlink.c | 16 +- datapath/linux/compat/include/linux/netdevice.h | 19 - datapath/linux/compat/include/linux/openvswitch.h | 53 ++ datapath/linux/compat/include/net/netlink.h | 9 + datapath/meter.c | 597 ++ datapath/meter.h | 54 ++ datapath/vport-internal_dev.c | 19 +- 13 files changed, 821 insertions(+), 100 deletions(-) create mode 100644 datapath/meter.c create mode 100644 datapath/meter.h -- 1.8.3.1 ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH 01/12] datapath: Fix an error handling path in 'ovs_nla_init_match_and_action()
From: Christophe JAILLET
Upstream commit:
commit 5829e62ac17a40ab08c1b905565604a4b5fa7af6
Author: Christophe JAILLET
Date: Mon Sep 11 21:56:20 2017 +0200
openvswitch: Fix an error handling path in 'ovs_nla_init_match_and_action()'
All other error handling paths in this function go through the 'error'
label. This one should do the same.
Fixes: 9cc9a5cb176c ("datapath: Avoid using stack larger than 1024.")
Signed-off-by: Christophe JAILLET
Acked-by: Pravin B Shelar
Signed-off-by: David S. Miller
Cc: Christophe JAILLET
Fixes: 850c2a4d1a ("datapath: Avoid using stack larger than 1024.")
Signed-off-by: Greg Rose
---
datapath/datapath.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/datapath/datapath.c b/datapath/datapath.c
index 1780819..eeab72a 100644
--- a/datapath/datapath.c
+++ b/datapath/datapath.c
@@ -1136,7 +1136,8 @@ static int ovs_nla_init_match_and_action(struct net *net,
if (!a[OVS_FLOW_ATTR_KEY]) {
OVS_NLERR(log,
"Flow key attribute not present in set
flow.");
- return -EINVAL;
+ error = -EINVAL;
+ goto error;
}
*acts = get_flow_actions(net, a[OVS_FLOW_ATTR_ACTIONS], key,
--
1.8.3.1
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH 03/12] datapath: export get_dp() API
From: Andy Zhou
Upstream commit:
commit 9602c01e57f7b868d748c2ba2aef0efa64b71ffc
Author: Andy Zhou
Date: Fri Nov 10 12:09:41 2017 -0800
openvswitch: export get_dp() API.
Later patches will invoke get_dp() outside of datapath.c. Export it.
Signed-off-by: Andy Zhou
Signed-off-by: David S. Miller
Cc: Andy Zhou
Signed-off-by: Greg Rose
---
datapath/datapath.c | 29 -
datapath/datapath.h | 31 +++
2 files changed, 31 insertions(+), 29 deletions(-)
diff --git a/datapath/datapath.c b/datapath/datapath.c
index 6a795b1..884df02 100644
--- a/datapath/datapath.c
+++ b/datapath/datapath.c
@@ -145,35 +145,6 @@ static int queue_userspace_packet(struct datapath *dp,
struct sk_buff *,
const struct dp_upcall_info *,
uint32_t cutlen);
-/* Must be called with rcu_read_lock. */
-static struct datapath *get_dp_rcu(struct net *net, int dp_ifindex)
-{
- struct net_device *dev = dev_get_by_index_rcu(net, dp_ifindex);
-
- if (dev) {
- struct vport *vport = ovs_internal_dev_get_vport(dev);
- if (vport)
- return vport->dp;
- }
-
- return NULL;
-}
-
-/* The caller must hold either ovs_mutex or rcu_read_lock to keep the
- * returned dp pointer valid.
- */
-static inline struct datapath *get_dp(struct net *net, int dp_ifindex)
-{
- struct datapath *dp;
-
- WARN_ON_ONCE(!rcu_read_lock_held() && !lockdep_ovsl_is_held());
- rcu_read_lock();
- dp = get_dp_rcu(net, dp_ifindex);
- rcu_read_unlock();
-
- return dp;
-}
-
/* Must be called with rcu_read_lock or ovs_mutex. */
const char *ovs_dp_name(const struct datapath *dp)
{
diff --git a/datapath/datapath.h b/datapath/datapath.h
index 1c73fb4..7481d6d 100644
--- a/datapath/datapath.h
+++ b/datapath/datapath.h
@@ -31,6 +31,7 @@
#include "compat.h"
#include "flow.h"
#include "flow_table.h"
+#include "vport-internal_dev.h"
#define DP_MAX_PORTS USHRT_MAX
#define DP_VPORT_HASH_BUCKETS 1024
@@ -197,6 +198,36 @@ static inline struct vport *ovs_vport_ovsl(const struct
datapath *dp, int port_n
return ovs_lookup_vport(dp, port_no);
}
+/* Must be called with rcu_read_lock. */
+static inline struct datapath *get_dp_rcu(struct net *net, int dp_ifindex)
+{
+ struct net_device *dev = dev_get_by_index_rcu(net, dp_ifindex);
+
+ if (dev) {
+ struct vport *vport = ovs_internal_dev_get_vport(dev);
+
+ if (vport)
+ return vport->dp;
+ }
+
+ return NULL;
+}
+
+/* The caller must hold either ovs_mutex or rcu_read_lock to keep the
+ * returned dp pointer valid.
+ */
+static inline struct datapath *get_dp(struct net *net, int dp_ifindex)
+{
+ struct datapath *dp;
+
+ WARN_ON_ONCE(!rcu_read_lock_held() && !lockdep_ovsl_is_held());
+ rcu_read_lock();
+ dp = get_dp_rcu(net, dp_ifindex);
+ rcu_read_unlock();
+
+ return dp;
+}
+
extern struct notifier_block ovs_dp_device_notifier;
extern struct genl_family dp_vport_genl_family;
extern struct genl_multicast_group ovs_dp_vport_multicast_group;
--
1.8.3.1
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH 02/12] datapath: reliable interface indentification in port dumps
From: Jiri Benc
Upstream commit:
commit 9354d452034273a50a4fd703bea31e5d6b1fc20b
Author: Jiri Benc
Date: Thu Nov 2 17:04:37 2017 -0200
openvswitch: reliable interface indentification in port dumps
This patch allows reliable identification of netdevice interfaces connected
to openvswitch bridges. In particular, user space queries the netdev
interfaces belonging to the ports for statistics, up/down state, etc.
Datapath dump needs to provide enough information for the user space to be
able to do that.
Currently, only interface names are returned. This is not sufficient, as
openvswitch allows its ports to be in different name spaces and the
interface name is valid only in its name space. What is needed and generally
used in other netlink APIs, is the pair ifindex+netnsid.
The solution is addition of the ifindex+netnsid pair (or only ifindex if in
the same name space) to vport get/dump operation.
On request side, ideally the ifindex+netnsid pair could be used to
get/set/del the corresponding vport. This is not implemented by this patch
and can be added later if needed.
Signed-off-by: Jiri Benc
Signed-off-by: David S. Miller
Cc: Jiri Benc
Signed-off-by: Greg Rose
---
acinclude.m4 | 3 ++
datapath/datapath.c | 49 +--
datapath/datapath.h | 4 +-
datapath/dp_notify.c | 3 +-
datapath/linux/compat/include/linux/openvswitch.h | 2 +
5 files changed, 45 insertions(+), 16 deletions(-)
diff --git a/acinclude.m4 b/acinclude.m4
index 1179a40..30a2103 100644
--- a/acinclude.m4
+++ b/acinclude.m4
@@ -785,6 +785,9 @@ AC_DEFUN([OVS_CHECK_LINUX_COMPAT], [
OVS_FIND_FIELD_IFELSE([$KSRC/include/linux/netfilter.h], [nf_hook_ops],
[list],
[OVS_DEFINE([HAVE_LIST_IN_NF_HOOK_OPS])])
+ OVS_GREP_IFELSE([$KSRC/include/net/net_namespace.h],
+ [EXPORT_SYMBOL_GPL(peernet2id_alloc)],
+ [OVS_DEFINE([HAVE_PEERNET2ID_ALLOC])])
if cmp -s datapath/linux/kcompat.h.new \
datapath/linux/kcompat.h >/dev/null 2>&1; then
diff --git a/datapath/datapath.c b/datapath/datapath.c
index eeab72a..6a795b1 100644
--- a/datapath/datapath.c
+++ b/datapath/datapath.c
@@ -1873,7 +1873,8 @@ static struct genl_family dp_datapath_genl_family
__ro_after_init = {
/* Called with ovs_mutex or RCU read lock. */
static int ovs_vport_cmd_fill_info(struct vport *vport, struct sk_buff *skb,
- u32 portid, u32 seq, u32 flags, u8 cmd)
+ struct net *net, u32 portid, u32 seq,
+ u32 flags, u8 cmd)
{
struct ovs_header *ovs_header;
struct ovs_vport_stats vport_stats;
@@ -1889,9 +1890,19 @@ static int ovs_vport_cmd_fill_info(struct vport *vport,
struct sk_buff *skb,
if (nla_put_u32(skb, OVS_VPORT_ATTR_PORT_NO, vport->port_no) ||
nla_put_u32(skb, OVS_VPORT_ATTR_TYPE, vport->ops->type) ||
nla_put_string(skb, OVS_VPORT_ATTR_NAME,
- ovs_vport_name(vport)))
+ ovs_vport_name(vport)) ||
+ nla_put_u32(skb, OVS_VPORT_ATTR_IFINDEX, vport->dev->ifindex))
goto nla_put_failure;
+#ifdef HAVE_PEERNET2ID_ALLOC
+ if (!net_eq(net, dev_net(vport->dev))) {
+ int id = peernet2id_alloc(net, dev_net(vport->dev));
+
+ if (nla_put_s32(skb, OVS_VPORT_ATTR_NETNSID, id))
+ goto nla_put_failure;
+ }
+
+#endif
ovs_vport_get_stats(vport, &vport_stats);
if (nla_put_64bit(skb, OVS_VPORT_ATTR_STATS,
sizeof(struct ovs_vport_stats), &vport_stats,
@@ -1921,8 +1932,8 @@ static struct sk_buff *ovs_vport_cmd_alloc_info(void)
}
/* Called with ovs_mutex, only via ovs_dp_notify_wq(). */
-struct sk_buff *ovs_vport_cmd_build_info(struct vport *vport, u32 portid,
-u32 seq, u8 cmd)
+struct sk_buff *ovs_vport_cmd_build_info(struct vport *vport, struct net *net,
+u32 portid, u32 seq, u8 cmd)
{
struct sk_buff *skb;
int retval;
@@ -1931,7 +1942,7 @@ struct sk_buff *ovs_vport_cmd_build_info(struct vport
*vport, u32 portid,
if (!skb)
return ERR_PTR(-ENOMEM);
- retval = ovs_vport_cmd_fill_info(vport, skb, portid, seq, 0, cmd);
+ retval = ovs_vport_cmd_fill_info(vport, skb, net, portid, seq, 0, cmd);
BUG_ON(retval < 0);
return skb;
@@ -1945,6 +1956,8 @@ static struct vport *lookup_vport(struct net *net,
struct datapath *dp;
struct vport *vport;
+ if (a[OVS_VPORT_ATTR_IFINDEX])
+ return ERR_PTR(-EOPNOTSUPP);
if (a[OVS_VPORT_ATTR_NAME]) {
vport = o
[ovs-dev] [PATCH 04/12] datapath: Add meter netlink definitions
From: Andy Zhou
Upstream commit:
commit 5794040647de4011598a6d005fdad95d24fd385b
Author: Andy Zhou
Date: Fri Nov 10 12:09:40 2017 -0800
openvswitch: Add meter netlink definitions
Meter has its own netlink family. Define netlink messages and attributes
for communicating with the user space programs.
Signed-off-by: Andy Zhou
Signed-off-by: David S. Miller
Cc: Andy Zhou
Signed-off-by: Greg Rose
---
datapath/linux/compat/include/linux/openvswitch.h | 51 +++
1 file changed, 51 insertions(+)
diff --git a/datapath/linux/compat/include/linux/openvswitch.h
b/datapath/linux/compat/include/linux/openvswitch.h
index f28d140..58d888f 100644
--- a/datapath/linux/compat/include/linux/openvswitch.h
+++ b/datapath/linux/compat/include/linux/openvswitch.h
@@ -947,4 +947,55 @@ enum ovs_action_attr {
#define OVS_ACTION_ATTR_MAX (__OVS_ACTION_ATTR_MAX - 1)
+/* Meters. */
+#define OVS_METER_FAMILY "ovs_meter"
+#define OVS_METER_MCGROUP "ovs_meter"
+#define OVS_METER_VERSION 0x1
+
+enum ovs_meter_cmd {
+ OVS_METER_CMD_UNSPEC,
+ OVS_METER_CMD_FEATURES, /* Get features supported by the datapath. */
+ OVS_METER_CMD_SET, /* Add or modify a meter. */
+ OVS_METER_CMD_DEL, /* Delete a meter. */
+ OVS_METER_CMD_GET /* Get meter stats. */
+};
+
+enum ovs_meter_attr {
+ OVS_METER_ATTR_UNSPEC,
+ OVS_METER_ATTR_ID, /* u32 meter ID within datapath. */
+ OVS_METER_ATTR_KBPS,/* No argument. If set, units in kilobits
+* per second. Otherwise, units in
+* packets per second.
+*/
+ OVS_METER_ATTR_STATS, /* struct ovs_flow_stats for the meter. */
+ OVS_METER_ATTR_BANDS, /* Nested attributes for meter bands. */
+ OVS_METER_ATTR_USED,/* u64 msecs last used in monotonic time. */
+ OVS_METER_ATTR_CLEAR, /* Flag to clear stats, used. */
+ OVS_METER_ATTR_MAX_METERS, /* u32 number of meters supported. */
+ OVS_METER_ATTR_MAX_BANDS, /* u32 max number of bands per meter. */
+ OVS_METER_ATTR_PAD,
+ __OVS_METER_ATTR_MAX
+};
+
+#define OVS_METER_ATTR_MAX (__OVS_METER_ATTR_MAX - 1)
+
+enum ovs_band_attr {
+ OVS_BAND_ATTR_UNSPEC,
+ OVS_BAND_ATTR_TYPE, /* u32 OVS_METER_BAND_TYPE_* constant. */
+ OVS_BAND_ATTR_RATE, /* u32 band rate in meter units (see above). */
+ OVS_BAND_ATTR_BURST,/* u32 burst size in meter units. */
+ OVS_BAND_ATTR_STATS,/* struct ovs_flow_stats for the band. */
+ __OVS_BAND_ATTR_MAX
+};
+
+#define OVS_BAND_ATTR_MAX (__OVS_BAND_ATTR_MAX - 1)
+
+enum ovs_meter_band_type {
+ OVS_METER_BAND_TYPE_UNSPEC,
+ OVS_METER_BAND_TYPE_DROP, /* Drop exceeding packets. */
+ __OVS_METER_BAND_TYPE_MAX
+};
+
+#define OVS_METER_BAND_TYPE_MAX (__OVS_METER_BAND_TYPE_MAX - 1)
+
#endif /* _LINUX_OPENVSWITCH_H */
--
1.8.3.1
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH 05/12] datapath: Add meter infrastructure
From: Andy Zhou
Upstream commit:
commit 96fbc13d7e770b542d2d1fcf700d0baadc6e8063
Author: Andy Zhou
Date: Fri Nov 10 12:09:42 2017 -0800
openvswitch: Add meter infrastructure
OVS kernel datapath so far does not support Openflow meter action.
This is the first stab at adding kernel datapath meter support.
This implementation supports only drop band type.
Signed-off-by: Andy Zhou
Signed-off-by: David S. Miller
Added a compat layer fixup for nla_parse.
Cc: Andy Zhou
Signed-off-by: Greg Rose
---
datapath/Modules.mk | 6 +-
datapath/datapath.c | 14 +-
datapath/datapath.h | 3 +
datapath/linux/compat/include/net/netlink.h | 9 +
datapath/meter.c| 604
datapath/meter.h| 54 +++
6 files changed, 686 insertions(+), 4 deletions(-)
create mode 100644 datapath/meter.c
create mode 100644 datapath/meter.h
diff --git a/datapath/Modules.mk b/datapath/Modules.mk
index 21f04a0..a9e2880 100644
--- a/datapath/Modules.mk
+++ b/datapath/Modules.mk
@@ -26,7 +26,8 @@ openvswitch_sources = \
flow_table.c \
vport.c \
vport-internal_dev.c \
- vport-netdev.c
+ vport-netdev.c \
+ meter.c
vport_geneve_sources = vport-geneve.c
vport_vxlan_sources = vport-vxlan.c
@@ -43,7 +44,8 @@ openvswitch_headers = \
flow_table.h \
vport.h \
vport-internal_dev.h \
- vport-netdev.h
+ vport-netdev.h \
+ meter.h
dist_sources = $(foreach module,$(dist_modules),$($(module)_sources))
dist_headers = $(foreach module,$(dist_modules),$($(module)_headers))
diff --git a/datapath/datapath.c b/datapath/datapath.c
index 884df02..eff9f85 100644
--- a/datapath/datapath.c
+++ b/datapath/datapath.c
@@ -57,6 +57,7 @@
#include "flow.h"
#include "flow_table.h"
#include "flow_netlink.h"
+#include "meter.h"
#include "gso.h"
#include "vport-internal_dev.h"
#include "vport-netdev.h"
@@ -177,6 +178,7 @@ static void destroy_dp_rcu(struct rcu_head *rcu)
ovs_flow_tbl_destroy(&dp->table);
free_percpu(dp->stats_percpu);
kfree(dp->ports);
+ ovs_meters_exit(dp);
kfree(dp);
}
@@ -1598,6 +1600,10 @@ static int ovs_dp_cmd_new(struct sk_buff *skb, struct
genl_info *info)
for (i = 0; i < DP_VPORT_HASH_BUCKETS; i++)
INIT_HLIST_HEAD(&dp->ports[i]);
+ err = ovs_meters_init(dp);
+ if (err)
+ goto err_destroy_ports_array;
+
/* Set up our datapath device. */
parms.name = nla_data(a[OVS_DP_ATTR_NAME]);
parms.type = OVS_VPORT_TYPE_INTERNAL;
@@ -1626,7 +1632,7 @@ static int ovs_dp_cmd_new(struct sk_buff *skb, struct
genl_info *info)
ovs_dp_reset_user_features(skb, info);
}
- goto err_destroy_ports_array;
+ goto err_destroy_meters;
}
err = ovs_dp_cmd_fill_info(dp, reply, info->snd_portid,
@@ -1641,8 +1647,10 @@ static int ovs_dp_cmd_new(struct sk_buff *skb, struct
genl_info *info)
ovs_notify(&dp_datapath_genl_family, &ovs_dp_datapath_multicast_group,
reply, info);
return 0;
-err_destroy_ports_array:
+err_destroy_meters:
ovs_unlock();
+ ovs_meters_exit(dp);
+err_destroy_ports_array:
kfree(dp->ports);
err_destroy_percpu:
free_percpu(dp->stats_percpu);
@@ -2292,6 +2300,7 @@ static struct genl_family *dp_genl_families[] = {
&dp_vport_genl_family,
&dp_flow_genl_family,
&dp_packet_genl_family,
+ &dp_meter_genl_family,
};
static void dp_unregister_genl(int n_families)
@@ -2485,3 +2494,4 @@ MODULE_ALIAS_GENL_FAMILY(OVS_DATAPATH_FAMILY);
MODULE_ALIAS_GENL_FAMILY(OVS_VPORT_FAMILY);
MODULE_ALIAS_GENL_FAMILY(OVS_FLOW_FAMILY);
MODULE_ALIAS_GENL_FAMILY(OVS_PACKET_FAMILY);
+MODULE_ALIAS_GENL_FAMILY(OVS_METER_FAMILY);
diff --git a/datapath/datapath.h b/datapath/datapath.h
index 7481d6d..93c9ed5 100644
--- a/datapath/datapath.h
+++ b/datapath/datapath.h
@@ -93,6 +93,9 @@ struct datapath {
u32 user_features;
u32 max_headroom;
+
+ /* Switch meters. */
+ struct hlist_head *meters;
};
/**
diff --git a/datapath/linux/compat/include/net/netlink.h
b/datapath/linux/compat/include/net/netlink.h
index 4325b9b..ba24a34 100644
--- a/datapath/linux/compat/include/net/netlink.h
+++ b/datapath/linux/compat/include/net/netlink.h
@@ -169,6 +169,15 @@ static inline int rpl_nla_parse_nested(struct nlattr
*tb[], int maxtype,
return nla_parse_nested(tb, maxtype, nla, policy);
}
#define nla_parse_nested rpl_nla_parse_nested
+
+static inline int rpl_nla_parse(struct nlattr **tb, int maxtype,
+ const struct nlattr *head, int len,
+ const struct nla_policy *policy,
+ struct netlink_ext_ack *extack
[ovs-dev] [PATCH 06/12] datapath: Fix return value check in ovs_meter_cmd_features()
From: Wei Yongjun
Upstream commit:
commit 8a860c2bcc84a8e4fbcabb928cd97e4c51b17d93
Author: Wei Yongjun
Date: Tue Nov 14 06:20:16 2017 +
openvswitch: Fix return value check in ovs_meter_cmd_features()
In case of error, the function ovs_meter_cmd_reply_start() returns
ERR_PTR() not NULL. The NULL test in the return value check should
be replaced with IS_ERR().
Fixes: 96fbc13d7e77 ("openvswitch: Add meter infrastructure")
Signed-off-by: Wei Yongjun
Acked-by: Pravin B Shelar
Signed-off-by: David S. Miller
Cc: Wei Yongjun
Signed-off-by: Greg Rose
---
datapath/meter.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/datapath/meter.c b/datapath/meter.c
index 2a5ba35..2e58b6c 100644
--- a/datapath/meter.c
+++ b/datapath/meter.c
@@ -166,7 +166,7 @@ static int ovs_meter_cmd_features(struct sk_buff *skb,
struct genl_info *info)
reply = ovs_meter_cmd_reply_start(info, OVS_METER_CMD_FEATURES,
&ovs_reply_header);
- if (!reply)
+ if (IS_ERR(reply))
return PTR_ERR(reply);
if (nla_put_u32(reply, OVS_METER_ATTR_MAX_METERS, U32_MAX) ||
--
1.8.3.1
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH 07/12] datapath: Using kfree_rcu() to simplify the code
From: Wei Yongjun
Upstream commit:
commit 6dc14dc40a1d1dafd8491c349b5f3e15aabc4edb
Author: Wei Yongjun
Date: Tue Nov 14 06:27:12 2017 +
openvswitch: Using kfree_rcu() to simplify the code
The callback function of call_rcu() just calls a kfree(), so we
can use kfree_rcu() instead of call_rcu() + callback function.
Signed-off-by: Wei Yongjun
Acked-by: Pravin B Shelar
Signed-off-by: David S. Miller
Cc: Wei Yongjun
Signed-off-by: Greg Rose
---
datapath/meter.c | 9 +
1 file changed, 1 insertion(+), 8 deletions(-)
diff --git a/datapath/meter.c b/datapath/meter.c
index 2e58b6c..52ddd6c 100644
--- a/datapath/meter.c
+++ b/datapath/meter.c
@@ -42,19 +42,12 @@ static const struct nla_policy
band_policy[OVS_BAND_ATTR_MAX + 1] = {
[OVS_BAND_ATTR_STATS] = { .len = sizeof(struct ovs_flow_stats) },
};
-static void rcu_free_ovs_meter_callback(struct rcu_head *rcu)
-{
- struct dp_meter *meter = container_of(rcu, struct dp_meter, rcu);
-
- kfree(meter);
-}
-
static void ovs_meter_free(struct dp_meter *meter)
{
if (!meter)
return;
- call_rcu(&meter->rcu, rcu_free_ovs_meter_callback);
+ kfree_rcu(meter, rcu);
}
static struct hlist_head *meter_hash_bucket(const struct datapath *dp,
--
1.8.3.1
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH 08/12] datapath: meter: fix NULL pointer dereference in ovs_meter_cmd_reply_start
From: "Gustavo A. R. Silva"
Upstream commit:
commit b74912a2fdae9aadd20da502644aa8848c861954
Author: Gustavo A. R. Silva
Date: Tue Nov 14 14:26:16 2017 -0600
openvswitch: meter: fix NULL pointer dereference in ovs_meter_cmd_reply_star
It seems that the intention of the code is to null check the value
returned by function genlmsg_put. But the current code is null
checking the address of the pointer that holds the value returned
by genlmsg_put.
Fix this by properly null checking the value returned by function
genlmsg_put in order to avoid a pontential null pointer dereference.
Addresses-Coverity-ID: 1461561 ("Dereference before null check")
Addresses-Coverity-ID: 1461562 ("Dereference null return value")
Fixes: 96fbc13d7e77 ("openvswitch: Add meter infrastructure")
Signed-off-by: Gustavo A. R. Silva
Signed-off-by: David S. Miller
Cc: Gustavo A. R. Silva
Signed-off-by: Greg Rose
---
datapath/meter.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/datapath/meter.c b/datapath/meter.c
index 52ddd6c..3fbfc78 100644
--- a/datapath/meter.c
+++ b/datapath/meter.c
@@ -99,7 +99,7 @@ ovs_meter_cmd_reply_start(struct genl_info *info, u8 cmd,
*ovs_reply_header = genlmsg_put(skb, info->snd_portid,
info->snd_seq,
&dp_meter_genl_family, 0, cmd);
- if (!ovs_reply_header) {
+ if (!*ovs_reply_header) {
nlmsg_free(skb);
return ERR_PTR(-EMSGSIZE);
}
--
1.8.3.1
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH 09/12] datapath: fix data type in queue_gso_packets
From: "Gustavo A. R. Silva"
Upstream commit:
commit 2734166e89639c973c6e125ac8bcfc2d9db72b70
Author: Gustavo A. R. Silva
Date: Sat Nov 25 13:14:40 2017 -0600
net: openvswitch: datapath: fix data type in queue_gso_packets
gso_type is being used in binary AND operations together with SKB_GSO_UDP.
The issue is that variable gso_type is of type unsigned short and
SKB_GSO_UDP expands to more than 16 bits:
SKB_GSO_UDP = 1 << 16
this makes any binary AND operation between gso_type and SKB_GSO_UDP to
be always zero, hence making some code unreachable and likely causing
undesired behavior.
Fix this by changing the data type of variable gso_type to unsigned int.
Addresses-Coverity-ID: 1462223
Fixes: 0c19f846d582 ("net: accept UFO datagrams from tuntap and packet")
Signed-off-by: Gustavo A. R. Silva
Acked-by: Willem de Bruijn
Signed-off-by: David S. Miller
Cc: Gustavo A. R. Silva
Signed-off-by: Greg Rose
---
datapath/datapath.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/datapath/datapath.c b/datapath/datapath.c
index eff9f85..9baa52e 100644
--- a/datapath/datapath.c
+++ b/datapath/datapath.c
@@ -312,7 +312,7 @@ static int queue_gso_packets(struct datapath *dp, struct
sk_buff *skb,
const struct dp_upcall_info *upcall_info,
uint32_t cutlen)
{
- unsigned short gso_type = skb_shinfo(skb)->gso_type;
+ unsigned int gso_type = skb_shinfo(skb)->gso_type;
struct sw_flow_key later_key;
struct sk_buff *segs, *nskb;
struct ovs_skb_cb ovs_cb;
--
1.8.3.1
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH 10/12] datapath: fix the incorrect flow action alloc size
From: zhangliping
Upstream commit:
commit 67c8d22a73128ff910e2287567132530abcf5b71
Author: zhangliping
Date: Sat Nov 25 22:02:12 2017 +0800
openvswitch: fix the incorrect flow action alloc size
If we want to add a datapath flow, which has more than 500 vxlan outputs'
action, we will get the following error reports:
openvswitch: netlink: Flow action size 32832 bytes exceeds max
openvswitch: netlink: Flow action size 32832 bytes exceeds max
openvswitch: netlink: Actions may not be safe on all matching packets
... ...
It seems that we can simply enlarge the MAX_ACTIONS_BUFSIZE to fix it, but
this is not the root cause. For example, for a vxlan output action, we need
about 60 bytes for the nlattr, but after it is converted to the flow
action, it only occupies 24 bytes. This means that we can still support
more than 1000 vxlan output actions for a single datapath flow under the
the current 32k max limitation.
So even if the nla_len(attr) is larger than MAX_ACTIONS_BUFSIZE, we
shouldn't report EINVAL and keep it move on, as the judgement can be
done by the reserve_sfa_size.
Signed-off-by: zhangliping
Acked-by: Pravin B Shelar
Signed-off-by: David S. Miller
Cc: zhangliping
Signed-off-by: Greg Rose
---
datapath/flow_netlink.c | 16
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/datapath/flow_netlink.c b/datapath/flow_netlink.c
index b3b2092..f687153 100644
--- a/datapath/flow_netlink.c
+++ b/datapath/flow_netlink.c
@@ -1908,14 +1908,11 @@ int ovs_nla_put_mask(const struct sw_flow *flow, struct
sk_buff *skb)
#define MAX_ACTIONS_BUFSIZE(32 * 1024)
#endif
-static struct sw_flow_actions *nla_alloc_flow_actions(int size, bool log)
+static struct sw_flow_actions *nla_alloc_flow_actions(int size)
{
struct sw_flow_actions *sfa;
- if (size > MAX_ACTIONS_BUFSIZE) {
- OVS_NLERR(log, "Flow action size %u bytes exceeds max", size);
- return ERR_PTR(-EINVAL);
- }
+ WARN_ON_ONCE(size > MAX_ACTIONS_BUFSIZE);
sfa = kmalloc(sizeof(*sfa) + size, GFP_KERNEL);
if (!sfa)
@@ -1988,12 +1985,15 @@ static struct nlattr *reserve_sfa_size(struct
sw_flow_actions **sfa,
new_acts_size = ksize(*sfa) * 2;
if (new_acts_size > MAX_ACTIONS_BUFSIZE) {
- if ((MAX_ACTIONS_BUFSIZE - next_offset) < req_size)
+ if ((MAX_ACTIONS_BUFSIZE - next_offset) < req_size) {
+ OVS_NLERR(log, "Flow action size exceeds max %u",
+ MAX_ACTIONS_BUFSIZE);
return ERR_PTR(-EMSGSIZE);
+ }
new_acts_size = MAX_ACTIONS_BUFSIZE;
}
- acts = nla_alloc_flow_actions(new_acts_size, log);
+ acts = nla_alloc_flow_actions(new_acts_size);
if (IS_ERR(acts))
return (void *)acts;
@@ -2668,7 +2668,7 @@ int ovs_nla_copy_actions(struct net *net, const struct
nlattr *attr,
{
int err;
- *sfa = nla_alloc_flow_actions(nla_len(attr), log);
+ *sfa = nla_alloc_flow_actions(min(nla_len(attr), MAX_ACTIONS_BUFSIZE));
if (IS_ERR(*sfa))
return PTR_ERR(*sfa);
--
1.8.3.1
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH 12/12] datapath: do not propagate headroom updates to internal port
From: Paolo Abeni
Upstream commit:
commit 183dea5818315c0a172d21ecbcd2554894bf01e3
Author: Paolo Abeni
Date: Thu Nov 30 15:35:33 2017 +0100
openvswitch: do not propagate headroom updates to internal port
After commit 3a927bc7cf9d ("ovs: propagate per dp max headroom to
all vports") the need_headroom for the internal vport is updated
accordingly to the max needed headroom in its datapath.
That avoids the pskb_expand_head() costs when sending/forwarding
packets towards tunnel devices, at least for some scenarios.
We still require such copy when using the ovs-preferred configuration
for vxlan tunnels:
br_int
/ \
tap vxlan
(remote_ip:X)
br_phy
\
NIC
where the route towards the IP 'X' is via 'br_phy'.
When forwarding traffic from the tap towards the vxlan device, we
will call pskb_expand_head() in vxlan_build_skb() because
br-phy->needed_headroom is equal to tun->needed_headroom.
With this change we avoid updating the internal vport needed_headroom,
so that in the above scenario no head copy is needed, giving 5%
performance improvement in UDP throughput test.
As a trade-off, packets sent from the internal port towards a tunnel
device will now experience the head copy overhead. The rationale is
that the latter use-case is less relevant performance-wise.
Signed-off-by: Paolo Abeni
Acked-by: Pravin B Shelar
Signed-off-by: David S. Miller
Also removed related compat layer changes.
Cc: Paolo Abeni
Signed-off-by: Greg Rose
---
acinclude.m4| 1 -
datapath/linux/compat/include/linux/netdevice.h | 19 ---
datapath/vport-internal_dev.c | 19 +--
3 files changed, 1 insertion(+), 38 deletions(-)
diff --git a/acinclude.m4 b/acinclude.m4
index 30a2103..0cf41d3 100644
--- a/acinclude.m4
+++ b/acinclude.m4
@@ -553,7 +553,6 @@ AC_DEFUN([OVS_CHECK_LINUX_COMPAT], [
OVS_GREP_IFELSE([$KSRC/include/linux/netdevice.h], [udp_offload.*uoff],
[OVS_DEFINE([HAVE_UDP_OFFLOAD_ARG_UOFF])])
OVS_GREP_IFELSE([$KSRC/include/linux/netdevice.h], [gro_remcsum])
- OVS_GREP_IFELSE([$KSRC/include/linux/netdevice.h], [IFF_PHONY_HEADROOM])
OVS_FIND_FIELD_IFELSE([$KSRC/include/linux/netdevice.h], [net_device_ops],
[extended])
OVS_FIND_PARAM_IFELSE([$KSRC/include/linux/netdevice.h],
diff --git a/datapath/linux/compat/include/linux/netdevice.h
b/datapath/linux/compat/include/linux/netdevice.h
index 3c3cf42..87f8469 100644
--- a/datapath/linux/compat/include/linux/netdevice.h
+++ b/datapath/linux/compat/include/linux/netdevice.h
@@ -244,25 +244,6 @@ int ovs_dev_fill_metadata_dst(struct net_device *dev,
struct sk_buff *skb);
#define NETDEV_OFFLOAD_PUSH_GENEVE 0x001D
#endif
-#ifndef HAVE_IFF_PHONY_HEADROOM
-
-#define IFF_PHONY_HEADROOM 0
-static inline unsigned netdev_get_fwd_headroom(struct net_device *dev)
-{
- return 0;
-}
-
-static inline void netdev_set_rx_headroom(struct net_device *dev, int new_hr)
-{
-}
-
-/* set the device rx headroom to the dev's default */
-static inline void netdev_reset_rx_headroom(struct net_device *dev)
-{
-}
-
-#endif
-
#ifdef IFF_NO_QUEUE
#define HAVE_IFF_NO_QUEUE
#else
diff --git a/datapath/vport-internal_dev.c b/datapath/vport-internal_dev.c
index 0aa331a..9bb8751 100644
--- a/datapath/vport-internal_dev.c
+++ b/datapath/vport-internal_dev.c
@@ -149,13 +149,6 @@ internal_get_stats(struct net_device *dev, struct
rtnl_link_stats64 *stats)
}
}
-#ifdef HAVE_IFF_PHONY_HEADROOM
-static void internal_set_rx_headroom(struct net_device *dev, int new_hr)
-{
- dev->needed_headroom = new_hr < 0 ? 0 : new_hr;
-}
-#endif
-
static const struct net_device_ops internal_dev_netdev_ops = {
.ndo_open = internal_dev_open,
.ndo_stop = internal_dev_stop,
@@ -165,13 +158,6 @@ static const struct net_device_ops internal_dev_netdev_ops
= {
.ndo_change_mtu = internal_dev_change_mtu,
#endif
.ndo_get_stats64 = (void *)internal_get_stats,
-#ifdef HAVE_IFF_PHONY_HEADROOM
-#ifndef HAVE_NET_DEVICE_OPS_WITH_EXTENDED
- .ndo_set_rx_headroom = internal_set_rx_headroom,
-#else
- .extended.ndo_set_rx_headroom = internal_set_rx_headroom,
-#endif
-#endif
};
static struct rtnl_link_ops internal_dev_link_ops __read_mostly = {
@@ -189,7 +175,7 @@ static void do_setup(struct net_device *netdev)
netdev->priv_flags &= ~IFF_TX_SKB_SHARING;
netdev->priv_flags |= IFF_LIVE_ADDR_CHANGE | IFF_OPENVSWITCH |
- IFF_PHONY_HEADROOM | IFF_NO_QUEUE;
+ IFF_NO_QUEUE;
#ifndef HAVE_NEEDS_FREE_NETDEV
netdev->destructor = internal_dev_destructor;
#else
@@ -239,9 +225,6 @@ static struct vport *internal_dev_create(const struct
vport_parms *parms)
goto error_free_netdev;
}
[ovs-dev] [PATCH 11/12] datapath: use ktime_get_ts64() instead of ktime_get_ts()
From: Arnd Bergmann
Upstream commit:
commit 311af51dcb5629f04976a8e451673f77e3301041
Author: Arnd Bergmann
Date: Mon Nov 27 12:41:38 2017 +0100
openvswitch: use ktime_get_ts64() instead of ktime_get_ts()
timespec is deprecated because of the y2038 overflow, so let's convert
this one to ktime_get_ts64(). The code is already safe even on 32-bit
architectures, since it uses monotonic times. On 64-bit architectures,
nothing changes, while on 32-bit architectures this avoids one
type conversion.
Signed-off-by: Arnd Bergmann
Signed-off-by: David S. Miller
Cc: Arnd Bergmann
Signed-off-by: Greg Rose
---
datapath/flow.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/datapath/flow.c b/datapath/flow.c
index 5da7e3e..c119df2 100644
--- a/datapath/flow.c
+++ b/datapath/flow.c
@@ -54,12 +54,12 @@
u64 ovs_flow_used_time(unsigned long flow_jiffies)
{
- struct timespec cur_ts;
+ struct timespec64 cur_ts;
u64 cur_ms, idle_ms;
- ktime_get_ts(&cur_ts);
+ ktime_get_ts64(&cur_ts);
idle_ms = jiffies_to_msecs(jiffies - flow_jiffies);
- cur_ms = (u64)cur_ts.tv_sec * MSEC_PER_SEC +
+ cur_ms = (u64)(u32)cur_ts.tv_sec * MSEC_PER_SEC +
cur_ts.tv_nsec / NSEC_PER_MSEC;
return cur_ms - idle_ms;
--
1.8.3.1
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] OVS DPDK: dpdk_merge pull request
On Mon, Dec 11, 2017 at 06:56:12PM +, Stokes, Ian wrote: > > On Fri, Dec 08, 2017 at 10:14:49PM +, Stokes, Ian wrote: > > > The following changes since commit > > 65d9759c4fc433dbda89ad3d7225c1f5eac274ca: > > > > > > ovsdb-data: Add OVS_WARN_UNUSED_RESULT annotations to function > > definitions. (2017-12-08 13:39:29 -0800) > > > > > > are available in the git repository at: > > > > > > https://github.com/istokes/ovs dpdk_merge > > > > > > for you to fetch changes up to 3eb8d4fa0db3159a8ffc8f52223417b3417263b3: > > > > Thanks a lot. I merged this to master. > > Thanks for this Ben. > > > > > This merge added one new "sparse" warning. I sent a fix for review: > > https://patchwork.ozlabs.org/patch/847165/ > > > I recompiled with sparse to see if I could reproduce the issue but I don't > see that warning. > > I'm concerned that there could be a problem with sparse on my build system at > this stage as this is the second time an issue picked up by your build but > not on mine. Do you build with C=1 on the make command line? That is required to enable sparse, if it is installed. This has caused surprises for others in the past. Maybe we should change it. > I obviously need to look into this, out of interest what version of sparse do > you use? It looks like I have v0.5.0-44-g40791b9. I don't think the particular version should matter in this case. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [patch v3 3/3] conntrack: Disable algs by default.
Thanks, Darrell and Aaron. I applied this series to master. On Wed, Dec 06, 2017 at 07:10:03PM +, Darrell Ball wrote: > Thanks for the reviews Aaron. > > Darrell > > > > On 12/6/17, 11:04 AM, "ovs-dev-boun...@openvswitch.org on behalf of Aaron > Conole" > wrote: > > Darrell Ball writes: > > > Presently, alg processing is enabled by default to better exercise code. > > This is similar to kernels before 4.7 as well. The recommended default > > behavior in the newer kernels is to only process algs if a helper is > > supplied in a conntrack rule. The behavior is changed to match the > > later kernels. > > > > A test is extended to check that the control connection is still > > created in such a case. > > > > Signed-off-by: Darrell Ball > > --- > > LGTM. > > Acked-by: Aaron Conole > ___ > dev mailing list > d...@openvswitch.org > > https://urldefense.proofpoint.com/v2/url?u=https-3A__mail.openvswitch.org_mailman_listinfo_ovs-2Ddev&d=DwICAg&c=uilaK90D4TOVoH58JNXRgQ&r=BVhFA09CGX7JQ5Ih-uZnsw&m=D7-nKQ8low4ohzl_EtbVC_dQ1XBd0pg8gTmTzeYVte0&s=o95-s6j6eDyJTjLmhlMjK8t_qwOO439Wigl_cgMNv0Q&e= > > > ___ > dev mailing list > d...@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [patch v1] conntrack: Fix icmp error address sanity check.
On Wed, Dec 06, 2017 at 06:04:20PM -0800, Darrell Ball wrote:
> An address sanity check is done on icmp error packets to
> check that the icmp error payload makes sense w.r.t. the
> packet itself.
>
> The sanity check was partially incorrect since it tried
> to verify the source address of the error packet against the
> original destination, which does not makes since the error
> can be generated by any intermediate node.
>
> Reported-by: wangzhike
> Reported-at:
> https://mail.openvswitch.org/pipermail/ovs-dev/2017-December/341609.html
> Fixes: a489b1685 ("conntrack: New userspace connection tracker.")
> CC: Daniele Di Proietto
> Signed-off-by: Darrell Ball
> Signed-off-by: wangzhike
> Co-authored-by: wangzhike
Thanks Darrell and wangzhike, I applied this to master.
Let me know if this or the other series I recently applied needs
backporting.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [patch v1] conntrack: Fix icmp error address sanity check.
Needs to go back to 2.6; at least the changes in lib/conntrack.c
Thanks Darrell
On 12/11/17, 2:20 PM, "ovs-dev-boun...@openvswitch.org on behalf of Ben Pfaff"
wrote:
On Wed, Dec 06, 2017 at 06:04:20PM -0800, Darrell Ball wrote:
> An address sanity check is done on icmp error packets to
> check that the icmp error payload makes sense w.r.t. the
> packet itself.
>
> The sanity check was partially incorrect since it tried
> to verify the source address of the error packet against the
> original destination, which does not makes since the error
> can be generated by any intermediate node.
>
> Reported-by: wangzhike
> Reported-at:
https://urldefense.proofpoint.com/v2/url?u=https-3A__mail.openvswitch.org_pipermail_ovs-2Ddev_2017-2DDecember_341609.html&d=DwICAg&c=uilaK90D4TOVoH58JNXRgQ&r=BVhFA09CGX7JQ5Ih-uZnsw&m=C2QYD5JosF5j-X3I7nmikMajbKHF9WqhgGQZdigkYP8&s=1wKg_9t4kVTzgXsNG4PwRgPDVzKsu1POFWnH6x-eX-U&e=
> Fixes: a489b1685 ("conntrack: New userspace connection tracker.")
> CC: Daniele Di Proietto
> Signed-off-by: Darrell Ball
> Signed-off-by: wangzhike
> Co-authored-by: wangzhike
Thanks Darrell and wangzhike, I applied this to master.
Let me know if this or the other series I recently applied needs
backporting.
___
dev mailing list
d...@openvswitch.org
https://urldefense.proofpoint.com/v2/url?u=https-3A__mail.openvswitch.org_mailman_listinfo_ovs-2Ddev&d=DwICAg&c=uilaK90D4TOVoH58JNXRgQ&r=BVhFA09CGX7JQ5Ih-uZnsw&m=C2QYD5JosF5j-X3I7nmikMajbKHF9WqhgGQZdigkYP8&s=DzvbErqrp1sqYj50u5y8oMwBgVZVFjUdhRitCukx98c&e=
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [patch v1] conntrack: Fix icmp error address sanity check.
It fails to apply due to conflicts in system-traffic.at. Is it safe to
drop that change and apply the rest?
On Mon, Dec 11, 2017 at 10:22:39PM +, Darrell Ball wrote:
> Needs to go back to 2.6; at least the changes in lib/conntrack.c
>
> Thanks Darrell
>
> On 12/11/17, 2:20 PM, "ovs-dev-boun...@openvswitch.org on behalf of Ben
> Pfaff" wrote:
>
> On Wed, Dec 06, 2017 at 06:04:20PM -0800, Darrell Ball wrote:
> > An address sanity check is done on icmp error packets to
> > check that the icmp error payload makes sense w.r.t. the
> > packet itself.
> >
> > The sanity check was partially incorrect since it tried
> > to verify the source address of the error packet against the
> > original destination, which does not makes since the error
> > can be generated by any intermediate node.
> >
> > Reported-by: wangzhike
> > Reported-at:
> https://urldefense.proofpoint.com/v2/url?u=https-3A__mail.openvswitch.org_pipermail_ovs-2Ddev_2017-2DDecember_341609.html&d=DwICAg&c=uilaK90D4TOVoH58JNXRgQ&r=BVhFA09CGX7JQ5Ih-uZnsw&m=C2QYD5JosF5j-X3I7nmikMajbKHF9WqhgGQZdigkYP8&s=1wKg_9t4kVTzgXsNG4PwRgPDVzKsu1POFWnH6x-eX-U&e=
> > Fixes: a489b1685 ("conntrack: New userspace connection tracker.")
> > CC: Daniele Di Proietto
> > Signed-off-by: Darrell Ball
> > Signed-off-by: wangzhike
> > Co-authored-by: wangzhike
>
> Thanks Darrell and wangzhike, I applied this to master.
>
> Let me know if this or the other series I recently applied needs
> backporting.
> ___
> dev mailing list
> d...@openvswitch.org
>
> https://urldefense.proofpoint.com/v2/url?u=https-3A__mail.openvswitch.org_mailman_listinfo_ovs-2Ddev&d=DwICAg&c=uilaK90D4TOVoH58JNXRgQ&r=BVhFA09CGX7JQ5Ih-uZnsw&m=C2QYD5JosF5j-X3I7nmikMajbKHF9WqhgGQZdigkYP8&s=DzvbErqrp1sqYj50u5y8oMwBgVZVFjUdhRitCukx98c&e=
>
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH 4/5] ovsdb-idl: Remove 'uuid' member of struct ovsdb_idl.
> -Original Message- > From: ovs-dev-boun...@openvswitch.org [mailto:ovs-dev- > boun...@openvswitch.org] On Behalf Of Ben Pfaff > Sent: Friday, December 8, 2017 11:24 PM > To: d...@openvswitch.org > Cc: Ben Pfaff > Subject: [ovs-dev] [PATCH 4/5] ovsdb-idl: Remove 'uuid' member of struct > ovsdb_idl. > > This was used to uniquely identify the monitor, but there's no need for that. > A fixed monitor name works fine. > > Signed-off-by: Ben Pfaff > --- Acked-by: Alin Gabriel Serdean ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [patch v1] conntrack: Fix icmp error address sanity check.
Yes, it is Ben
Thanks Darrell
On 12/11/17, 2:27 PM, "Ben Pfaff" wrote:
It fails to apply due to conflicts in system-traffic.at. Is it safe to
drop that change and apply the rest?
On Mon, Dec 11, 2017 at 10:22:39PM +, Darrell Ball wrote:
> Needs to go back to 2.6; at least the changes in lib/conntrack.c
>
> Thanks Darrell
>
> On 12/11/17, 2:20 PM, "ovs-dev-boun...@openvswitch.org on behalf of Ben
Pfaff" wrote:
>
> On Wed, Dec 06, 2017 at 06:04:20PM -0800, Darrell Ball wrote:
> > An address sanity check is done on icmp error packets to
> > check that the icmp error payload makes sense w.r.t. the
> > packet itself.
> >
> > The sanity check was partially incorrect since it tried
> > to verify the source address of the error packet against the
> > original destination, which does not makes since the error
> > can be generated by any intermediate node.
> >
> > Reported-by: wangzhike
> > Reported-at:
https://urldefense.proofpoint.com/v2/url?u=https-3A__mail.openvswitch.org_pipermail_ovs-2Ddev_2017-2DDecember_341609.html&d=DwICAg&c=uilaK90D4TOVoH58JNXRgQ&r=BVhFA09CGX7JQ5Ih-uZnsw&m=C2QYD5JosF5j-X3I7nmikMajbKHF9WqhgGQZdigkYP8&s=1wKg_9t4kVTzgXsNG4PwRgPDVzKsu1POFWnH6x-eX-U&e=
> > Fixes: a489b1685 ("conntrack: New userspace connection tracker.")
> > CC: Daniele Di Proietto
> > Signed-off-by: Darrell Ball
> > Signed-off-by: wangzhike
> > Co-authored-by: wangzhike
>
> Thanks Darrell and wangzhike, I applied this to master.
>
> Let me know if this or the other series I recently applied needs
> backporting.
> ___
> dev mailing list
> d...@openvswitch.org
>
https://urldefense.proofpoint.com/v2/url?u=https-3A__mail.openvswitch.org_mailman_listinfo_ovs-2Ddev&d=DwICAg&c=uilaK90D4TOVoH58JNXRgQ&r=BVhFA09CGX7JQ5Ih-uZnsw&m=C2QYD5JosF5j-X3I7nmikMajbKHF9WqhgGQZdigkYP8&s=DzvbErqrp1sqYj50u5y8oMwBgVZVFjUdhRitCukx98c&e=
>
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH 1/5] ovsdb-idl: Improve comments.
On Mon, Dec 11, 2017 at 08:23:44PM +, Alin Serdean wrote: > LGTM. Just two nits. > > +/* Terminal error state that indicates that nothing useful can be done. > > + * The most likely reason is that the database server doesn't actually > > have > [Alin Serdean] doesn't have, maybe? OK, thanks. > > @@ -1118,7 +1153,7 @@ ovsdb_idl_condition_clone(struct > > ovsdb_idl_condition *dst, > > * arranges to send the new condition to the database server. > > * > > * Return the next conditional update sequence number. When this > > - * value and ovsdb_idl_get_condition_seqno() matchs, the 'idl' > > + * value and ovsdb_idl_get_condition_seqno() matches, the 'idl' > > * contains rows that match the 'condition'. > > */ > > unsigned int > > -- > s/. *\//. *\// I don't understand that suggestion. Maybe you were suggesting that */ should be on the same line as the last word; if so, OK, sure. > Acked-by: Alin Gabriel Serdean Thank you for the review. I'll apply patches 1-3 soon. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH 5/5] ovsdb-idl: Tolerate initialization races for singleton tables.
> -Original Message- > From: ovs-dev-boun...@openvswitch.org [mailto:ovs-dev- > boun...@openvswitch.org] On Behalf Of Ben Pfaff > Sent: Friday, December 8, 2017 11:25 PM > To: d...@openvswitch.org > Cc: Ben Pfaff > Subject: [ovs-dev] [PATCH 5/5] ovsdb-idl: Tolerate initialization races for > singleton tables. > > By verifying that singleton tables (that is, tables that should have exactly > one > row) are empty when they emit transactions that insert into them, ovs-vsctl > and similar tools tolerate initialization races, where more than one client > at a > time tries to initialize a singleton table. > > The upshot is that if you create a database and then run multiple ovs-vsctl > (etc.) commands against it in parallel (without first initializing it > serially), then > without this patch sometimes you will sometimes get failures but this patch > avoids them. > > Signed-off-by: Ben Pfaff > --- I ran some tests and all the unit tests. Seems ok to me. Acked-by: Alin Gabriel Serdean ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH 1/5] ovsdb-idl: Improve comments.
> -Original Message- > From: Ben Pfaff [mailto:b...@ovn.org] > Sent: Tuesday, December 12, 2017 12:30 AM > To: Alin Serdean > Cc: d...@openvswitch.org > Subject: Re: [ovs-dev] [PATCH 1/5] ovsdb-idl: Improve comments. > > On Mon, Dec 11, 2017 at 08:23:44PM +, Alin Serdean wrote: > > LGTM. Just two nits. > > > > +/* Terminal error state that indicates that nothing useful can be > > > done. > > > + * The most likely reason is that the database server doesn't > > > + actually have > > [Alin Serdean] doesn't have, maybe? > > OK, thanks. > > > > @@ -1118,7 +1153,7 @@ ovsdb_idl_condition_clone(struct > > > ovsdb_idl_condition *dst, > > > * arranges to send the new condition to the database server. > > > * > > > * Return the next conditional update sequence number. When this > > > - * value and ovsdb_idl_get_condition_seqno() matchs, the 'idl' > > > + * value and ovsdb_idl_get_condition_seqno() matches, the 'idl' > > > * contains rows that match the 'condition'. > > > */ > > > unsigned int > > > -- > > s/. *\//. *\// > > I don't understand that suggestion. Maybe you were suggesting that */ > should be on the same line as the last word; if so, OK, sure. [Alin Serdean] I think in some places there is `. */` vs `. */` , but maybe my email client messed up the formatting. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH 1/5] ovsdb-idl: Improve comments.
On Mon, Dec 11, 2017 at 10:34:06PM +, Alin Serdean wrote: > > > > -Original Message- > > From: Ben Pfaff [mailto:b...@ovn.org] > > Sent: Tuesday, December 12, 2017 12:30 AM > > To: Alin Serdean > > Cc: d...@openvswitch.org > > Subject: Re: [ovs-dev] [PATCH 1/5] ovsdb-idl: Improve comments. > > > > On Mon, Dec 11, 2017 at 08:23:44PM +, Alin Serdean wrote: > > > LGTM. Just two nits. > > > > > > +/* Terminal error state that indicates that nothing useful can be > > > > done. > > > > + * The most likely reason is that the database server doesn't > > > > + actually have > > > [Alin Serdean] doesn't have, maybe? > > > > OK, thanks. > > > > > > @@ -1118,7 +1153,7 @@ ovsdb_idl_condition_clone(struct > > > > ovsdb_idl_condition *dst, > > > > * arranges to send the new condition to the database server. > > > > * > > > > * Return the next conditional update sequence number. When this > > > > - * value and ovsdb_idl_get_condition_seqno() matchs, the 'idl' > > > > + * value and ovsdb_idl_get_condition_seqno() matches, the 'idl' > > > > * contains rows that match the 'condition'. > > > > */ > > > > unsigned int > > > > -- > > > s/. *\//. *\// > > > > I don't understand that suggestion. Maybe you were suggesting that */ > > should be on the same line as the last word; if so, OK, sure. > [Alin Serdean] I think in some places there is `. */` vs `. */` , but maybe > my email client messed up the formatting. Oh. Got it. I'll try to be more consistent. (I was a little confused by regex syntax. '.' and '*' are special characters.) ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [patch v1] conntrack: Fix icmp error address sanity check.
OK, I made that change and applied it to branch-2.8. It didn't apply
cleanly to 2.7 or 2.6, can you look at that?
On Mon, Dec 11, 2017 at 10:28:32PM +, Darrell Ball wrote:
> Yes, it is Ben
>
> Thanks Darrell
>
> On 12/11/17, 2:27 PM, "Ben Pfaff" wrote:
>
> It fails to apply due to conflicts in system-traffic.at. Is it safe to
> drop that change and apply the rest?
>
> On Mon, Dec 11, 2017 at 10:22:39PM +, Darrell Ball wrote:
> > Needs to go back to 2.6; at least the changes in lib/conntrack.c
> >
> > Thanks Darrell
> >
> > On 12/11/17, 2:20 PM, "ovs-dev-boun...@openvswitch.org on behalf of Ben
> Pfaff" wrote:
> >
> > On Wed, Dec 06, 2017 at 06:04:20PM -0800, Darrell Ball wrote:
> > > An address sanity check is done on icmp error packets to
> > > check that the icmp error payload makes sense w.r.t. the
> > > packet itself.
> > >
> > > The sanity check was partially incorrect since it tried
> > > to verify the source address of the error packet against the
> > > original destination, which does not makes since the error
> > > can be generated by any intermediate node.
> > >
> > > Reported-by: wangzhike
> > > Reported-at:
> https://urldefense.proofpoint.com/v2/url?u=https-3A__mail.openvswitch.org_pipermail_ovs-2Ddev_2017-2DDecember_341609.html&d=DwICAg&c=uilaK90D4TOVoH58JNXRgQ&r=BVhFA09CGX7JQ5Ih-uZnsw&m=C2QYD5JosF5j-X3I7nmikMajbKHF9WqhgGQZdigkYP8&s=1wKg_9t4kVTzgXsNG4PwRgPDVzKsu1POFWnH6x-eX-U&e=
> > > Fixes: a489b1685 ("conntrack: New userspace connection tracker.")
> > > CC: Daniele Di Proietto
> > > Signed-off-by: Darrell Ball
> > > Signed-off-by: wangzhike
> > > Co-authored-by: wangzhike
> >
> > Thanks Darrell and wangzhike, I applied this to master.
> >
> > Let me know if this or the other series I recently applied needs
> > backporting.
> > ___
> > dev mailing list
> > d...@openvswitch.org
> >
> https://urldefense.proofpoint.com/v2/url?u=https-3A__mail.openvswitch.org_mailman_listinfo_ovs-2Ddev&d=DwICAg&c=uilaK90D4TOVoH58JNXRgQ&r=BVhFA09CGX7JQ5Ih-uZnsw&m=C2QYD5JosF5j-X3I7nmikMajbKHF9WqhgGQZdigkYP8&s=DzvbErqrp1sqYj50u5y8oMwBgVZVFjUdhRitCukx98c&e=
> >
> >
>
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH 1/5] ovsdb-idl: Improve comments.
> -Original Message- > From: Ben Pfaff [mailto:b...@ovn.org] > Sent: Tuesday, December 12, 2017 12:40 AM > To: Alin Serdean > Cc: d...@openvswitch.org > Subject: Re: [ovs-dev] [PATCH 1/5] ovsdb-idl: Improve comments. > > On Mon, Dec 11, 2017 at 10:34:06PM +, Alin Serdean wrote: > > > > > > > -Original Message- > > > From: Ben Pfaff [mailto:b...@ovn.org] > > > Sent: Tuesday, December 12, 2017 12:30 AM > > > To: Alin Serdean > > > Cc: d...@openvswitch.org > > > Subject: Re: [ovs-dev] [PATCH 1/5] ovsdb-idl: Improve comments. > > > > > > On Mon, Dec 11, 2017 at 08:23:44PM +, Alin Serdean wrote: > > > > LGTM. Just two nits. > > > > > > > > +/* Terminal error state that indicates that nothing useful can be > done. > > > > > + * The most likely reason is that the database server > > > > > + doesn't actually have > > > > [Alin Serdean] doesn't have, maybe? > > > > > > OK, thanks. > > > > > > > > @@ -1118,7 +1153,7 @@ ovsdb_idl_condition_clone(struct > > > > > ovsdb_idl_condition *dst, > > > > > * arranges to send the new condition to the database server. > > > > > * > > > > > * Return the next conditional update sequence number. When > > > > > this > > > > > - * value and ovsdb_idl_get_condition_seqno() matchs, the 'idl' > > > > > + * value and ovsdb_idl_get_condition_seqno() matches, the 'idl' > > > > > * contains rows that match the 'condition'. > > > > > */ > > > > > unsigned int > > > > > -- > > > > s/. *\//. *\// > > > > > > I don't understand that suggestion. Maybe you were suggesting that > > > */ should be on the same line as the last word; if so, OK, sure. > > [Alin Serdean] I think in some places there is `. */` vs `. */` , but > > maybe my > email client messed up the formatting. > > Oh. Got it. I'll try to be more consistent. > > (I was a little confused by regex syntax. '.' and '*' are special > characters.) [Alin Serdean] My bad, I forgot to escape them 😊. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH 5/5] ovsdb-idl: Tolerate initialization races for singleton tables.
On Mon, Dec 11, 2017 at 10:30:46PM +, Alin Serdean wrote: > > > > -Original Message- > > From: ovs-dev-boun...@openvswitch.org [mailto:ovs-dev- > > boun...@openvswitch.org] On Behalf Of Ben Pfaff > > Sent: Friday, December 8, 2017 11:25 PM > > To: d...@openvswitch.org > > Cc: Ben Pfaff > > Subject: [ovs-dev] [PATCH 5/5] ovsdb-idl: Tolerate initialization races for > > singleton tables. > > > > By verifying that singleton tables (that is, tables that should have > > exactly one > > row) are empty when they emit transactions that insert into them, ovs-vsctl > > and similar tools tolerate initialization races, where more than one client > > at a > > time tries to initialize a singleton table. > > > > The upshot is that if you create a database and then run multiple ovs-vsctl > > (etc.) commands against it in parallel (without first initializing it > > serially), then > > without this patch sometimes you will sometimes get failures but this patch > > avoids them. > > > > Signed-off-by: Ben Pfaff > > --- > I ran some tests and all the unit tests. Seems ok to me. > > Acked-by: Alin Gabriel Serdean Thank you for the reviews and the testing. I applied all of these patches to master. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v2 2/3] conntrack: Support conntrack flush by ct 5-tuple
A few suggestions Yi-hung
Thanks Darrell
On 11/21/17, 5:04 PM, "ovs-dev-boun...@openvswitch.org on behalf of Yi-Hung
Wei" wrote:
This patch adds support of flushing a conntrack entry specified by the
conntrack 5-tuple in dpif-netdev.
Signed-off-by: Yi-Hung Wei
---
lib/conntrack.c | 64
+++
lib/conntrack.h | 3 +++
lib/dpif-netdev.c | 2 +-
3 files changed, 68 insertions(+), 1 deletion(-)
diff --git a/lib/conntrack.c b/lib/conntrack.c
index f5a3aa9fab34..c9077c07042c 100644
--- a/lib/conntrack.c
+++ b/lib/conntrack.c
@@ -2335,6 +2335,18 @@ ct_endpoint_to_ct_dpif_inet_addr(const struct
ct_addr *a,
}
static void
+ct_dpif_inet_addr_to_ct_endpoint(const union ct_dpif_inet_addr *a,
+ struct ct_addr *b,
+ const uint16_t l3_type)
[Darrell]
1/ typo here: no need for ‘const’ qualifier to l3_type or if dl_type were to be
used instead.
2/ In this file we typically select on dl_type, not AF_* for V4 vs V6
differentiation.
The caller already finds the dl_types, so it might be better just to use
dl_type here
to be consistent throughout the file
+{
+if (l3_type == AF_INET) {
+b->ipv4_aligned = a->ip;
+} else if (l3_type == AF_INET6) {
+b->ipv6_aligned = a->in6;
+}
+}
+
+static void
conn_key_to_tuple(const struct conn_key *key, struct ct_dpif_tuple *tuple)
{
if (key->dl_type == htons(ETH_TYPE_IP)) {
@@ -2359,6 +2371,35 @@ conn_key_to_tuple(const struct conn_key *key, struct
ct_dpif_tuple *tuple)
}
static void
+tuple_to_conn_key(const struct ct_dpif_tuple *tuple, uint16_t zone,
+ struct conn_key *key)
+{
+if (tuple->l3_type == AF_INET) {
+key->dl_type = htons(ETH_TYPE_IP);
+} else if (tuple->l3_type == AF_INET6) {
+key->dl_type = htons(ETH_TYPE_IPV6);
+}
+key->nw_proto = tuple->ip_proto;
+ct_dpif_inet_addr_to_ct_endpoint(&tuple->src, &key->src.addr,
+ tuple->l3_type);
+ct_dpif_inet_addr_to_ct_endpoint(&tuple->dst, &key->dst.addr,
+ tuple->l3_type);
+
+if (tuple->ip_proto == IPPROTO_ICMP || tuple->ip_proto ==
IPPROTO_ICMPV6) {
+key->src.icmp_id = tuple->icmp_id;
+key->src.icmp_type = tuple->icmp_type;
+key->src.icmp_code = tuple->icmp_code;
+key->dst.icmp_id = tuple->icmp_id;
+key->dst.icmp_type = reverse_icmp_type(tuple->icmp_type);
+key->dst.icmp_code = tuple->icmp_code;
+} else {
+key->src.port = tuple->src_port;
+key->dst.port = tuple->dst_port;
+}
+key->zone = zone;
+}
+
+static void
conn_to_ct_dpif_entry(const struct conn *conn, struct ct_dpif_entry *entry,
long long now, int bkt)
{
@@ -2485,6 +2526,29 @@ conntrack_flush(struct conntrack *ct, const uint16_t
*zone)
return 0;
}
+int
+conntrack_flush_tuple(struct conntrack *ct, const struct ct_dpif_tuple
*tuple,
+ uint16_t zone)
+{
+struct conn_lookup_ctx ctx;
+int error = 0;
+
+memset(&ctx, 0, sizeof(ctx));
+tuple_to_conn_key(tuple, zone, &ctx.key);
+ctx.hash = conn_key_hash(&ctx.key, ct->hash_basis);
+unsigned bucket = hash_to_bucket(ctx.hash);
+
+ct_lock_lock(&ct->buckets[bucket].lock);
+conn_key_lookup(&ct->buckets[bucket], &ctx, time_msec());
+if (ctx.conn) {
+conn_clean(ct, ctx.conn, &ct->buckets[bucket]);
+} else {
+error = ENOENT;
[Darrell] Not finding a conntrack entry is pretty normal since all entries
timeout normally.
Sending the error upwards is fine. However, I see dpctl raising
an error as a result, in the
other patches; is this intentional ?
+}
+ct_lock_unlock(&ct->buckets[bucket].lock);
+return error;
+}
+
/* This function must be called with the ct->resources read lock taken. */
static struct alg_exp_node *
expectation_lookup(struct hmap *alg_expectations,
diff --git a/lib/conntrack.h b/lib/conntrack.h
index fbeef1c4754e..b5b26dd96e00 100644
--- a/lib/conntrack.h
+++ b/lib/conntrack.h
@@ -107,6 +107,7 @@ struct conntrack_dump {
};
struct ct_dpif_entry;
+struct ct_dpif_tuple;
int conntrack_dump_start(struct conntrack *, struct conntrack_dump *,
const uint16_t *pzone, int *);
@@ -114,6 +115,8 @@ int conntrack_dump_next(struct conntrack_dump *, struct
ct_dpif_entry *);
int conntrack_dump_done(struct conntrack_dump *);
Re: [ovs-dev] [PATCH] travis: Use pip2 instead of pip for OSX build.
I applied this on master. > -Original Message- > From: ovs-dev-boun...@openvswitch.org [mailto:ovs-dev- > boun...@openvswitch.org] On Behalf Of aserd...@ovn.org > Sent: Monday, December 11, 2017 4:23 PM > To: 'Ilya Maximets' ; ovs-dev@openvswitch.org > Cc: 'Lance Richardson' ; 'Heetae Ahn' > > Subject: Re: [ovs-dev] [PATCH] travis: Use pip2 instead of pip for OSX build. > > > -Original Message- > > From: ovs-dev-boun...@openvswitch.org [mailto:ovs-dev- > > boun...@openvswitch.org] On Behalf Of Ilya Maximets > > Sent: Friday, December 8, 2017 2:53 PM > > To: ovs-dev@openvswitch.org > > Cc: Ilya Maximets ; Lance Richardson > > ; Heetae Ahn > > Subject: [ovs-dev] [PATCH] travis: Use pip2 instead of pip for OSX build. > > > > xcode8.3 is a new default image for OS X on Travis-CI, but it does not > have > > 'pip': > > > > pip install --user six > > ./.travis/osx-prepare.sh: line 3: pip: command not found > > > > 'pip2' or 'pip3' should be used explicitly instead: > > > > https://github.com/travis-ci/travis-ci/issues/8829 > > > > Signed-off-by: Ilya Maximets > > I haven't tested it, but I saw other people complaining about the same issue > on travis-ci. > > Acked-by: Alin Gabriel Serdean > > ___ > dev mailing list > d...@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [patch 2.7] conntrack: Fix icmp error address sanity check.
An address sanity check is done on icmp error packets to
check that the icmp error payload makes sense w.r.t. the
packet itself.
The sanity check was partially incorrect since it tried
to verify the source address of the error packet against the
original destination, which does not makes since the error
can be generated by any intermediate node.
Reported-by: wangzhike
Reported-at:
https://mail.openvswitch.org/pipermail/ovs-dev/2017-December/341609.html
Fixes: a489b1685 ("conntrack: New userspace connection tracker.")
CC: Daniele Di Proietto
Signed-off-by: Darrell Ball
Signed-off-by: wangzhike
Co-authored-by: wangzhike
Signed-off-by: Darrell Ball
---
lib/conntrack.c | 7 ++-
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/lib/conntrack.c b/lib/conntrack.c
index 677c0d2..4284770 100644
--- a/lib/conntrack.c
+++ b/lib/conntrack.c
@@ -780,8 +780,7 @@ extract_l4_icmp(struct conn_key *key, const void *data,
size_t size,
}
/* pf doesn't do this, but it seems a good idea */
-if (inner_key.src.addr.ipv4_aligned != key->dst.addr.ipv4_aligned
-|| inner_key.dst.addr.ipv4_aligned != key->src.addr.ipv4_aligned) {
+if (inner_key.src.addr.ipv4_aligned != key->dst.addr.ipv4_aligned) {
return false;
}
@@ -869,9 +868,7 @@ extract_l4_icmp6(struct conn_key *key, const void *data,
size_t size,
/* pf doesn't do this, but it seems a good idea */
if (!ipv6_addr_equals(&inner_key.src.addr.ipv6_aligned,
- &key->dst.addr.ipv6_aligned)
-|| !ipv6_addr_equals(&inner_key.dst.addr.ipv6_aligned,
- &key->src.addr.ipv6_aligned)) {
+ &key->dst.addr.ipv6_aligned)) {
return false;
}
--
1.9.1
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v6 0/4] nsh: add new nsh key ttl and action dec_nsh_ttl
On Tue, Dec 12, 2017 at 03:15:49AM +0800, Gregory Rose wrote: > On 12/11/2017 10:55 AM, Ben Pfaff wrote: > > On Mon, Dec 11, 2017 at 10:25:54AM -0800, Gregory Rose wrote: > >> On 12/10/2017 4:39 PM, Yang, Yi wrote: > >>> On Sat, Dec 09, 2017 at 12:42:32AM +0800, Gregory Rose wrote: > On 12/8/2017 6:04 AM, Yi Yang wrote: > > v5->v6 > > - Rebase v5 to master > > - Refactor netlink message format to align to NSH kernel > > implementation > > - Add dec_nsh_ttl unit test into tests/nsh.at > > - Fix unit test unstable issue > > > I don't see any backport of the upstream kernel datapath changes? I'm > working on catching our out of tree datapath code with the upstream > Linux kernel datapath and your patch (commit > b2d0f5d5dc53532e6f07bc546a476a55ebdfe0f3 " openvswitch: enable NSH > support") needs the backport as well as compat layer changes. > > Do you plan on doing that work? > > Thanks, > > - Greg > >>> Greg, yes, I'll backport kernel datapath patches once this patch series > >>> is merged. BTW this doesn't depend on the kernel datapath patches. > >> Understood that this series of patches you sent does not require the > >> kernel > >> datapath backport > >> but I found that the kernel datapath patches do modify the openvswitch uapi > >> header and adds > >> new switch cases. These need to be handled eventually which can be done > >> when you do the > >> backport. > > Greg and Yi, can you help me understand the compatibility and patch > > workflow implications of accepting this series now instead of after the > > kernel datapath backport? In your opinions, is it important to apply > > them in a particular order? > > Ben, > > I started backporting the upstream NSH patch from Yi and found that I > had to make some > related user space changes to make it compile correctly. So it would > appear that the upstream > kernel patch would require this set of patches first. > > That said, let's get Yi's response since he is far more familiar with > the NSH work than I am. > > Thanks, > > - Greg > We need to apply this patch series first, we do need to change some header files when we backport NSH kernel patch, this is inevitable , we only can support NSH userspace in current OVS. After applying this patch series, NSH kernel backport patches will make sure both userspace and kernel data path can work. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [patch v1] conntrack: Fix icmp error address sanity check.
Ben
I sent a 2.7 patch here:
https://patchwork.ozlabs.org/patch/847308/
it should be applicable for 2.6 as well.
Thanks Darrell
On 12/11/17, 2:43 PM, "Ben Pfaff" wrote:
OK, I made that change and applied it to branch-2.8. It didn't apply
cleanly to 2.7 or 2.6, can you look at that?
On Mon, Dec 11, 2017 at 10:28:32PM +, Darrell Ball wrote:
> Yes, it is Ben
>
> Thanks Darrell
>
> On 12/11/17, 2:27 PM, "Ben Pfaff" wrote:
>
> It fails to apply due to conflicts in system-traffic.at. Is it safe
to
> drop that change and apply the rest?
>
> On Mon, Dec 11, 2017 at 10:22:39PM +, Darrell Ball wrote:
> > Needs to go back to 2.6; at least the changes in lib/conntrack.c
> >
> > Thanks Darrell
> >
> > On 12/11/17, 2:20 PM, "ovs-dev-boun...@openvswitch.org on behalf of
Ben Pfaff" wrote:
> >
> > On Wed, Dec 06, 2017 at 06:04:20PM -0800, Darrell Ball wrote:
> > > An address sanity check is done on icmp error packets to
> > > check that the icmp error payload makes sense w.r.t. the
> > > packet itself.
> > >
> > > The sanity check was partially incorrect since it tried
> > > to verify the source address of the error packet against the
> > > original destination, which does not makes since the error
> > > can be generated by any intermediate node.
> > >
> > > Reported-by: wangzhike
> > > Reported-at:
https://urldefense.proofpoint.com/v2/url?u=https-3A__mail.openvswitch.org_pipermail_ovs-2Ddev_2017-2DDecember_341609.html&d=DwICAg&c=uilaK90D4TOVoH58JNXRgQ&r=BVhFA09CGX7JQ5Ih-uZnsw&m=C2QYD5JosF5j-X3I7nmikMajbKHF9WqhgGQZdigkYP8&s=1wKg_9t4kVTzgXsNG4PwRgPDVzKsu1POFWnH6x-eX-U&e=
> > > Fixes: a489b1685 ("conntrack: New userspace connection
tracker.")
> > > CC: Daniele Di Proietto
> > > Signed-off-by: Darrell Ball
> > > Signed-off-by: wangzhike
> > > Co-authored-by: wangzhike
> >
> > Thanks Darrell and wangzhike, I applied this to master.
> >
> > Let me know if this or the other series I recently applied needs
> > backporting.
> > ___
> > dev mailing list
> > d...@openvswitch.org
> >
https://urldefense.proofpoint.com/v2/url?u=https-3A__mail.openvswitch.org_mailman_listinfo_ovs-2Ddev&d=DwICAg&c=uilaK90D4TOVoH58JNXRgQ&r=BVhFA09CGX7JQ5Ih-uZnsw&m=C2QYD5JosF5j-X3I7nmikMajbKHF9WqhgGQZdigkYP8&s=DzvbErqrp1sqYj50u5y8oMwBgVZVFjUdhRitCukx98c&e=
> >
> >
>
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [patch v2 2.7] conntrack: Fix icmp error address sanity check.
An address sanity check is done on icmp error packets to
check that the icmp error payload makes sense w.r.t. the
packet itself.
The sanity check was partially incorrect since it tried
to verify the source address of the error packet against the
original destination, which does not makes since the error
can be generated by any intermediate node.
Reported-by: wangzhike
Reported-at:
https://mail.openvswitch.org/pipermail/ovs-dev/2017-December/341609.html
Fixes: a489b1685 ("conntrack: New userspace connection tracker.")
CC: Daniele Di Proietto
Signed-off-by: Darrell Ball
Signed-off-by: wangzhike
Co-authored-by: wangzhike
---
lib/conntrack.c | 7 ++-
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/lib/conntrack.c b/lib/conntrack.c
index 677c0d2..4284770 100644
--- a/lib/conntrack.c
+++ b/lib/conntrack.c
@@ -780,8 +780,7 @@ extract_l4_icmp(struct conn_key *key, const void *data,
size_t size,
}
/* pf doesn't do this, but it seems a good idea */
-if (inner_key.src.addr.ipv4_aligned != key->dst.addr.ipv4_aligned
-|| inner_key.dst.addr.ipv4_aligned != key->src.addr.ipv4_aligned) {
+if (inner_key.src.addr.ipv4_aligned != key->dst.addr.ipv4_aligned) {
return false;
}
@@ -869,9 +868,7 @@ extract_l4_icmp6(struct conn_key *key, const void *data,
size_t size,
/* pf doesn't do this, but it seems a good idea */
if (!ipv6_addr_equals(&inner_key.src.addr.ipv6_aligned,
- &key->dst.addr.ipv6_aligned)
-|| !ipv6_addr_equals(&inner_key.dst.addr.ipv6_aligned,
- &key->src.addr.ipv6_aligned)) {
+ &key->dst.addr.ipv6_aligned)) {
return false;
}
--
1.9.1
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [patch v1] conntrack: Fix icmp error address sanity check.
I sent a V2 here
https://patchwork.ozlabs.org/patch/847315/
an extra signoff had snuck into v1
Thanks Darrell
On 12/11/17, 5:22 PM, "Darrell Ball" wrote:
Ben
I sent a 2.7 patch here:
https://patchwork.ozlabs.org/patch/847308/
it should be applicable for 2.6 as well.
Thanks Darrell
On 12/11/17, 2:43 PM, "Ben Pfaff" wrote:
OK, I made that change and applied it to branch-2.8. It didn't apply
cleanly to 2.7 or 2.6, can you look at that?
On Mon, Dec 11, 2017 at 10:28:32PM +, Darrell Ball wrote:
> Yes, it is Ben
>
> Thanks Darrell
>
> On 12/11/17, 2:27 PM, "Ben Pfaff" wrote:
>
> It fails to apply due to conflicts in system-traffic.at. Is it
safe to
> drop that change and apply the rest?
>
> On Mon, Dec 11, 2017 at 10:22:39PM +, Darrell Ball wrote:
> > Needs to go back to 2.6; at least the changes in lib/conntrack.c
> >
> > Thanks Darrell
> >
> > On 12/11/17, 2:20 PM, "ovs-dev-boun...@openvswitch.org on
behalf of Ben Pfaff" wrote:
> >
> > On Wed, Dec 06, 2017 at 06:04:20PM -0800, Darrell Ball
wrote:
> > > An address sanity check is done on icmp error packets to
> > > check that the icmp error payload makes sense w.r.t. the
> > > packet itself.
> > >
> > > The sanity check was partially incorrect since it tried
> > > to verify the source address of the error packet against
the
> > > original destination, which does not makes since the error
> > > can be generated by any intermediate node.
> > >
> > > Reported-by: wangzhike
> > > Reported-at:
https://urldefense.proofpoint.com/v2/url?u=https-3A__mail.openvswitch.org_pipermail_ovs-2Ddev_2017-2DDecember_341609.html&d=DwICAg&c=uilaK90D4TOVoH58JNXRgQ&r=BVhFA09CGX7JQ5Ih-uZnsw&m=C2QYD5JosF5j-X3I7nmikMajbKHF9WqhgGQZdigkYP8&s=1wKg_9t4kVTzgXsNG4PwRgPDVzKsu1POFWnH6x-eX-U&e=
> > > Fixes: a489b1685 ("conntrack: New userspace connection
tracker.")
> > > CC: Daniele Di Proietto
> > > Signed-off-by: Darrell Ball
> > > Signed-off-by: wangzhike
> > > Co-authored-by: wangzhike
> >
> > Thanks Darrell and wangzhike, I applied this to master.
> >
> > Let me know if this or the other series I recently applied
needs
> > backporting.
> > ___
> > dev mailing list
> > d...@openvswitch.org
> >
https://urldefense.proofpoint.com/v2/url?u=https-3A__mail.openvswitch.org_mailman_listinfo_ovs-2Ddev&d=DwICAg&c=uilaK90D4TOVoH58JNXRgQ&r=BVhFA09CGX7JQ5Ih-uZnsw&m=C2QYD5JosF5j-X3I7nmikMajbKHF9WqhgGQZdigkYP8&s=DzvbErqrp1sqYj50u5y8oMwBgVZVFjUdhRitCukx98c&e=
> >
> >
>
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
