Re: [ovs-dev] [PATCH 20/20] Documentation: Update NEWS and faq
> On Jan 31, 2018, at 7:55 PM, Pravin Shelar wrote: > > On Wed, Jan 31, 2018 at 10:50 AM, Justin Pettit wrote: >> >> Pravin, thank you for reviewing the patches. When you're going though them, >> can you make a decision whether you think they're safe enough to pull into >> 2.9? >> > Sure. > Some of patches can be applied to 2.9. I will comment on it individually. Great. Feel free to just backport them directly unless you need some help from me. (Which would be surprising. :-) ) --Justin ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v4] openvswitch: Remove padding from packet before L3+ conntrack processing
On Wed, Jan 31, 2018 at 6:48 PM, Ed Swierk wrote: > IPv4 and IPv6 packets may arrive with lower-layer padding that is not > included in the L3 length. For example, a short IPv4 packet may have > up to 6 bytes of padding following the IP payload when received on an > Ethernet device with a minimum packet length of 64 bytes. > > Higher-layer processing functions in netfilter (e.g. nf_ip_checksum(), > and help() in nf_conntrack_ftp) assume skb->len reflects the length of > the L3 header and payload, rather than referring back to > ip_hdr->tot_len or ipv6_hdr->payload_len, and get confused by > lower-layer padding. > > In the normal IPv4 receive path, ip_rcv() trims the packet to > ip_hdr->tot_len before invoking netfilter hooks. In the IPv6 receive > path, ip6_rcv() does the same using ipv6_hdr->payload_len. Similarly > in the br_netfilter receive path, br_validate_ipv4() and > br_validate_ipv6() trim the packet to the L3 length before invoking > netfilter hooks. > > Currently in the OVS conntrack receive path, ovs_ct_execute() pulls > the skb to the L3 header but does not trim it to the L3 length before > calling nf_conntrack_in(NF_INET_PRE_ROUTING). When > nf_conntrack_proto_tcp encounters a packet with lower-layer padding, > nf_ip_checksum() fails causing a "nf_ct_tcp: bad TCP checksum" log > message. While extra zero bytes don't affect the checksum, the length > in the IP pseudoheader does. That length is based on skb->len, and > without trimming, it doesn't match the length the sender used when > computing the checksum. > > In ovs_ct_execute(), trim the skb to the L3 length before higher-layer > processing. > > Signed-off-by: Ed Swierk Acked-by: Pravin B Shelar ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH 20/20] Documentation: Update NEWS and faq
On Wed, Jan 31, 2018 at 10:50 AM, Justin Pettit wrote: > >> On Jan 30, 2018, at 3:49 PM, Gregory Rose wrote: >> >> On 1/30/2018 3:43 PM, Justin Pettit wrote: >>> On Jan 30, 2018, at 3:40 PM, Greg Rose wrote: Signed-off-by: Greg Rose --- Documentation/faq/releases.rst | 1 + NEWS | 2 ++ 2 files changed, 3 insertions(+) diff --git a/Documentation/faq/releases.rst b/Documentation/faq/releases.rst index 62a1957..2f03c89 100644 --- a/Documentation/faq/releases.rst +++ b/Documentation/faq/releases.rst @@ -67,6 +67,7 @@ Q: What Linux kernel versions does each Open vSwitch release work with? 2.7.x3.10 to 4.9 2.8.x3.10 to 4.12 2.9.x3.10 to 4.13 +2.10.x 3.10 to 4.14 >>> Thanks for the patches, Greg. Should we try to get these into 2.9 or just >>> wait for 2.10? >>> >>> --Justin >>> >>> >> >> I assumed 2.10 but... well, I'm not sure. The major features new in the >> Linux kernel since 4.13 are meters, NSH and erspan. Not sure if we need >> those for 2.9 or not. > > Thanks, Greg. > > Pravin, thank you for reviewing the patches. When you're going though them, > can you make a decision whether you think they're safe enough to pull into > 2.9? > Sure. Some of patches can be applied to 2.9. I will comment on it individually. Thanks, Pravin. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v7 0/6] OVS-DPDK flow offload with rte_flow
On Mon, Jan 29, 2018 at 02:59:42PM +0800, Yuanhan Liu wrote:
> Hi,
>
> Here is a joint work from Mellanox and Napatech, to enable the flow hw
> offload with the DPDK generic flow interface (rte_flow).
>
> The basic idea is to associate the flow with a mark id (a unit32_t number).
> Later, we then get the flow directly from the mark id, which could bypass
> some heavy CPU operations, including but not limiting to mini flow extract,
> emc lookup, dpcls lookup, etc.
>
> The association is done with CMAP in patch 1. The CPU workload bypassing
> is done in patch 2. The flow offload is done in patch 3, which mainly does
> two things:
>
> - translate the ovs match to DPDK rte flow patterns
> - bind those patterns with a RSS + MARK action.
>
> Patch 5 makes the offload work happen in another thread, for leaving the
> datapath as light as possible.
>
> A PHY-PHY forwarding with 1000 mega flows (udp,tp_src=1000-1999) and 1
> million streams (tp_src=1000-1999, tp_dst=2000-2999) show more than 260%
> performance boost.
>
> Note that it's disabled by default, which can be enabled by:
Hi,
First of all, thanks for working on this feature.
I have some general comments I'd like to discuss before going deeper
on it.
The documentation is too simple. It should mention the HW requirements
in order to use this feature. Also some important limitations, like no
support for IP frags, MPLS or conntrack, for instance.
It seems it would be possible to leave the HW offloading code outside
of dpif-netdev.c which is quite long already. I hope it will improve
isolation and code clarity too.
So far there is no synchronization between PMDs in the fast path.
However, we got a new mutex to sync PMDs and a new thread to manage.
Even without the patch adding the thread, there would be a new mutex
in the fast path. It seems the slow path today causes issues, so maybe
the whole upcall processing could be pushed to another thread. I
realize this is outside of the scope of this patchset, but it is
something we should consider.
As an alternative solution, maybe we could use a DPDK ring to have a
lockless way to push flows to the auxiliary thread.
There are some memory allocations and deallocations in the fast path
using OVS functions. Perhaps it is better to use rte_* functions
instead (another reason to split the code out of dpif-netdev.c)
I am curious to know why there is no flow dump or flush?
The function to help debugging (dump_flow_pattern) should use an
initial condition to return asap if debug is not enabled.
E.g.:
if (VLOG_DROP_DBG(rl)) {
return;
}
I am still wrapping my head around the RSS+MARK action and rte_flow
API, so I can't really comment those yet.
Thanks!
fbl
>
> $ ovs-vsctl set Open_vSwitch . other_config:hw-offload=true
>
> v7: - fixed wrong hash for mark_to_flow that has been refactored in v6
> - set the rss_conf for rss action to NULL, to workaround a mlx5 change
> in DPDK v17.11. Note that it will obey the rss settings OVS-DPDK has
> set in the beginning. Thus, nothing should be effected.
>
> v6: - fixed a sparse warning
> - added documentation
> - used hash_int to compute mark to flow hash
> - added more comments
> - added lock for pot lookup
> - rebased on top of the latest code
>
> v5: - fixed an issue that it took too long if we do flow add/remove
> repeatedly.
> - removed an unused mutex lock
> - turned most of the log level to DBG
> - rebased on top of the latest code
>
> v4: - use RSS action instead of QUEUE action with MARK
> - make it work with multiple queue (see patch 1)
> - rebased on top of latest code
>
> v3: - The mark and id association is done with array instead of CMAP.
> - Added a thread to do hw offload operations
> - Removed macros completely
> - dropped the patch to set FDIR_CONF, which is a workround some
> Intel NICs.
> - Added a debug patch to show all flow patterns we have created.
> - Misc fixes
>
> v2: - workaround the queue action issue
> - fixed the tcp_flags being skipped issue, which also fixed the
> build warnings
> - fixed l2 patterns for Intel nic
> - Converted some macros to functions
> - did not hardcode the max number of flow/action
> - rebased on top of the latest code
>
> Thanks.
>
> --yliu
>
> ---
> Finn Christensen (1):
> netdev-dpdk: implement flow offload with rte flow
>
> Yuanhan Liu (5):
> dpif-netdev: associate flow with a mark id
> dpif-netdev: retrieve flow directly from the flow mark
> netdev-dpdk: add debug for rte flow patterns
> dpif-netdev: do hw flow offload in a thread
> Documentation: document ovs-dpdk flow offload
>
> Documentation/howto/dpdk.rst | 17 +
> NEWS | 1 +
> lib/dp-packet.h | 13 +
> lib/dpif-netdev.c| 495 -
> lib/flow.c | 155 +++--
> lib/flow.h | 1
Re: [ovs-dev] [PATCH] learn: improve test case
On Wed, Jan 31, 2018 at 6:32 PM, William Tu wrote: > Current learn test cases use only ovs-ofctl add/del flows. > The patch add a new test case for learn with delete_learned and > limit option enabled. > > Signed-off-by: William Tu > --- > tests/learn.at | 38 ++ > 1 file changed, 38 insertions(+) > > diff --git a/tests/learn.at b/tests/learn.at > index 07ad043212ed..406a827aae36 100644 > --- a/tests/learn.at > +++ b/tests/learn.at > @@ -625,6 +625,44 @@ AT_CHECK([ovs-ofctl dump-flows br0 --no-stats | sort]) > OVS_VSWITCHD_STOP > AT_CLEANUP > > +AT_SETUP([learning action - delete_learned/limit with packet]) > Thanks William. I have question - is this test case trying to delineate some related bug or observed special behavior ? I am asking because I don't immediately see the intersection b/w the delete_learned and limit options ? BTW, what is "limit with packet" as opposed to just "limit" > +OVS_VSWITCHD_START( > +[add-port br0 p1 -- set Interface p1 type=dummy ofport_request=1 --\ > + add-port br0 p2 -- set Interface p2 type=dummy ofport_request=2]) > + > +# Add some initial flows and check that it was successful. > +AT_DATA([flows.txt], [dnl > +table=0 actions=set_field:0x2->reg7,set_field:0xabcdef01->metadata, > resubmit(,1) > +table=1 actions=learn(table=10,delete_learned,cookie=0x123,limit=3, > result_dst=NXM_NX_REG6[[0]],NXM_OF_ETH_DST[[]]=NXM_OF_ETH_ > SRC[[]],OXM_OF_METADATA[[]],output:NXM_NX_REG7) > +]) > + > +AT_CHECK([ovs-ofctl add-flows br0 flows.txt]) > +AT_CHECK([ovs-ofctl dump-flows br0 --no-stats | sort], [0], [dnl > + actions=load:0x2->NXM_NX_REG7[[]],load:0xabcdef01->OXM_OF_ > METADATA[[]],resubmit(,1) > + table=1, actions=learn(table=10,delete_learned,cookie=0x123,limit=3, > result_dst=NXM_NX_REG6[[0]],NXM_OF_ETH_DST[[]]=NXM_OF_ETH_ > SRC[[]],OXM_OF_METADATA[[]],output:NXM_NX_REG7[[]]) > +]) > + > +dnl Each packet will generate its own flow at table=10, except last one > +AT_CHECK([ovs-appctl netdev-dummy/receive p1 'in_port(1),eth(src=50:54:00: > 00:00:01,dst=50:54:00:00:00:ff),eth_type(0x1234)']) > +AT_CHECK([ovs-appctl netdev-dummy/receive p1 'in_port(1),eth(src=50:54:00: > 00:00:02,dst=50:54:00:00:00:ff),eth_type(0x1234)']) > +AT_CHECK([ovs-appctl netdev-dummy/receive p1 'in_port(1),eth(src=50:54:00: > 00:00:03,dst=50:54:00:00:00:ff),eth_type(0x1234)']) > +AT_CHECK([ovs-appctl netdev-dummy/receive p1 'in_port(1),eth(src=50:54:00: > 00:00:04,dst=50:54:00:00:00:ff),eth_type(0x1234)']) > + > +AT_CHECK([ovs-ofctl dump-flows br0 table=10 --no-stats | sort], [0], [dnl > + cookie=0x123, table=10, metadata=0xabcdef01,dl_dst=50:54:00:00:00:01 > actions=output:2 > + cookie=0x123, table=10, metadata=0xabcdef01,dl_dst=50:54:00:00:00:02 > actions=output:2 > + cookie=0x123, table=10, metadata=0xabcdef01,dl_dst=50:54:00:00:00:03 > actions=output:2 > +]) > + > +ovs-appctl revalidator/wait > + > +AT_CHECK([ovs-ofctl del-flows br0 'table=1']) > +AT_CHECK([ovs-ofctl dump-flows br0 table=10 --no-stats | sort], [0], [dnl > It looks odd to sort an expected empty list. > +]) > + > +OVS_VSWITCHD_STOP > +AT_CLEANUP > + > AT_SETUP([learning action - limit]) > OVS_VSWITCHD_START > AT_CHECK([ovs-appctl vlog/set dpif:dbg dpif_netdev:dbg]) > -- > 2.7.4 > > ___ > 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 v1 1/2] datapath-windows: Allow compiling all targets using SDK 10.0
Previously, Win8/8.1 targets would use SDK8.1. However, its
recommended to use the newer SDK as newer VS versions typically
drop support for older SDKs later on. This patch adds support
to compile all targets (Win8/8.1/10) using the 10.0 SDK.
Note that his patch does not drop support for older SDKs.
Signed-off-by: Shashank Ram
---
datapath-windows/Package/package.VcxProj | 28 --
datapath-windows/ovsext/ovsext.vcxproj | 50 +++-
2 files changed, 49 insertions(+), 29 deletions(-)
diff --git a/datapath-windows/Package/package.VcxProj
b/datapath-windows/Package/package.VcxProj
index 47cadcd..de747ee 100644
--- a/datapath-windows/Package/package.VcxProj
+++ b/datapath-windows/Package/package.VcxProj
@@ -42,6 +42,8 @@
$(VCTargetsPath11)
+10.0
+8.1
@@ -52,46 +54,54 @@
WindowsV6.3
true
-WindowsKernelModeDriver8.1
+8.1
+
WindowsKernelModeDriver$(PlatformToolsetVer)
WindowsV6.3
true
-WindowsKernelModeDriver8.1
+8.1
+
WindowsKernelModeDriver$(PlatformToolsetVer)
true
-WindowsKernelModeDriver10.0
+10.0
+
WindowsKernelModeDriver$(PlatformToolsetVer)
Desktop
Windows8
true
-WindowsKernelModeDriver8.1
+8.1
+
WindowsKernelModeDriver$(PlatformToolsetVer)
Windows8
true
-WindowsKernelModeDriver8.1
+8.1
+
WindowsKernelModeDriver$(PlatformToolsetVer)
false
-WindowsKernelModeDriver10.0
+10.0
+
WindowsKernelModeDriver$(PlatformToolsetVer)
Universal
WindowsV6.3
false
-WindowsKernelModeDriver8.1
+8.1
+
WindowsKernelModeDriver$(PlatformToolsetVer)
Windows8
false
-WindowsKernelModeDriver8.1
+8.1
+
WindowsKernelModeDriver$(PlatformToolsetVer)
@@ -175,4 +185,4 @@
-
\ No newline at end of file
+
diff --git a/datapath-windows/ovsext/ovsext.vcxproj
b/datapath-windows/ovsext/ovsext.vcxproj
index 48055b9..0509b76 100644
--- a/datapath-windows/ovsext/ovsext.vcxproj
+++ b/datapath-windows/ovsext/ovsext.vcxproj
@@ -43,6 +43,8 @@
Win8 Debug
Win32
{0D37F250-E766-44C7-90B4-D7E07E77D1AA}
+10.0
+8.1
@@ -52,46 +54,54 @@
WindowsV6.3
True
-WindowsKernelModeDriver8.1
+8.1
+
WindowsKernelModeDriver$(PlatformToolsetVer)
WindowsV6.3
True
-WindowsKernelModeDriver8.1
+8.1
+
WindowsKernelModeDriver$(PlatformToolsetVer)
True
-WindowsKernelModeDriver10.0
+10.0
+
WindowsKernelModeDriver$(PlatformToolsetVer)
Desktop
-Win8
+Windows8
True
-WindowsKernelModeDriver8.1
+8.1
+
WindowsKernelModeDriver$(PlatformToolsetVer)
-Win8
+Windows8
True
-WindowsKernelModeDriver8.1
+8.1
+
WindowsKernelModeDriver$(PlatformToolsetVer)
WindowsV6.3
False
-WindowsKernelModeDriver8.1
+8.1
+
WindowsKernelModeDriver$(PlatformToolsetVer)
False
-WindowsKernelModeDriver10.0
+10.0
+
WindowsKernelModeDriver$(PlatformToolsetVer)
Desktop
-Win8
+Windows8
False
-WindowsKernelModeDriver8.1
+8.1
+
WindowsKernelModeDriver$(PlatformToolsetVer)
@@ -144,7 +154,7 @@
-
+
@@ -275,14 +285,14 @@
Level4
- $(IntDir);%(AdditionalIncludeDirectories);..\..
- $(IntDir);%(AdditionalIncludeDirectories);..\..
- $(IntDir);%(AdditionalIncludeDirectories);..\..
- $(IntDir);%(AdditionalIncludeDirectories);..\..
- $(IntDir);%(AdditionalIncludeDirectories);..\..;$(MSBuildProjectDirectory)
- $(IntDir);%(AdditionalIncludeDirectories);..\..
- $(IntDir);%(AdditionalIncludeDirectories);..\..
- $(IntDir);%(AdditionalIncludeDirectories);..\..;$(MSBuildProjectDirectory)
+ .;$(IntDir);%(AdditionalIncludeDirectories);..\..
+ .;$(IntDir);%(AdditionalIncludeDirectories);..\..
+ .;$(IntDir);%(AdditionalIncludeDirectories);..\..
+ .;$(IntDir);%(AdditionalIncludeDirectories);..\..
+ .;$(IntDir);%(AdditionalIncludeDirectories);..\..;$(MSBuildProjectDirectory)
+ .;$(IntDir);%(AdditionalIncludeDirectories);..\..
+ .;$(IntDir);%(AdditionalIncludeDirectories);..\..
+ .;$(IntDir);%(AdditionalIncludeDirectories);..\..;$(MSBuildProjectDirectory)
true
true
true
--
2.9.3.windows.2
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH v1 2/2] datapath-windows: Specify platform arch during compilation
Newer compilers expect the platorm architecture to be passed. Signed-off-by: Shashank Ram --- Makefile.am | 9 + 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/Makefile.am b/Makefile.am index ed4b7fd..5988c02 100644 --- a/Makefile.am +++ b/Makefile.am @@ -409,14 +409,15 @@ CLEANFILES += manpage-dep-check if VSTUDIO_DDK ALL_LOCAL += ovsext +ARCH = x64 ovsext: datapath-windows/ovsext.sln $(srcdir)/datapath-windows/include/OvsDpInterface.h - MSBuild.exe //nologo //maxcpucount datapath-windows/ovsext.sln /target:Build /property:Configuration="Win8$(VSTUDIO_CONFIG)" /property:Version="$(PACKAGE_VERSION)" - MSBuild.exe //nologo //maxcpucount datapath-windows/ovsext.sln /target:Build /property:Configuration="Win8.1$(VSTUDIO_CONFIG)" /property:Version="$(PACKAGE_VERSION)" + MSBuild.exe //nologo //maxcpucount datapath-windows/ovsext.sln /target:Build /property:Configuration="Win8$(VSTUDIO_CONFIG)" /property:Version="$(PACKAGE_VERSION)" //p:Platform=$(ARCH) + MSBuild.exe //nologo //maxcpucount datapath-windows/ovsext.sln /target:Build /property:Configuration="Win8.1$(VSTUDIO_CONFIG)" /property:Version="$(PACKAGE_VERSION)" //p:Platform=$(ARCH) CLEAN_LOCAL += ovsext_clean ovsext_clean: datapath-windows/ovsext.sln - MSBuild.exe //nologo //maxcpucount datapath-windows/ovsext.sln /target:Clean /property:Configuration="Win8$(VSTUDIO_CONFIG)" /property:Version="$(PACKAGE_VERSION)" - MSBuild.exe //nologo //maxcpucount datapath-windows/ovsext.sln /target:Clean /property:Configuration="Win8.1$(VSTUDIO_CONFIG)" /property:Version="$(PACKAGE_VERSION)" + MSBuild.exe //nologo //maxcpucount datapath-windows/ovsext.sln /target:Clean /property:Configuration="Win8$(VSTUDIO_CONFIG)" /property:Version="$(PACKAGE_VERSION)" //p:Platform=$(ARCH) + MSBuild.exe //nologo //maxcpucount datapath-windows/ovsext.sln /target:Clean /property:Configuration="Win8.1$(VSTUDIO_CONFIG)" /property:Version="$(PACKAGE_VERSION)" //p:Platform=$(ARCH) endif .PHONY: ovsext -- 2.9.3.windows.2 ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH v4] openvswitch: Remove padding from packet before L3+ conntrack processing
IPv4 and IPv6 packets may arrive with lower-layer padding that is not
included in the L3 length. For example, a short IPv4 packet may have
up to 6 bytes of padding following the IP payload when received on an
Ethernet device with a minimum packet length of 64 bytes.
Higher-layer processing functions in netfilter (e.g. nf_ip_checksum(),
and help() in nf_conntrack_ftp) assume skb->len reflects the length of
the L3 header and payload, rather than referring back to
ip_hdr->tot_len or ipv6_hdr->payload_len, and get confused by
lower-layer padding.
In the normal IPv4 receive path, ip_rcv() trims the packet to
ip_hdr->tot_len before invoking netfilter hooks. In the IPv6 receive
path, ip6_rcv() does the same using ipv6_hdr->payload_len. Similarly
in the br_netfilter receive path, br_validate_ipv4() and
br_validate_ipv6() trim the packet to the L3 length before invoking
netfilter hooks.
Currently in the OVS conntrack receive path, ovs_ct_execute() pulls
the skb to the L3 header but does not trim it to the L3 length before
calling nf_conntrack_in(NF_INET_PRE_ROUTING). When
nf_conntrack_proto_tcp encounters a packet with lower-layer padding,
nf_ip_checksum() fails causing a "nf_ct_tcp: bad TCP checksum" log
message. While extra zero bytes don't affect the checksum, the length
in the IP pseudoheader does. That length is based on skb->len, and
without trimming, it doesn't match the length the sender used when
computing the checksum.
In ovs_ct_execute(), trim the skb to the L3 length before higher-layer
processing.
Signed-off-by: Ed Swierk
---
net/openvswitch/conntrack.c | 34 ++
1 file changed, 34 insertions(+)
diff --git a/net/openvswitch/conntrack.c b/net/openvswitch/conntrack.c
index d558e88..285f879 100644
--- a/net/openvswitch/conntrack.c
+++ b/net/openvswitch/conntrack.c
@@ -1097,6 +1097,36 @@ static int ovs_ct_commit(struct net *net, struct
sw_flow_key *key,
return 0;
}
+/* Trim the skb to the length specified by the IP/IPv6 header,
+ * removing any trailing lower-layer padding. This prepares the skb
+ * for higher-layer processing that assumes skb->len excludes padding
+ * (such as nf_ip_checksum). The caller needs to pull the skb to the
+ * network header, and ensure ip_hdr/ipv6_hdr points to valid data.
+ */
+static int ovs_skb_network_trim(struct sk_buff *skb)
+{
+ unsigned int len;
+ int err;
+
+ switch (skb->protocol) {
+ case htons(ETH_P_IP):
+ len = ntohs(ip_hdr(skb)->tot_len);
+ break;
+ case htons(ETH_P_IPV6):
+ len = sizeof(struct ipv6hdr)
+ + ntohs(ipv6_hdr(skb)->payload_len);
+ break;
+ default:
+ len = skb->len;
+ }
+
+ err = pskb_trim_rcsum(skb, len);
+ if (err)
+ kfree_skb(skb);
+
+ return err;
+}
+
/* Returns 0 on success, -EINPROGRESS if 'skb' is stolen, or other nonzero
* value if 'skb' is freed.
*/
@@ -,6 +1141,10 @@ int ovs_ct_execute(struct net *net, struct sk_buff *skb,
nh_ofs = skb_network_offset(skb);
skb_pull_rcsum(skb, nh_ofs);
+ err = ovs_skb_network_trim(skb);
+ if (err)
+ return err;
+
if (key->ip.frag != OVS_FRAG_TYPE_NONE) {
err = handle_fragments(net, key, info->zone.id, skb);
if (err)
--
1.9.1
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH] learn: improve test case
Current learn test cases use only ovs-ofctl add/del flows. The patch add a new test case for learn with delete_learned and limit option enabled. Signed-off-by: William Tu --- tests/learn.at | 38 ++ 1 file changed, 38 insertions(+) diff --git a/tests/learn.at b/tests/learn.at index 07ad043212ed..406a827aae36 100644 --- a/tests/learn.at +++ b/tests/learn.at @@ -625,6 +625,44 @@ AT_CHECK([ovs-ofctl dump-flows br0 --no-stats | sort]) OVS_VSWITCHD_STOP AT_CLEANUP +AT_SETUP([learning action - delete_learned/limit with packet]) +OVS_VSWITCHD_START( +[add-port br0 p1 -- set Interface p1 type=dummy ofport_request=1 --\ + add-port br0 p2 -- set Interface p2 type=dummy ofport_request=2]) + +# Add some initial flows and check that it was successful. +AT_DATA([flows.txt], [dnl +table=0 actions=set_field:0x2->reg7,set_field:0xabcdef01->metadata, resubmit(,1) +table=1 actions=learn(table=10,delete_learned,cookie=0x123,limit=3,result_dst=NXM_NX_REG6[[0]],NXM_OF_ETH_DST[[]]=NXM_OF_ETH_SRC[[]],OXM_OF_METADATA[[]],output:NXM_NX_REG7) +]) + +AT_CHECK([ovs-ofctl add-flows br0 flows.txt]) +AT_CHECK([ovs-ofctl dump-flows br0 --no-stats | sort], [0], [dnl + actions=load:0x2->NXM_NX_REG7[[]],load:0xabcdef01->OXM_OF_METADATA[[]],resubmit(,1) + table=1, actions=learn(table=10,delete_learned,cookie=0x123,limit=3,result_dst=NXM_NX_REG6[[0]],NXM_OF_ETH_DST[[]]=NXM_OF_ETH_SRC[[]],OXM_OF_METADATA[[]],output:NXM_NX_REG7[[]]) +]) + +dnl Each packet will generate its own flow at table=10, except last one +AT_CHECK([ovs-appctl netdev-dummy/receive p1 'in_port(1),eth(src=50:54:00:00:00:01,dst=50:54:00:00:00:ff),eth_type(0x1234)']) +AT_CHECK([ovs-appctl netdev-dummy/receive p1 'in_port(1),eth(src=50:54:00:00:00:02,dst=50:54:00:00:00:ff),eth_type(0x1234)']) +AT_CHECK([ovs-appctl netdev-dummy/receive p1 'in_port(1),eth(src=50:54:00:00:00:03,dst=50:54:00:00:00:ff),eth_type(0x1234)']) +AT_CHECK([ovs-appctl netdev-dummy/receive p1 'in_port(1),eth(src=50:54:00:00:00:04,dst=50:54:00:00:00:ff),eth_type(0x1234)']) + +AT_CHECK([ovs-ofctl dump-flows br0 table=10 --no-stats | sort], [0], [dnl + cookie=0x123, table=10, metadata=0xabcdef01,dl_dst=50:54:00:00:00:01 actions=output:2 + cookie=0x123, table=10, metadata=0xabcdef01,dl_dst=50:54:00:00:00:02 actions=output:2 + cookie=0x123, table=10, metadata=0xabcdef01,dl_dst=50:54:00:00:00:03 actions=output:2 +]) + +ovs-appctl revalidator/wait + +AT_CHECK([ovs-ofctl del-flows br0 'table=1']) +AT_CHECK([ovs-ofctl dump-flows br0 table=10 --no-stats | sort], [0], [dnl +]) + +OVS_VSWITCHD_STOP +AT_CLEANUP + AT_SETUP([learning action - limit]) OVS_VSWITCHD_START AT_CHECK([ovs-appctl vlog/set dpif:dbg dpif_netdev:dbg]) -- 2.7.4 ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v1 0/5] datapath: enable NSH support in kernel compat mode
It still failed. Does "encap ip" require any special kernel module? vagrant@gbpsfc4:~/ovs-nsh-test$ export PATH=/home/vagrant/iproute2-4.9.0/ip:$PATH vagrant@gbpsfc4:~/ovs-nsh-test$ sudo -E ip link add dev at_vxlan1 type vxlan dstport 4790 external gpe vagrant@gbpsfc4:~/ovs-nsh-test$ sudo -E ip addr add dev at_vxlan1 10.1.1.1/24 vagrant@gbpsfc4:~/ovs-nsh-test$ sudo -E ip link set dev at_vxlan1 mtu 1450 up vagrant@gbpsfc4:~/ovs-nsh-test$ sudo -E ip route add 10.1.1.2/32 encap ip id 0 dst 172.31.1.100 dev at_vxlan1 RTNETLINK answers: Operation not supported vagrant@gbpsfc4:~/ovs-nsh-test$ sudo -E ip route add 10.1.1.2/32 encap ip id 1 dst 172.31.1.100 dev at_vxlan1 RTNETLINK answers: Operation not supported vagrant@gbpsfc4:~/ovs-nsh-test$ sudo -E ip route add 10.1.1.2/32 encap ip dst 172.31.1.100 dev at_vxlan1 RTNETLINK answers: Operation not supported vagrant@gbpsfc4:~/ovs-nsh-test$ lsmod Module Size Used by vxlan 53248 0 ip6_udp_tunnel 16384 1 vxlan udp_tunnel 16384 1 vxlan ip6_tunnel 32768 0 tunnel616384 1 ip6_tunnel ipip 16384 0 tunnel416384 1 ipip ip_tunnel 24576 1 ipip veth 16384 0 nf_conntrack_ipv6 20480 0 nf_defrag_ipv6 36864 1 nf_conntrack_ipv6 xt_addrtype16384 2 xt_conntrack 16384 1 ipt_MASQUERADE 16384 1 nf_nat_masquerade_ipv416384 1 ipt_MASQUERADE iptable_nat16384 1 dm_crypt 36864 0 nf_conntrack_ipv4 16384 3 nf_defrag_ipv4 16384 1 nf_conntrack_ipv4 nf_nat_ipv416384 1 iptable_nat nf_nat 28672 2 nf_nat_masquerade_ipv4,nf_nat_ipv4 nf_conntrack 131072 7 nf_conntrack_ipv6,nf_conntrack_ipv4,ipt_MASQUERADE,nf_nat_masquerade_ipv4,xt_conntrack,nf_nat_ipv4,nf_nat bridge143360 0 stp16384 1 bridge llc16384 2 bridge,stp dm_thin_pool 65536 1 dm_persistent_data 69632 1 dm_thin_pool dm_bio_prison 20480 1 dm_thin_pool dm_bufio 28672 1 dm_persistent_data libcrc32c 16384 3 nf_conntrack,dm_persistent_data,nf_nat nfsd 299008 2 iptable_filter 16384 1 ip_tables 24576 2 iptable_filter,iptable_nat x_tables 36864 5 ip_tables,iptable_filter,ipt_MASQUERADE,xt_addrtype,xt_conntrack auth_rpcgss57344 1 nfsd nfs_acl16384 1 nfsd nfs 241664 0 lockd 90112 2 nfsd,nfs grace 16384 2 nfsd,lockd sunrpc327680 6 auth_rpcgss,nfsd,nfs_acl,lockd,nfs fscache65536 1 nfs psmouse 139264 0 e1000 139264 0 ppdev 20480 0 parport_pc 32768 0 i2c_piix4 24576 0 mac_hid16384 0 sb_edac24576 0 serio_raw 16384 0 lp 20480 0 parport49152 3 lp,parport_pc,ppdev pata_acpi 16384 0 video 40960 0 vagrant@gbpsfc4:~/ovs-nsh-test$ -Original Message- From: Eric Garver [mailto:e...@erig.me] Sent: Thursday, February 1, 2018 5:10 AM To: Yang, Yi Y Cc: d...@openvswitch.org Subject: Re: [ovs-dev] [PATCH v1 0/5] datapath: enable NSH support in kernel compat mode On Wed, Jan 31, 2018 at 02:12:14PM +, Yang, Yi Y wrote: > Hi, Eric > > My kernel is 4.13.0-rc6, so I'm very sure it can support vxlan-gpe, I > can add vxlan-gpe port without any issue by command line, so I don't > know what happened. Your output below indicates this is failing on this line: 25 NS_CHECK_EXEC([at_ns0], [ip route add 10.1.1.2/32 encap ip id 0 dst 172.31.1.100 dev at_vxlan1]) It does not appear to be an OVS issue. Maybe encap id 0 is not accepted on older kernels. Try changing it to a non-zero value. > > Greg, I checked unit tests in tests/system-traffic.at and > tests/system-layer3-tunnels.at for vxlan and vxlan-gpe, I think they are very > tricky, for NSH, they won't work, current test infrastructure can't handle > NSH unit test in kernel data path, so I don't think I can add it. I have > posted v2 patch series, I have sfc test environment in my machine and have > verified kernel data path, my test environment has two vagrant VMs, each one > VM starts two containers which play SF roles, end-to-end ping and http are > both ok. > > -Original Message- > From: Eric Garver [mailto:e...@erig.me] > Sent: Tuesday, January 30, 2018 9:53 PM > To: Yang, Yi Y > Cc: Gregory Rose ; d...@openvswitch.org > Subject: Re: [ovs-dev] [PATCH v1 0/5] datapath: enable NSH support in > kernel compat mode > > On Tue, Jan 30, 2018 at 11:33:55AM +, Yang, Yi Y wrote: > > Hi, Greg > > > > I installed linux 3.10.107 in Ubuntu 14.04 and fixed > > skb_gso_error_unwind issue, but for unit test, > > tests/system-l
Re: [ovs-dev] [PATCH 10/15] ovsdb-server: Add new RPC "set_db_change_aware".
> On Dec 31, 2017, at 9:16 PM, Ben Pfaff wrote:
>
> + Traditionally, ovsdb-server disconnects all of its clients
> + when a significant configuration change occurs, because this prompts a
> + well-written client to reassess what is available from the server when
> it
> + reconnects. Because this table provides an alternative and more
> + efficient way to find out about those changes, OVS 2.9 also introduces
> + the set_db_change_aware RPC, documented in
> + ovsdb-server(1), to allow clients to suppress this
> + disconnection behavior.
Is that in section 1 or 7 of the ovsdb-server man pages?
> + When a database is removed from the server, in addition to
> + Database table updates, the server sends
> cancel
> + messages, as described in RFC 7047 section 4.1.4, in reply to
> outstanding
> + transactions for the removed database. The server also cancels any
> + outstanding monitoring initiated by monitor or
> + monitor_cond requested on the removed database, sending
> the
> + monitor_canceled RPC described in
> + ovsdb-server(5). Only clients that disable disconnection
> + with set_db_change_aware receive these messages.
I believe this file is section 5 of the ovsdb-server man page. Should it also
be section 7?
> @@ -333,17 +331,20 @@ ovsdb_jsonrpc_server_free_remote_status(
> free(status->locks_lost);
> }
>
> -/* Forces all of the JSON-RPC sessions managed by 'svr' to disconnect and
> - * reconnect. */
> +/* Makes all of the JSON-RPC sessions managed by 'svr' to disconnect. (They
> + * will then generally reconnect.).
I think you should drop "to".
> @@ -432,6 +433,20 @@ struct ovsdb_jsonrpc_session {
> struct ovsdb_session up;
> struct ovsdb_jsonrpc_remote *remote;
>
> +/* RFC 7047 does not contemplate how to alert clients to changes to the
> set
> + * of databases, e.g. databases that are added or removed while the
> + * database server is running. Traditionally, ovsdb-server disconnects
> all
> + * of its clients when this happens; a well-written client will reassess
> + * what is available from the server upon reconnection.
> + *
> + * OVS 2.9 introduces a way for clients to monitor changes to the
> databases
> + * being served, through the Database table in the _Server database that
> + * OVSDB adds in this version. ovsdb-server suppresses the connection
> + * close for clients that identify themselves as taking advantage of this
> + * mechanism.
> + */
> +bool db_change_aware;
I think it might be worth being explicit that setting this flag to true
indicates that the client has requested this suppression to occur.
> +/* Database 'db' is about to be removed from the database server. To
> prepare,
> + * this function removes all references to 'db' from session 's'. */
> +static void
> +ovsdb_jsonrpc_session_preremove_db(struct ovsdb_jsonrpc_remote *remote,
> + struct ovsdb *db)
The argument looks to be 'db', not 's', which may contain more than one session.
> +/* Makes all of the JSON-RPC sessions managed by 'remove' to disconnect.
> (They
> + * will then generally reconnect.).
> + *
> + * If 'force' is true, disconnects all sessions. Otherwise, disconnects only
> + * sesions that aren't database change aware. */
> static void
> +ovsdb_jsonrpc_session_reconnect_all(struct ovsdb_jsonrpc_remote *remote,
> +bool force)
The comment should be 'remote' instead of 'remove'.
Acked-by: Justin Pettit
Thanks,
--Justin
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] Balanced Scorecard
Logre sus objetivos a través de una gestión estratégica Balanced Scorecard: Una herramienta para la estrategia empresarial 15 de Febrero- C.P. Hugo Coca Chávez - 9am- 8pm El Balanced Scorecard da una serie de resultados que favorecen la administración de su organización, a través de la implementación y aplicación de una metodología, con la que se obtienen ventajas como la formación de los empleados hacia la visión de la empresa, la comunicación hacia todo el personal de los objetivos y su cumplimiento, replanteamiento de la estrategia con base en resultados, mejoría en los indicadores financieros entre muchas otras. BENEFICIOS DE ASISTIR: - Elaborará los objetivos de su empresa y los unificará con la planeación estratégica. - Conocerá los objetivos y la forma de medir el desempeño. - Aprenderá lo que es el mapa estratégico, así como la forma en la que se construye. - Identificará los sistemas: gráfico de información gerencial, de control de gestión y de remuneración por resultados. ¿Requiere la información a la Brevedad? responda este email con la palabra: Balanced + 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] util: Document and rely on ovs_assert() always evaluating its argument.
Thanks, looks good to me.
Reviewed-by: Yifeng Sun
On Wed, Jan 31, 2018 at 11:23 AM, Ben Pfaff wrote:
> The ovs_assert() macro always evaluates its argument, even when NDEBUG is
> defined so that failure is ignored. This behavior wasn't documented, and
> thus a lot of code didn't rely on it. This commit documents the behavior
> and simplifies bits of code that heretofore didn't rely on it.
>
> Signed-off-by: Ben Pfaff
> ---
> include/openvswitch/util.h | 7 +--
> lib/cmap.c | 6 ++
> lib/conntrack.c | 6 ++
> lib/hmapx.c | 6 ++
> lib/odp-execute.c| 5 +
> lib/odp-util.c | 5 +
> lib/ofp-msgs.c | 32
> lib/ofp-util.c | 11 ---
> lib/ovsdb-data.c | 4 +---
> lib/shash.c | 3 +--
> lib/sset.c | 6 ++
> ofproto/ofproto-dpif-xlate.c | 7 +++
> ovsdb/ovsdb-server.c | 4 +---
> ovsdb/replication.c | 3 +--
> 14 files changed, 34 insertions(+), 71 deletions(-)
>
> diff --git a/include/openvswitch/util.h b/include/openvswitch/util.h
> index ad1b184f78d9..b4d49ee21804 100644
> --- a/include/openvswitch/util.h
> +++ b/include/openvswitch/util.h
> @@ -44,8 +44,11 @@ const char *ovs_get_program_version(void);
> : (X) <= UINT_MAX / (Y) ? (unsigned int) (X) * (unsigned int) (Y) \
> : UINT_MAX)
>
> -/* Like the standard assert macro, except writes the failure message to
> the
> - * log. */
> +/* Like the standard assert macro, except:
> + *
> + *- Writes the failure message to the log.
> + *
> + *- Always evaluates the condition, even with NDEBUG. */
> #ifndef NDEBUG
> #define ovs_assert(CONDITION) \
> (OVS_LIKELY(CONDITION) \
> diff --git a/lib/cmap.c b/lib/cmap.c
> index af29639b049c..07719a8fd286 100644
> --- a/lib/cmap.c
> +++ b/lib/cmap.c
> @@ -843,11 +843,9 @@ cmap_replace(struct cmap *cmap, struct cmap_node
> *old_node,
> struct cmap_impl *impl = cmap_get_impl(cmap);
> uint32_t h1 = rehash(impl, hash);
> uint32_t h2 = other_hash(h1);
> -bool ok;
>
> -ok = cmap_replace__(impl, old_node, new_node, hash, h1)
> -|| cmap_replace__(impl, old_node, new_node, hash, h2);
> -ovs_assert(ok);
> +ovs_assert(cmap_replace__(impl, old_node, new_node, hash, h1) ||
> + cmap_replace__(impl, old_node, new_node, hash, h2));
>
> if (!new_node) {
> impl->n--;
> diff --git a/lib/conntrack.c b/lib/conntrack.c
> index 562e767833d1..fe5fd0fe8be1 100644
> --- a/lib/conntrack.c
> +++ b/lib/conntrack.c
> @@ -3080,11 +3080,9 @@ repl_ftp_v6_addr(struct dp_packet *pkt, struct
> ct_addr v6_addr_rep,
> return 0;
> }
>
> -const char *rc;
> char v6_addr_str[IPV6_SCAN_LEN] = {0};
> -rc = inet_ntop(AF_INET6, &v6_addr_rep.ipv6_aligned, v6_addr_str,
> - IPV6_SCAN_LEN - 1);
> -ovs_assert(rc != NULL);
> +ovs_assert(inet_ntop(AF_INET6, &v6_addr_rep.ipv6_aligned, v6_addr_str,
> + IPV6_SCAN_LEN - 1));
>
> size_t replace_addr_size = strlen(v6_addr_str);
>
> diff --git a/lib/hmapx.c b/lib/hmapx.c
> index 0440767c9c97..eadfe640ac11 100644
> --- a/lib/hmapx.c
> +++ b/lib/hmapx.c
> @@ -116,8 +116,7 @@ hmapx_add(struct hmapx *map, void *data)
> void
> hmapx_add_assert(struct hmapx *map, void *data)
> {
> -bool added OVS_UNUSED = hmapx_add(map, data);
> -ovs_assert(added);
> +ovs_assert(hmapx_add(map, data));
> }
>
> /* Removes all of the nodes from 'map'. */
> @@ -156,8 +155,7 @@ hmapx_find_and_delete(struct hmapx *map, const void
> *data)
> void
> hmapx_find_and_delete_assert(struct hmapx *map, const void *data)
> {
> -bool deleted OVS_UNUSED = hmapx_find_and_delete(map, data);
> -ovs_assert(deleted);
> +ovs_assert(hmapx_find_and_delete(map, data));
> }
>
> /* Searches for 'data' in 'map'. Returns its node, if found, otherwise a
> null
> diff --git a/lib/odp-execute.c b/lib/odp-execute.c
> index 20f33eb57acb..12bba83ea32c 100644
> --- a/lib/odp-execute.c
> +++ b/lib/odp-execute.c
> @@ -198,10 +198,7 @@ odp_set_sctp(struct dp_packet *packet, const struct
> ovs_key_sctp *key,
> static void
> odp_set_tunnel_action(const struct nlattr *a, struct flow_tnl *tun_key)
> {
> -enum odp_key_fitness fitness;
> -
> -fitness = odp_tun_key_from_attr(a, tun_key);
> -ovs_assert(fitness != ODP_FIT_ERROR);
> +ovs_assert(odp_tun_key_from_attr(a, tun_key) != ODP_FIT_ERROR);
> }
>
> static void
> diff --git a/lib/odp-util.c b/lib/odp-util.c
> index 6a29a76de5cd..14d5af097ee4 100644
> --- a/lib/odp-util.c
> +++ b/lib/odp-util.c
> @@ -6847,8 +6847,6 @@ commit_mpls_action(const struct flow *flow, struct
> flow *base,
> /* Otherwise, if there more LSEs in base than are common
> between
> * base and
[ovs-dev] Redes sociales y la atracción de talento
Reclutamiento 3.0 Febrero 14 - webinar Interactivo Dirigido a Personal de RRHH, Departamento de reclutamiento y selección de las empresas y cualquier persona que intervenga en el proceso de reclutamiento, selección y contratación de personal. Temario: Panorama actual y antecedentes de la selección de talento digital. Baby boomers, generación X, millennials y generación Z. Las redes sociales y la atracción de talento. Herramientas digitales para entrevistar. Temario e Inscripciones: Respondiendo por este medio "Linkedin"+TELÉFONO + NOMBRE o marcando al: 045 + 5515546630 ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCHv2] ovs-router: fix router entry cast
The offsetof(struct ovs_router_entry, cr) should always be 0,
thus the else statement should never be reached.
Signed-off-by: William Tu
---
lib/ovs-router.c | 6 +-
1 file changed, 1 insertion(+), 5 deletions(-)
diff --git a/lib/ovs-router.c b/lib/ovs-router.c
index cd2ab15fb003..c68bb3bc9500 100644
--- a/lib/ovs-router.c
+++ b/lib/ovs-router.c
@@ -68,11 +68,7 @@ struct ovs_router_entry {
static struct ovs_router_entry *
ovs_router_entry_cast(const struct cls_rule *cr)
{
-if (offsetof(struct ovs_router_entry, cr) == 0) {
-return CONTAINER_OF(cr, struct ovs_router_entry, cr);
-} else {
-return cr ? CONTAINER_OF(cr, struct ovs_router_entry, cr) : NULL;
-}
+return cr ? CONTAINER_OF(cr, struct ovs_router_entry, cr) : NULL;
}
static bool
--
2.7.4
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v1 0/5] datapath: enable NSH support in kernel compat mode
On Wed, Jan 31, 2018 at 02:12:14PM +, Yang, Yi Y wrote:
> Hi, Eric
>
> My kernel is 4.13.0-rc6, so I'm very sure it can support vxlan-gpe, I
> can add vxlan-gpe port without any issue by command line, so I don't
> know what happened.
Your output below indicates this is failing on this line:
25 NS_CHECK_EXEC([at_ns0], [ip route add 10.1.1.2/32 encap ip id 0 dst
172.31.1.100 dev at_vxlan1])
It does not appear to be an OVS issue. Maybe encap id 0 is not accepted
on older kernels. Try changing it to a non-zero value.
>
> Greg, I checked unit tests in tests/system-traffic.at and
> tests/system-layer3-tunnels.at for vxlan and vxlan-gpe, I think they are very
> tricky, for NSH, they won't work, current test infrastructure can't handle
> NSH unit test in kernel data path, so I don't think I can add it. I have
> posted v2 patch series, I have sfc test environment in my machine and have
> verified kernel data path, my test environment has two vagrant VMs, each one
> VM starts two containers which play SF roles, end-to-end ping and http are
> both ok.
>
> -Original Message-
> From: Eric Garver [mailto:e...@erig.me]
> Sent: Tuesday, January 30, 2018 9:53 PM
> To: Yang, Yi Y
> Cc: Gregory Rose ; d...@openvswitch.org
> Subject: Re: [ovs-dev] [PATCH v1 0/5] datapath: enable NSH support in kernel
> compat mode
>
> On Tue, Jan 30, 2018 at 11:33:55AM +, Yang, Yi Y wrote:
> > Hi, Greg
> >
> > I installed linux 3.10.107 in Ubuntu 14.04 and fixed
> > skb_gso_error_unwind issue, but for unit test,
> > tests/system-layer3-tunnels.at is a good reference for it because we
> > used vxlan-gpe for nsh, I ran unit test 90, but it always fails (I
> > have installed and used net-next kernel and the latest iproute2)
> >
> > Here is error log
> >
> > ./system-layer3-tunnels.at:25: ip netns exec at_ns0 sh <<
> > NS_EXEC_HEREDOC ip route add 10.1.1.2/32 encap ip id 0 dst
> > 172.31.1.100 dev at_vxlan1 NS_EXEC_HEREDOC
> > --- /dev/null 2018-01-30 02:18:43.272647961 +
> > +++
> > /home/vagrant/ovs-nsh-test/tests/system-kmod-testsuite.dir/at-groups/90/stderr
> > 2018-01-30 09:45:15.415934534 +
> > @@ -0,0 +1 @@
> > +RTNETLINK answers: Operation not supported
> > ./system-layer3-tunnels.at:25: exit code was 2, expected 0
> >
> > I think my system missed something so “ip route add 10.1.1.2/32 encap
> > ip id 0 dst 172.31.1.100 dev at_vxlan1 “ failed, Eric, what linux
> > distribution do you know I can run “ping over VXLAN-GPE” successfully, I’ll
> > use it as baseline to add NSH unit test for kernel data path.
>
> When I added the tests it was on RHEL-7.4 with a net-next kernel. It should
> skip the tests if the installed iproute2 does not have the options for GPE.
>
> The error you're seeing likely means your kernel does not have GPE support.
> VXLAN-GPE first appeared in kernel 4.7.
>
> e1e5314de08b ("vxlan: implement GPE")
>
> As such, I think the VXLAN-GPE test case should pass on any distro with a
> 4.7+ kernel.
>
> >
> > From: Gregory Rose [mailto:gvrose8...@gmail.com]
> > Sent: Tuesday, January 30, 2018 1:51 AM
> > To: Yang, Yi Y ; d...@openvswitch.org
> > Cc: b...@ovn.org; jan.scheur...@ericsson.com
> > Subject: Re: [PATCH v1 0/5] datapath: enable NSH support in kernel
> > compat mode
> >
> > On 1/10/2018 11:53 PM, Yi Yang wrote:
> >
> >
> > This patch series is to backport NSH support patches in Linux net-next
> > tree
> >
> > to OVS in order that it can support NSH in kernel compat mode.
> >
> >
> >
> > Yi Yang (5):
> >
> > datapath: ether: add NSH ethertype
> >
> > datapath: vxlan: factor out VXLAN-GPE next protocol
> >
> > datapath: net: add NSH header structures and helpers
> >
> > datapath: nsh: add GSO support
> >
> > datapath: enable NSH support
> >
> >
> >
> > NEWS | 1 +
> >
> > datapath/Modules.mk | 4 +-
> >
> > datapath/actions.c| 116
> >
> > datapath/datapath.c | 4 +
> >
> > datapath/flow.c | 51
> >
> > datapath/flow.h | 7 +
> >
> > datapath/flow_netlink.c | 343
> > +-
> >
> > datapath/flow_netlink.h | 5 +
> >
> > datapath/linux/Modules.mk | 2 +
> >
> > datapath/linux/compat/include/linux/if_ether.h| 4 +
> >
> > datapath/linux/compat/include/linux/openvswitch.h | 6 +-
> >
> > datapath/linux/compat/include/net/nsh.h | 313
> >
> >
> > datapath/linux/compat/include/net/tun_proto.h | 49
> >
> > datapath/linux/compat/include/net/vxlan.h | 6 -
> >
> > datapath/linux/compat/vxlan.c | 32 +-
> >
> > datapath/nsh.c| 142 +
> >
> > 16 files changed, 1048 ins
Re: [ovs-dev] [PATCH 07/15] ovsdb-idl: Break out database-specific stuff into new data structure.
On Wed, Jan 24, 2018 at 05:44:05PM -0800, Justin Pettit wrote:
>
>
> > On Dec 31, 2017, at 9:16 PM, Ben Pfaff wrote:
> >
> > +struct ovsdb_idl {
> > +struct ovsdb_idl_db server;
> > +struct ovsdb_idl_db db; /* XXX rename 'data'? */
>
> Did you want to change this?
I did intend to change it. Done.
> > @@ -353,51 +379,54 @@ ovsdb_idl_set_remote(struct ovsdb_idl *idl, const
> > char *remote,
> > bool retry)
> > {
> > if (idl) {
> > -ovs_assert(!idl->txn);
> > -jsonrpc_session_close(idl->session);
> > idl->session = jsonrpc_session_open(remote, retry);
> > +/* XXX update condition */
> > idl->state_seqno = UINT_MAX;
>
> Does this "XXX" need to be addressed.
It doesn't look like it. I removed the comment.
> > +static void
> > +ovsdb_idl_db_destroy(struct ovsdb_idl_db *db)
> > +{
> > +ovs_assert(!db->txn);
> > +for (size_t i = 0; i < db->class_->n_tables; i++) {
> > +struct ovsdb_idl_table *table = &db->tables[i];
> > +ovsdb_idl_condition_destroy(&table->condition);
> > +ovsdb_idl_destroy_indexes(table);
> > +shash_destroy(&table->columns);
> > +hmap_destroy(&table->rows);
> > +free(table->modes);
> > +}
> > +shash_destroy(&db->table_by_name);
> > +free(db->tables);
> > +json_destroy(db->schema);
> > +hmap_destroy(&db->outstanding_txns);
> > +free(db->lock_name);
> > +json_destroy(db->lock_request_id);
> > +}
>
> Do you need call json_destroy() on "db->monitor_id"?
Yes, thanks, fixed.
> There's an assert on "db->txn", but not one checking that 'outstanding_txns'
> empty.
Hmm. I added a function to abort transactions in an ovsdb_idl_db (based
on ovsdb_idl_txn_abort_all()) and added a call to it here.
> > +static void
> > +ovsdb_idl_send_cond_change(struct ovsdb_idl *idl)
> > +{
> > +/* When 'idl->request_id' is not NULL, there is an outstanding
> > + * conditional monitoring update request that we have not heard
> > + * from the server yet. Don't generate another request in this case.
> > + *
> > + * XXX per-db request_id */
> > +if (!jsonrpc_session_is_connected(idl->session)
> > +|| idl->state != IDL_S_MONITORING_COND
> > +|| idl->request_id) {
> > +return;
> > +}
>
> Did you need to address this "XXX" comment?
I don't think anything is needed. Comment removed.
> > +/* Turns off OVSDB_IDL_ALERT for 'column' in 'idl'.
> > + *
> > + * This function should be called between ovsdb_idl_create() and the first
> > call
> > + * to ovsdb_idl_run().
> > + */
> > +static void
> > +ovsdb_idl_db_omit_alert(struct ovsdb_idl_db *db,
> > +const struct ovsdb_idl_column *column)
>
> I think it should be 'db' instead of 'idl'.
In the comment? Thanks, fixed.
> > @@ -2972,10 +3072,10 @@ ovsdb_idl_txn_create(struct ovsdb_idl *idl)
> > {
> > struct ovsdb_idl_txn *txn;
> >
> > -ovs_assert(!idl->txn);
> > -idl->txn = txn = xmalloc(sizeof *txn);
> > +ovs_assert(!idl->db.txn);
> > +idl->db.txn = txn = xmalloc(sizeof *txn);
> > txn->request_id = NULL;
> > -txn->idl = idl;
> > +txn->db = &idl->db;
> > hmap_init(&txn->txn_rows);
>
>
> Should there be an assert in this function that 'outstanding_txns' is empty?
No, it's OK to have outstanding transactions committing in the
background, you just can't be constructing two transactions at once
since they would stomp on each other.
> I didn't look super closely at the patch, since it seemed to mostly be
> refactoring. Let me know if you'd like me to look at it more closely.
Thanks for the review. I don't think anything further is needed for
this one. As you said, it's mostly moving code around.
> Acked-by: Justin Pettit
I'm appending the incremental, although it's kind of hard to read due to
the renaming, sorry.
--8<--cut here-->8--
diff --git a/lib/ovsdb-idl.c b/lib/ovsdb-idl.c
index 24ba5b50fddc..285c453e1985 100644
--- a/lib/ovsdb-idl.c
+++ b/lib/ovsdb-idl.c
@@ -165,7 +165,7 @@ static void ovsdb_idl_send_monitor_request(struct ovsdb_idl
*,
struct ovsdb_idl {
struct ovsdb_idl_db server;
-struct ovsdb_idl_db db; /* XXX rename 'data'? */
+struct ovsdb_idl_db data;
/* Session state.
*
@@ -253,6 +253,7 @@ static void ovsdb_idl_row_clear_old(struct ovsdb_idl_row *);
static void ovsdb_idl_row_clear_new(struct ovsdb_idl_row *);
static void ovsdb_idl_row_clear_arcs(struct ovsdb_idl_row *, bool
destroy_dsts);
+static void ovsdb_idl_db_txn_abort_all(struct ovsdb_idl_db *);
static void ovsdb_idl_txn_abort_all(struct ovsdb_idl *);
static bool ovsdb_idl_db_txn_process_reply(struct ovsdb_idl_db *,
const struct jsonrpc_msg *msg);
@@ -365,7 +366,7 @@ ovsdb_idl_create(const char *remote, const struct
ovsdb_idl_class *class,
struct ovsdb_idl *idl;
idl = xzall
[ovs-dev] OVS DPDK: dpdk_merge pull request for branch-2.9
Hi Ben, The following changes since commit 448a18458328655539599eb4d24b525d311d13df: netdev-dpdk: Fix xstats leak on port destruction. (2018-01-26 20:40:52 +) are available in the git repository at: https://github.com/istokes/ovs dpdk_merge_2_9 for you to fetch changes up to a0b62aacb5021e5a16e6a916b4add61e1b68ec01: netdev-dpdk: Add support for vHost dequeue zero copy (experimental) (2018-01-31 18:44:10 +) Ciara Loftus (1): netdev-dpdk: Add support for vHost dequeue zero copy (experimental) Documentation/intro/install/dpdk.rst | 2 ++ Documentation/topics/dpdk/vhost-user.rst | 73 + NEWS | 1 + lib/netdev-dpdk.c| 18 ++ vswitchd/vswitch.xml | 11 +++ 5 files changed, 105 insertions(+) Thanks Ian ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] OVS DPDK: dpdk_merge pull request for master
Hi Ben, The following changes since commit 186667a83c2b09ed9ae08b35c596987cf7d33cfb: classifier: Fix typo in comment. (2018-01-30 13:00:52 -0800) are available in the git repository at: https://github.com/istokes/ovs dpdk_merge for you to fetch changes up to 10087cba9deec95aaea080c49f2cbe648ebe92c8: netdev-dpdk: Add support for vHost dequeue zero copy (experimental) (2018-01-31 14:04:35 +) Ciara Loftus (1): netdev-dpdk: Add support for vHost dequeue zero copy (experimental) Documentation/intro/install/dpdk.rst | 2 ++ Documentation/topics/dpdk/vhost-user.rst | 73 + NEWS | 1 + lib/netdev-dpdk.c| 18 ++ vswitchd/vswitch.xml | 11 +++ 5 files changed, 105 insertions(+) Thanks Ian ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] ovs-router: fix router entry cast
On Wed, Jan 31, 2018 at 11:27 AM, Ben Pfaff wrote:
> On Wed, Jan 31, 2018 at 11:03:04AM -0800, William Tu wrote:
>> The offsetof(struct ovs_router_entry, cr) should always be 0,
>> thus the else statement should never be reached.
>>
>> Signed-off-by: William Tu
>> ---
>> lib/ovs-router.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/lib/ovs-router.c b/lib/ovs-router.c
>> index cd2ab15fb003..bc8616858a84 100644
>> --- a/lib/ovs-router.c
>> +++ b/lib/ovs-router.c
>> @@ -71,7 +71,7 @@ ovs_router_entry_cast(const struct cls_rule *cr)
>> if (offsetof(struct ovs_router_entry, cr) == 0) {
>> return CONTAINER_OF(cr, struct ovs_router_entry, cr);
>> } else {
>> -return cr ? CONTAINER_OF(cr, struct ovs_router_entry, cr) : NULL;
>> +OVS_NOT_REACHED();
>> }
>> }
>
> This code seems odd, I would prefer to just write the general-purpose
> code and skip the microoptimizations, like this:
>
> struct ovs_router_entry *
> ovs_router_entry_cast(const struct cls_rule *cr)
> {
> return cr ? CONTAINER_OF(cr, struct ovs_router_entry, cr) : NULL;
> }
>
> GCC optimizes it properly anyhow, in all three versions:
>
> (gdb) disass ovs_router_entry_cast
> Dump of assembler code for function ovs_router_entry_cast:
>0x0e30 <+0>: mov0x4(%esp),%eax
>0x0e34 <+4>: ret
> End of assembler dump.
OK thanks!
I will fix it in v2.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v2 0/3] support table names in CLI
I just noticed these acks. No need for individual acks. Thank you for the reviews. Since I sent out v3 already, I'll give it a few days and then apply this if there are no further comments. On Mon, Jan 15, 2018 at 02:49:09PM -0600, Mark Michelson wrote: > Hi Ben, > > For the series: > Acked-by: Mark Michelson > > If you need me to add my ack to each individual patch, I can. Thanks for the > nice functionality. > > On 01/12/2018 02:57 PM, Ben Pfaff wrote: > >These patches are not intended to be applied until we've branched for 2.9, > >since I didn't finish or send them out in time for 2.9. > > > >v1->v2: Rebase and fix up against concurrent changes in master. > > > >Ben Pfaff (3): > > ofp-actions: Make formatting and parsing functions take a struct > > argument. > > ofp-util: New data structure for mapping between table names and > > numbers. > > Support accepting and displaying table names in OVS tools. > > > > NEWS | 11 + > > include/openvswitch/ofp-actions.h | 34 +- > > include/openvswitch/ofp-parse.h | 20 +- > > include/openvswitch/ofp-print.h | 12 +- > > include/openvswitch/ofp-util.h| 41 +- > > lib/learn.c | 20 +- > > lib/learn.h |6 +- > > lib/learning-switch.c |2 +- > > lib/ofp-actions.c | 1170 > > +++-- > > lib/ofp-parse.c | 104 +++- > > lib/ofp-print.c | 242 +--- > > lib/ofp-util.c| 255 ++-- > > lib/vconn.c | 10 +- > > ofproto/ofproto-dpif-trace.c |8 +- > > ofproto/ofproto-dpif-xlate.c | 12 +- > > ofproto/ofproto-dpif.c|4 +- > > ofproto/ofproto.c |5 +- > > ovn/controller/ofctrl.c | 15 +- > > ovn/controller/pinctrl.c |2 +- > > ovn/utilities/ovn-sbctl.c |5 +- > > ovn/utilities/ovn-trace.c |2 +- > > tests/ofproto.at | 89 ++- > > tests/test-ovn.c |3 +- > > utilities/ovs-ofctl.8.in | 90 +-- > > utilities/ovs-ofctl.c | 313 -- > > utilities/ovs-testcontroller.c|2 +- > > 26 files changed, 1434 insertions(+), 1043 deletions(-) > > > ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v2 3/3] Support accepting and displaying table names in OVS tools.
On Fri, Jan 12, 2018 at 01:07:24PM -0800, Kei Nohguchi wrote: > Hi Ben, > > On Fri, Jan 12, 2018 at 12:57:22PM -0800, Ben Pfaff wrote: > > OpenFlow has little-known support for naming tables. Open vSwitch has > > supported table names for ages, but it has never used or displayed them > > outside of commands dedicated to table manipulation. This commit adds > > support for table names in ovs-ofctl. When a table has a name, it displays > > that name in flows and actions, so that, for example, the following: > > table=1, arp, actions=resubmit(,2) > > might become: > > table=ingress_acl, arp, actions=resubmit(,mac_learning) > > given appropriately named tables. > > > > For backward compatibility, only interactive ovs-ofctl commands by default > > display table names; to display them in scripts, use the new --names > > option. > > > > This feature was inspired by a talk that Kei Nohguchi presented at Open > > vSwitch 2017 Fall Conference. > > > > CC: Kei Nohguchi > > Signed-off-by: Ben Pfaff > > Woo! Thank you, Ben, for conidering that! > > I'll apply it locally and get back to you. Do you still plan to try this out? I just sent out v3, but it's just a rebase: https://patchwork.ozlabs.org/project/openvswitch/list/?series=26321 ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v2 2/3] ofp-util: New data structure for mapping between table names and numbers.
Thanks for the reviews of patches 1 and 2. Do you plan to review patch
3 as well? I rebased and sent out v3 just now.
On Tue, Jan 30, 2018 at 01:29:56PM -0800, Yifeng Sun wrote:
> Looks good to me, thanks.
>
> Reviewed-by: Yifeng Sun
>
>
> On Fri, Jan 12, 2018 at 12:57 PM, Ben Pfaff wrote:
>
> > This shares the infrastructure for mapping port names and numbers. It will
> > be used in an upcoming commit.
> >
> > Signed-off-by: Ben Pfaff
> > ---
> > include/openvswitch/ofp-util.h | 33 +++--
> > lib/ofp-util.c | 150 ++
> > ---
> > 2 files changed, 138 insertions(+), 45 deletions(-)
> >
> > diff --git a/include/openvswitch/ofp-util.h b/include/openvswitch/ofp-
> > util.h
> > index 296078a2fe8b..d9780dd44582 100644
> > --- a/include/openvswitch/ofp-util.h
> > +++ b/include/openvswitch/ofp-util.h
> > @@ -43,15 +43,24 @@ union ofp_action;
> > struct ofpact_set_field;
> > struct vl_mff_map;
> >
> > -/* Mapping between port numbers and names. */
> > -struct ofputil_port_map {
> > +/* Name-number mapping.
> > + *
> > + * This is not exported directly but only through specializations for port
> > + * name-number and table name-number mappings. */
> > +struct ofputil_name_map {
> > struct hmap by_name;
> > struct hmap by_number;
> > };
> > -
> > -#define OFPUTIL_PORT_MAP_INITIALIZER(MAP) \
> > +#define OFPUTIL_NAME_MAP_INITIALIZER(MAP) \
> > { HMAP_INITIALIZER(&(MAP)->by_name),
> > HMAP_INITIALIZER(&(MAP)->by_number)
> > }
> >
> > +/* Mapping between port numbers and names. */
> > +struct ofputil_port_map {
> > +struct ofputil_name_map map;
> > +};
> > +#define OFPUTIL_PORT_MAP_INITIALIZER(MAP) \
> > +{ OFPUTIL_NAME_MAP_INITIALIZER(&(MAP)->map) }
> > +
> > void ofputil_port_map_init(struct ofputil_port_map *);
> > const char *ofputil_port_map_get_name(const struct ofputil_port_map *,
> >ofp_port_t);
> > @@ -791,6 +800,22 @@ struct ofputil_table_mod_prop_vacancy {
> > uint8_t vacancy; /* Current vacancy (%). */
> > };
> >
> > +/* Mapping between table numbers and names. */
> > +struct ofputil_table_map {
> > +struct ofputil_name_map map;
> > +};
> > +#define OFPUTIL_TABLE_MAP_INITIALIZER(MAP) \
> > +{ OFPUTIL_NAME_MAP_INITIALIZER((MAP).map) }
> > +
> > +void ofputil_table_map_init(struct ofputil_table_map *);
> > +const char *ofputil_table_map_get_name(const struct ofputil_table_map *,
> > + uint8_t);
> > +uint8_t ofputil_table_map_get_number(const struct ofputil_table_map *,
> > + const char *name);
> > +void ofputil_table_map_put(struct ofputil_table_map *,
> > + uint8_t, const char *name);
> > +void ofputil_table_map_destroy(struct ofputil_table_map *);
> > +
> > /* Abstract ofp_table_mod. */
> > struct ofputil_table_mod {
> > uint8_t table_id; /* ID of the table, 0xff indicates all
> > tables. */
> > diff --git a/lib/ofp-util.c b/lib/ofp-util.c
> > index 597112e4f84e..f3b2e3f6108c 100644
> > --- a/lib/ofp-util.c
> > +++ b/lib/ofp-util.c
> > @@ -7454,12 +7454,14 @@ ofputil_port_to_string(ofp_port_t port,
> > snprintf(namebuf, bufsize, "%"PRIu32, port);
> > }
> >
> > -/* ofputil_port_map. */
> > -struct ofputil_port_map_node {
> > +/* ofputil_name_map. */
> > +
> > +struct ofputil_name_map_node {
> > struct hmap_node name_node;
> > struct hmap_node number_node;
> > -ofp_port_t ofp_port;/* Port number. */
> > -char *name; /* Port name. */
> > +
> > +uint32_t number;
> > +char *name;
> >
> > /* OpenFlow doesn't require port names to be unique, although that's
> > the
> > * only sensible way. However, even in Open vSwitch it's possible
> > for two
> > @@ -7469,22 +7471,25 @@ struct ofputil_port_map_node {
> > * corner case.
> > *
> > * OpenFlow does require port numbers to be unique. We check for
> > duplicate
> > - * ports numbers just in case a switch has a bug. */
> > + * ports numbers just in case a switch has a bug.
> > + *
> > + * OpenFlow doesn't require table names to be unique and Open vSwitch
> > + * doesn't try to make them unique. */
> > bool duplicate;
> > };
> >
> > -void
> > -ofputil_port_map_init(struct ofputil_port_map *map)
> > +static void
> > +ofputil_name_map_init(struct ofputil_name_map *map)
> > {
> > hmap_init(&map->by_name);
> > hmap_init(&map->by_number);
> > }
> >
> > -static struct ofputil_port_map_node *
> > -ofputil_port_map_find_by_name(const struct ofputil_port_map *map,
> > +static struct ofputil_name_map_node *
> > +ofputil_name_map_find_by_name(const struct ofputil_name_map *map,
> >const char *name)
> > {
> > -struct ofputil_port_map_node *node;
> > +struct ofputil_name_map_node *node;
> >
> > HMAP_FOR_EACH_WITH_HASH (node, name_node, hash_s
[ovs-dev] [PATCH v3 3/3] Support accepting and displaying table names in OVS tools.
OpenFlow has little-known support for naming tables. Open vSwitch has
supported table names for ages, but it has never used or displayed them
outside of commands dedicated to table manipulation. This commit adds
support for table names in ovs-ofctl. When a table has a name, it displays
that name in flows and actions, so that, for example, the following:
table=1, arp, actions=resubmit(,2)
might become:
table=ingress_acl, arp, actions=resubmit(,mac_learning)
given appropriately named tables.
For backward compatibility, only interactive ovs-ofctl commands by default
display table names; to display them in scripts, use the new --names
option.
This feature was inspired by a talk that Kei Nohguchi presented at Open
vSwitch 2017 Fall Conference.
CC: Kei Nohguchi
Signed-off-by: Ben Pfaff
---
NEWS | 8 ++
include/openvswitch/ofp-actions.h | 2 +
include/openvswitch/ofp-parse.h | 20 ++-
include/openvswitch/ofp-print.h | 12 +-
include/openvswitch/ofp-util.h| 8 ++
lib/learn.c | 20 ++-
lib/learn.h | 6 +-
lib/learning-switch.c | 2 +-
lib/ofp-actions.c | 45 +++---
lib/ofp-parse.c | 80 +++
lib/ofp-print.c | 202 --
lib/ofp-util.c| 109 --
lib/vconn.c | 10 +-
ofproto/ofproto-dpif.c| 4 +-
ofproto/ofproto.c | 2 +-
ovn/controller/ofctrl.c | 12 +-
ovn/controller/pinctrl.c | 2 +-
ovn/utilities/ovn-sbctl.c | 2 +-
ovn/utilities/ovn-trace.c | 2 +-
tests/ofproto.at | 89
utilities/ovs-ofctl.8.in | 90 +++-
utilities/ovs-ofctl.c | 294 +++---
utilities/ovs-testcontroller.c| 2 +-
23 files changed, 736 insertions(+), 287 deletions(-)
diff --git a/NEWS b/NEWS
index 726589ce3896..d76958b8adc0 100644
--- a/NEWS
+++ b/NEWS
@@ -2,6 +2,14 @@ Post-v2.9.0
- ovs-vswitchd:
* New options --l7 and --l7-len to "ofproto/trace" command.
+ - ovs-ofctl:
+ * ovs-ofctl now accepts and display table names in place of numbers. By
+ default it always accepts names and in interactive use it displays them;
+ use --names or --no-names to override. See ovs-ofctl(8) for details.
+ - ovs-vswitchd:
+ * Previous versions gave OpenFlow tables default names of the form
+ "table#". These are not helpful names for the purpose of accepting
+ and displaying table names, so now tables by default have no names.
v2.9.0 - xx xxx
diff --git a/include/openvswitch/ofp-actions.h
b/include/openvswitch/ofp-actions.h
index 454c705ccf73..cba027b1d945 100644
--- a/include/openvswitch/ofp-actions.h
+++ b/include/openvswitch/ofp-actions.h
@@ -1068,6 +1068,7 @@ uint32_t ofpacts_get_meter(const struct ofpact[], size_t
ofpacts_len);
struct ofpact_format_params {
/* Input. */
const struct ofputil_port_map *port_map;
+const struct ofputil_table_map *table_map;
/* Output. */
struct ds *s;
@@ -1080,6 +1081,7 @@ const char *ofpact_name(enum ofpact_type);
struct ofpact_parse_params {
/* Input. */
const struct ofputil_port_map *port_map;
+const struct ofputil_table_map *table_map;
/* Output. */
struct ofpbuf *ofpacts;
diff --git a/include/openvswitch/ofp-parse.h b/include/openvswitch/ofp-parse.h
index 013a8f3edf70..c4228a50358f 100644
--- a/include/openvswitch/ofp-parse.h
+++ b/include/openvswitch/ofp-parse.h
@@ -45,26 +45,33 @@ enum ofputil_protocol;
char *parse_ofp_str(struct ofputil_flow_mod *, int command, const char *str_,
const struct ofputil_port_map *,
+const struct ofputil_table_map *,
enum ofputil_protocol *usable_protocols)
OVS_WARN_UNUSED_RESULT;
char *parse_ofp_flow_mod_str(struct ofputil_flow_mod *, const char *string,
- const struct ofputil_port_map *, int command,
+ const struct ofputil_port_map *,
+ const struct ofputil_table_map *,
+ int command,
enum ofputil_protocol *usable_protocols)
OVS_WARN_UNUSED_RESULT;
char *parse_ofp_packet_out_str(struct ofputil_packet_out *po, const char *str_,
const struct ofputil_port_map *,
+ const struct ofputil_table_map *,
enum ofputil_protocol *usable_protocols)
OVS_WARN_UNUSED_RESULT;
char *parse_ofp_table_mod(struct ofputil_table_mod *,
const char *table_id, const char *flow_miss_handling,
+ const struct ofputil_table_map *,
uint32_t *usable_versions)
OVS_WA
[ovs-dev] [PATCH v3 1/3] ofp-actions: Make formatting and parsing functions take a struct argument.
An upcoming commit will add another parameter for parsing and formatting
actions. It is much easier to add these parameters if they are
encapsulated in a struct, so this commit first makes that change.
Signed-off-by: Ben Pfaff
Reviewed-by: Yifeng Sun
---
include/openvswitch/ofp-actions.h | 32 +-
lib/ofp-actions.c | 1137 +++--
lib/ofp-parse.c | 24 +-
lib/ofp-print.c | 40 +-
ofproto/ofproto-dpif-trace.c |8 +-
ofproto/ofproto-dpif-xlate.c | 12 +-
ofproto/ofproto.c |3 +-
ovn/controller/ofctrl.c |3 +-
ovn/utilities/ovn-sbctl.c |3 +-
tests/test-ovn.c |3 +-
utilities/ovs-ofctl.c | 19 +-
11 files changed, 565 insertions(+), 719 deletions(-)
diff --git a/include/openvswitch/ofp-actions.h
b/include/openvswitch/ofp-actions.h
index 4e957358f90a..454c705ccf73 100644
--- a/include/openvswitch/ofp-actions.h
+++ b/include/openvswitch/ofp-actions.h
@@ -1064,18 +1064,32 @@ bool ofpacts_equal_stringwise(const struct ofpact a[],
size_t a_len,
const struct mf_field *ofpact_get_mf_dst(const struct ofpact *ofpact);
uint32_t ofpacts_get_meter(const struct ofpact[], size_t ofpacts_len);
-/* Formatting and parsing ofpacts. */
+/* Formatting ofpacts. */
+struct ofpact_format_params {
+/* Input. */
+const struct ofputil_port_map *port_map;
+
+/* Output. */
+struct ds *s;
+};
void ofpacts_format(const struct ofpact[], size_t ofpacts_len,
-const struct ofputil_port_map *, struct ds *);
-char *ofpacts_parse_actions(const char *, const struct ofputil_port_map *,
-struct ofpbuf *ofpacts,
-enum ofputil_protocol *usable_protocols)
+const struct ofpact_format_params *);
+const char *ofpact_name(enum ofpact_type);
+
+/* Parsing ofpacts. */
+struct ofpact_parse_params {
+/* Input. */
+const struct ofputil_port_map *port_map;
+
+/* Output. */
+struct ofpbuf *ofpacts;
+enum ofputil_protocol *usable_protocols;
+};
+char *ofpacts_parse_actions(const char *, const struct ofpact_parse_params *)
OVS_WARN_UNUSED_RESULT;
-char *ofpacts_parse_instructions(const char *, const struct ofputil_port_map *,
- struct ofpbuf *ofpacts,
- enum ofputil_protocol *usable_protocols)
+char *ofpacts_parse_instructions(const char *,
+ const struct ofpact_parse_params *)
OVS_WARN_UNUSED_RESULT;
-const char *ofpact_name(enum ofpact_type);
/* Internal use by the helpers below. */
void ofpact_init(struct ofpact *, enum ofpact_type, size_t len);
diff --git a/lib/ofp-actions.c b/lib/ofp-actions.c
index 012494976600..1000d6d1de6d 100644
--- a/lib/ofp-actions.c
+++ b/lib/ofp-actions.c
@@ -417,8 +417,7 @@ static void *ofpact_put_raw(struct ofpbuf *, enum
ofp_version,
enum ofp_raw_action_type, uint64_t arg);
static char *OVS_WARN_UNUSED_RESULT ofpacts_parse(
-char *str, const struct ofputil_port_map *,
-struct ofpbuf *ofpacts, enum ofputil_protocol *usable_protocols,
+char *str, const struct ofpact_parse_params *pp,
bool allow_instructions, enum ofpact_type outer_action);
static enum ofperr ofpacts_pull_openflow_actions__(
struct ofpbuf *openflow, unsigned int actions_len,
@@ -426,8 +425,7 @@ static enum ofperr ofpacts_pull_openflow_actions__(
struct ofpbuf *ofpacts, enum ofpact_type outer_action,
const struct vl_mff_map *vl_mff_map, uint64_t *ofpacts_tlv_bitmap);
static char * OVS_WARN_UNUSED_RESULT ofpacts_parse_copy(
-const char *s_, const struct ofputil_port_map *, struct ofpbuf *ofpacts,
-enum ofputil_protocol *usable_protocols,
+const char *s_, const struct ofpact_parse_params *pp,
bool allow_instructions, enum ofpact_type outer_action);
/* Returns the ofpact following 'ofpact', except that if 'ofpact' contains
@@ -607,16 +605,16 @@ encode_OUTPUT(const struct ofpact_output *output,
}
static char * OVS_WARN_UNUSED_RESULT
-parse_truncate_subfield(struct ofpact_output_trunc *output_trunc,
-const char *arg_,
-const struct ofputil_port_map *port_map)
+parse_truncate_subfield(const char *arg_,
+const struct ofpact_parse_params *pp,
+struct ofpact_output_trunc *output_trunc)
{
char *key, *value;
char *arg = CONST_CAST(char *, arg_);
while (ofputil_parse_key_value(&arg, &key, &value)) {
if (!strcmp(key, "port")) {
-if (!ofputil_port_from_string(value, port_map,
+if (!ofputil_port_from_string(value, pp->port_map,
&output_trunc->port)) {
return xasprintf("output to unknown truncate port: %s",
value);
@@ -643,21 +
[ovs-dev] [PATCH v3 2/3] ofp-util: New data structure for mapping between table names and numbers.
This shares the infrastructure for mapping port names and numbers. It will
be used in an upcoming commit.
Signed-off-by: Ben Pfaff
Reviewed-by: Yifeng Sun
---
include/openvswitch/ofp-util.h | 33 +++--
lib/ofp-util.c | 150 ++---
2 files changed, 138 insertions(+), 45 deletions(-)
diff --git a/include/openvswitch/ofp-util.h b/include/openvswitch/ofp-util.h
index 296078a2fe8b..d9780dd44582 100644
--- a/include/openvswitch/ofp-util.h
+++ b/include/openvswitch/ofp-util.h
@@ -43,15 +43,24 @@ union ofp_action;
struct ofpact_set_field;
struct vl_mff_map;
-/* Mapping between port numbers and names. */
-struct ofputil_port_map {
+/* Name-number mapping.
+ *
+ * This is not exported directly but only through specializations for port
+ * name-number and table name-number mappings. */
+struct ofputil_name_map {
struct hmap by_name;
struct hmap by_number;
};
-
-#define OFPUTIL_PORT_MAP_INITIALIZER(MAP) \
+#define OFPUTIL_NAME_MAP_INITIALIZER(MAP) \
{ HMAP_INITIALIZER(&(MAP)->by_name), HMAP_INITIALIZER(&(MAP)->by_number) }
+/* Mapping between port numbers and names. */
+struct ofputil_port_map {
+struct ofputil_name_map map;
+};
+#define OFPUTIL_PORT_MAP_INITIALIZER(MAP) \
+{ OFPUTIL_NAME_MAP_INITIALIZER(&(MAP)->map) }
+
void ofputil_port_map_init(struct ofputil_port_map *);
const char *ofputil_port_map_get_name(const struct ofputil_port_map *,
ofp_port_t);
@@ -791,6 +800,22 @@ struct ofputil_table_mod_prop_vacancy {
uint8_t vacancy; /* Current vacancy (%). */
};
+/* Mapping between table numbers and names. */
+struct ofputil_table_map {
+struct ofputil_name_map map;
+};
+#define OFPUTIL_TABLE_MAP_INITIALIZER(MAP) \
+{ OFPUTIL_NAME_MAP_INITIALIZER((MAP).map) }
+
+void ofputil_table_map_init(struct ofputil_table_map *);
+const char *ofputil_table_map_get_name(const struct ofputil_table_map *,
+ uint8_t);
+uint8_t ofputil_table_map_get_number(const struct ofputil_table_map *,
+ const char *name);
+void ofputil_table_map_put(struct ofputil_table_map *,
+ uint8_t, const char *name);
+void ofputil_table_map_destroy(struct ofputil_table_map *);
+
/* Abstract ofp_table_mod. */
struct ofputil_table_mod {
uint8_t table_id; /* ID of the table, 0xff indicates all tables. */
diff --git a/lib/ofp-util.c b/lib/ofp-util.c
index 597112e4f84e..f3b2e3f6108c 100644
--- a/lib/ofp-util.c
+++ b/lib/ofp-util.c
@@ -7454,12 +7454,14 @@ ofputil_port_to_string(ofp_port_t port,
snprintf(namebuf, bufsize, "%"PRIu32, port);
}
-/* ofputil_port_map. */
-struct ofputil_port_map_node {
+/* ofputil_name_map. */
+
+struct ofputil_name_map_node {
struct hmap_node name_node;
struct hmap_node number_node;
-ofp_port_t ofp_port;/* Port number. */
-char *name; /* Port name. */
+
+uint32_t number;
+char *name;
/* OpenFlow doesn't require port names to be unique, although that's the
* only sensible way. However, even in Open vSwitch it's possible for two
@@ -7469,22 +7471,25 @@ struct ofputil_port_map_node {
* corner case.
*
* OpenFlow does require port numbers to be unique. We check for duplicate
- * ports numbers just in case a switch has a bug. */
+ * ports numbers just in case a switch has a bug.
+ *
+ * OpenFlow doesn't require table names to be unique and Open vSwitch
+ * doesn't try to make them unique. */
bool duplicate;
};
-void
-ofputil_port_map_init(struct ofputil_port_map *map)
+static void
+ofputil_name_map_init(struct ofputil_name_map *map)
{
hmap_init(&map->by_name);
hmap_init(&map->by_number);
}
-static struct ofputil_port_map_node *
-ofputil_port_map_find_by_name(const struct ofputil_port_map *map,
+static struct ofputil_name_map_node *
+ofputil_name_map_find_by_name(const struct ofputil_name_map *map,
const char *name)
{
-struct ofputil_port_map_node *node;
+struct ofputil_name_map_node *node;
HMAP_FOR_EACH_WITH_HASH (node, name_node, hash_string(name, 0),
&map->by_name) {
@@ -7495,38 +7500,38 @@ ofputil_port_map_find_by_name(const struct
ofputil_port_map *map,
return NULL;
}
-static struct ofputil_port_map_node *
-ofputil_port_map_find_by_number(const struct ofputil_port_map *map,
-ofp_port_t ofp_port)
+static struct ofputil_name_map_node *
+ofputil_name_map_find_by_number(const struct ofputil_name_map *map,
+uint32_t number)
{
-struct ofputil_port_map_node *node;
+struct ofputil_name_map_node *node;
-HMAP_FOR_EACH_IN_BUCKET (node, number_node, hash_ofp_port(ofp_port),
+HMAP_FOR_EACH_IN_BUCKET (node, number_node, hash_int(number, 0),
&map->by_nu
[ovs-dev] [PATCH v3 0/3] support table names in CLI
These patches are intended for master only. v1->v2: Rebase and fix up against concurrent changes in master. v2->v3: Rebase and fix up against master. Ben Pfaff (3): ofp-actions: Make formatting and parsing functions take a struct argument. ofp-util: New data structure for mapping between table names and numbers. Support accepting and displaying table names in OVS tools. NEWS |8 + include/openvswitch/ofp-actions.h | 34 +- include/openvswitch/ofp-parse.h | 20 +- include/openvswitch/ofp-print.h | 12 +- include/openvswitch/ofp-util.h| 41 +- lib/learn.c | 20 +- lib/learn.h |6 +- lib/learning-switch.c |2 +- lib/ofp-actions.c | 1170 +++-- lib/ofp-parse.c | 104 +++- lib/ofp-print.c | 242 +--- lib/ofp-util.c| 255 ++-- lib/vconn.c | 10 +- ofproto/ofproto-dpif-trace.c |8 +- ofproto/ofproto-dpif-xlate.c | 12 +- ofproto/ofproto-dpif.c|4 +- ofproto/ofproto.c |5 +- ovn/controller/ofctrl.c | 15 +- ovn/controller/pinctrl.c |2 +- ovn/utilities/ovn-sbctl.c |5 +- ovn/utilities/ovn-trace.c |2 +- tests/ofproto.at | 89 ++- tests/test-ovn.c |3 +- utilities/ovs-ofctl.8.in | 90 +-- utilities/ovs-ofctl.c | 313 -- utilities/ovs-testcontroller.c|2 +- 26 files changed, 1431 insertions(+), 1043 deletions(-) -- 2.15.1 ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] ovs-router: fix router entry cast
On Wed, Jan 31, 2018 at 11:03:04AM -0800, William Tu wrote:
> The offsetof(struct ovs_router_entry, cr) should always be 0,
> thus the else statement should never be reached.
>
> Signed-off-by: William Tu
> ---
> lib/ovs-router.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/lib/ovs-router.c b/lib/ovs-router.c
> index cd2ab15fb003..bc8616858a84 100644
> --- a/lib/ovs-router.c
> +++ b/lib/ovs-router.c
> @@ -71,7 +71,7 @@ ovs_router_entry_cast(const struct cls_rule *cr)
> if (offsetof(struct ovs_router_entry, cr) == 0) {
> return CONTAINER_OF(cr, struct ovs_router_entry, cr);
> } else {
> -return cr ? CONTAINER_OF(cr, struct ovs_router_entry, cr) : NULL;
> +OVS_NOT_REACHED();
> }
> }
This code seems odd, I would prefer to just write the general-purpose
code and skip the microoptimizations, like this:
struct ovs_router_entry *
ovs_router_entry_cast(const struct cls_rule *cr)
{
return cr ? CONTAINER_OF(cr, struct ovs_router_entry, cr) : NULL;
}
GCC optimizes it properly anyhow, in all three versions:
(gdb) disass ovs_router_entry_cast
Dump of assembler code for function ovs_router_entry_cast:
0x0e30 <+0>: mov0x4(%esp),%eax
0x0e34 <+4>: ret
End of assembler dump.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH] util: Document and rely on ovs_assert() always evaluating its argument.
The ovs_assert() macro always evaluates its argument, even when NDEBUG is
defined so that failure is ignored. This behavior wasn't documented, and
thus a lot of code didn't rely on it. This commit documents the behavior
and simplifies bits of code that heretofore didn't rely on it.
Signed-off-by: Ben Pfaff
---
include/openvswitch/util.h | 7 +--
lib/cmap.c | 6 ++
lib/conntrack.c | 6 ++
lib/hmapx.c | 6 ++
lib/odp-execute.c| 5 +
lib/odp-util.c | 5 +
lib/ofp-msgs.c | 32
lib/ofp-util.c | 11 ---
lib/ovsdb-data.c | 4 +---
lib/shash.c | 3 +--
lib/sset.c | 6 ++
ofproto/ofproto-dpif-xlate.c | 7 +++
ovsdb/ovsdb-server.c | 4 +---
ovsdb/replication.c | 3 +--
14 files changed, 34 insertions(+), 71 deletions(-)
diff --git a/include/openvswitch/util.h b/include/openvswitch/util.h
index ad1b184f78d9..b4d49ee21804 100644
--- a/include/openvswitch/util.h
+++ b/include/openvswitch/util.h
@@ -44,8 +44,11 @@ const char *ovs_get_program_version(void);
: (X) <= UINT_MAX / (Y) ? (unsigned int) (X) * (unsigned int) (Y) \
: UINT_MAX)
-/* Like the standard assert macro, except writes the failure message to the
- * log. */
+/* Like the standard assert macro, except:
+ *
+ *- Writes the failure message to the log.
+ *
+ *- Always evaluates the condition, even with NDEBUG. */
#ifndef NDEBUG
#define ovs_assert(CONDITION) \
(OVS_LIKELY(CONDITION) \
diff --git a/lib/cmap.c b/lib/cmap.c
index af29639b049c..07719a8fd286 100644
--- a/lib/cmap.c
+++ b/lib/cmap.c
@@ -843,11 +843,9 @@ cmap_replace(struct cmap *cmap, struct cmap_node *old_node,
struct cmap_impl *impl = cmap_get_impl(cmap);
uint32_t h1 = rehash(impl, hash);
uint32_t h2 = other_hash(h1);
-bool ok;
-ok = cmap_replace__(impl, old_node, new_node, hash, h1)
-|| cmap_replace__(impl, old_node, new_node, hash, h2);
-ovs_assert(ok);
+ovs_assert(cmap_replace__(impl, old_node, new_node, hash, h1) ||
+ cmap_replace__(impl, old_node, new_node, hash, h2));
if (!new_node) {
impl->n--;
diff --git a/lib/conntrack.c b/lib/conntrack.c
index 562e767833d1..fe5fd0fe8be1 100644
--- a/lib/conntrack.c
+++ b/lib/conntrack.c
@@ -3080,11 +3080,9 @@ repl_ftp_v6_addr(struct dp_packet *pkt, struct ct_addr
v6_addr_rep,
return 0;
}
-const char *rc;
char v6_addr_str[IPV6_SCAN_LEN] = {0};
-rc = inet_ntop(AF_INET6, &v6_addr_rep.ipv6_aligned, v6_addr_str,
- IPV6_SCAN_LEN - 1);
-ovs_assert(rc != NULL);
+ovs_assert(inet_ntop(AF_INET6, &v6_addr_rep.ipv6_aligned, v6_addr_str,
+ IPV6_SCAN_LEN - 1));
size_t replace_addr_size = strlen(v6_addr_str);
diff --git a/lib/hmapx.c b/lib/hmapx.c
index 0440767c9c97..eadfe640ac11 100644
--- a/lib/hmapx.c
+++ b/lib/hmapx.c
@@ -116,8 +116,7 @@ hmapx_add(struct hmapx *map, void *data)
void
hmapx_add_assert(struct hmapx *map, void *data)
{
-bool added OVS_UNUSED = hmapx_add(map, data);
-ovs_assert(added);
+ovs_assert(hmapx_add(map, data));
}
/* Removes all of the nodes from 'map'. */
@@ -156,8 +155,7 @@ hmapx_find_and_delete(struct hmapx *map, const void *data)
void
hmapx_find_and_delete_assert(struct hmapx *map, const void *data)
{
-bool deleted OVS_UNUSED = hmapx_find_and_delete(map, data);
-ovs_assert(deleted);
+ovs_assert(hmapx_find_and_delete(map, data));
}
/* Searches for 'data' in 'map'. Returns its node, if found, otherwise a null
diff --git a/lib/odp-execute.c b/lib/odp-execute.c
index 20f33eb57acb..12bba83ea32c 100644
--- a/lib/odp-execute.c
+++ b/lib/odp-execute.c
@@ -198,10 +198,7 @@ odp_set_sctp(struct dp_packet *packet, const struct
ovs_key_sctp *key,
static void
odp_set_tunnel_action(const struct nlattr *a, struct flow_tnl *tun_key)
{
-enum odp_key_fitness fitness;
-
-fitness = odp_tun_key_from_attr(a, tun_key);
-ovs_assert(fitness != ODP_FIT_ERROR);
+ovs_assert(odp_tun_key_from_attr(a, tun_key) != ODP_FIT_ERROR);
}
static void
diff --git a/lib/odp-util.c b/lib/odp-util.c
index 6a29a76de5cd..14d5af097ee4 100644
--- a/lib/odp-util.c
+++ b/lib/odp-util.c
@@ -6847,8 +6847,6 @@ commit_mpls_action(const struct flow *flow, struct flow
*base,
/* Otherwise, if there more LSEs in base than are common between
* base and flow then pop the topmost one. */
ovs_be16 dl_type;
-bool popped;
-
/* If all the LSEs are to be popped and this is not the outermost
* LSE then use ETH_TYPE_MPLS as the ethertype parameter of the
* POP_MPLS action instead of flow->dl_type.
@@ -6865,8 +6863,7 @@ commit_mpls_actio
Re: [ovs-dev] [PATCH] classifier: Refactor interface for classifier_remove().
Thanks for the review! Sorry about duplicating your work--I think that
we both prepared the same change at the same time.
I applied this to master.
On Tue, Jan 30, 2018 at 01:16:26PM -0800, Yifeng Sun wrote:
> Thanks for the patch, which is better than the one I submitted.
>
> Tested-by: Yifeng Sun
>
> Reviewed-by: Yifeng Sun
>
> On Tue, Jan 30, 2018 at 1:00 PM, Ben Pfaff wrote:
>
> > Until now, classifier_remove() returned either null or the classifier rule
> > passed to it, which is an unusual interface. This commit changes it to
> > return true if it succeeds or false on failure.
> >
> > In addition, most of classifier_remove()'s callers know ahead of time that
> > it must succeed, even though most of them didn't bother with an assertion,
> > so this commit adds a classifier_remove_assert() function as a helper.
> >
> > Signed-off-by: Ben Pfaff
> > ---
> > lib/classifier.c| 25 +
> > lib/classifier.h| 4 ++--
> > lib/ovs-router.c| 19 ---
> > lib/tnl-ports.c | 5 ++---
> > ofproto/ofproto.c | 14 --
> > tests/test-classifier.c | 19 +--
> > tests/test-ovn.c| 2 +-
> > utilities/ovs-ofctl.c | 2 +-
> > 8 files changed, 44 insertions(+), 46 deletions(-)
> >
> > diff --git a/lib/classifier.c b/lib/classifier.c
> > index 16c451da1b30..9ad3710d61a1 100644
> > --- a/lib/classifier.c
> > +++ b/lib/classifier.c
> > @@ -695,15 +695,16 @@ classifier_insert(struct classifier *cls, const
> > struct cls_rule *rule,
> > ovs_assert(!displaced_rule);
> > }
> >
> > -/* Removes 'rule' from 'cls'. It is the caller's responsibility to
> > destroy
> > - * 'rule' with cls_rule_destroy(), freeing the memory block in which
> > 'rule'
> > - * resides, etc., as necessary.
> > +/* If 'rule' is in 'cls', removes 'rule' from 'cls' and returns true. It
> > is
> > + * the caller's responsibility to destroy 'rule' with cls_rule_destroy(),
> > + * freeing the memory block in which 'rule' resides, etc., as necessary.
> > *
> > - * Does nothing if 'rule' has been already removed, or was never inserted.
> > + * If 'rule' is not in any classifier, returns false without making any
> > + * changes.
> > *
> > - * Returns the removed rule, or NULL, if it was already removed.
> > + * 'rule' must not be in some classifier other than 'cls'.
> > */
> > -const struct cls_rule *
> > +bool
> > classifier_remove(struct classifier *cls, const struct cls_rule *cls_rule)
> > {
> > struct cls_match *rule, *prev, *next, *head;
> > @@ -716,7 +717,7 @@ classifier_remove(struct classifier *cls, const struct
> > cls_rule *cls_rule)
> >
> > rule = get_cls_match_protected(cls_rule);
> > if (!rule) {
> > -return NULL;
> > +return false;
> > }
> > /* Mark as removed. */
> > ovsrcu_set(&CONST_CAST(struct cls_rule *, cls_rule)->cls_match, NULL);
> > @@ -820,7 +821,15 @@ check_priority:
> > ovsrcu_postpone(cls_match_free_cb, rule);
> > cls->n_rules--;
> >
> > -return cls_rule;
> > +return true;
> > +}
> > +
> > +void
> > +classifier_remove_assert(struct classifier *cls,
> > + const struct cls_rule *cls_rule)
> > +{
> > +bool OVS_UNUSED removed = classifier_remove(cls, cls_rule);
> > +ovs_assert(removed);
> > }
> >
> > /* Prefix tree context. Valid when 'lookup_done' is true. Can skip all
> > diff --git a/lib/classifier.h b/lib/classifier.h
> > index 71c2e507d7c3..31d4a1b08bd2 100644
> > --- a/lib/classifier.h
> > +++ b/lib/classifier.h
> > @@ -387,8 +387,8 @@ const struct cls_rule *classifier_replace(struct
> > classifier *,
> >ovs_version_t,
> >const struct cls_conjunction *,
> >size_t n_conjunctions);
> > -const struct cls_rule *classifier_remove(struct classifier *,
> > - const struct cls_rule *);
> > +bool classifier_remove(struct classifier *, const struct cls_rule *);
> > +void classifier_remove_assert(struct classifier *, const struct cls_rule
> > *);
> > static inline void classifier_defer(struct classifier *);
> > static inline void classifier_publish(struct classifier *);
> >
> > diff --git a/lib/ovs-router.c b/lib/ovs-router.c
> > index cd2ab15fb003..e6cc81fd0827 100644
> > --- a/lib/ovs-router.c
> > +++ b/lib/ovs-router.c
> > @@ -245,19 +245,14 @@ ovs_router_insert(uint32_t mark, const struct
> > in6_addr *ip_dst, uint8_t plen,
> > ovs_router_insert__(mark, plen, ip_dst, plen, output_bridge, gw);
> > }
> >
> > -static bool
> > -__rt_entry_delete(const struct cls_rule *cr)
> > +static void
> > +rt_entry_delete__(const struct cls_rule *cr)
> > {
> > struct ovs_router_entry *p = ovs_router_entry_cast(cr);
> >
> > tnl_port_map_delete_ipdev(p->output_bridge);
> > -/* Remove it. */
> > -cr = classifier_remove(&cls, cr);
>
[ovs-dev] [PATCH] ovs-router: fix router entry cast
The offsetof(struct ovs_router_entry, cr) should always be 0,
thus the else statement should never be reached.
Signed-off-by: William Tu
---
lib/ovs-router.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lib/ovs-router.c b/lib/ovs-router.c
index cd2ab15fb003..bc8616858a84 100644
--- a/lib/ovs-router.c
+++ b/lib/ovs-router.c
@@ -71,7 +71,7 @@ ovs_router_entry_cast(const struct cls_rule *cr)
if (offsetof(struct ovs_router_entry, cr) == 0) {
return CONTAINER_OF(cr, struct ovs_router_entry, cr);
} else {
-return cr ? CONTAINER_OF(cr, struct ovs_router_entry, cr) : NULL;
+OVS_NOT_REACHED();
}
}
--
2.7.4
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH 20/20] Documentation: Update NEWS and faq
> On Jan 30, 2018, at 3:49 PM, Gregory Rose wrote: > > On 1/30/2018 3:43 PM, Justin Pettit wrote: >> >>> On Jan 30, 2018, at 3:40 PM, Greg Rose wrote: >>> >>> Signed-off-by: Greg Rose >>> --- >>> Documentation/faq/releases.rst | 1 + >>> NEWS | 2 ++ >>> 2 files changed, 3 insertions(+) >>> >>> diff --git a/Documentation/faq/releases.rst b/Documentation/faq/releases.rst >>> index 62a1957..2f03c89 100644 >>> --- a/Documentation/faq/releases.rst >>> +++ b/Documentation/faq/releases.rst >>> @@ -67,6 +67,7 @@ Q: What Linux kernel versions does each Open vSwitch >>> release work with? >>> 2.7.x3.10 to 4.9 >>> 2.8.x3.10 to 4.12 >>> 2.9.x3.10 to 4.13 >>> +2.10.x 3.10 to 4.14 >> Thanks for the patches, Greg. Should we try to get these into 2.9 or just >> wait for 2.10? >> >> --Justin >> >> > > I assumed 2.10 but... well, I'm not sure. The major features new in the > Linux kernel since 4.13 are meters, NSH and erspan. Not sure if we need > those for 2.9 or not. Thanks, Greg. Pravin, thank you for reviewing the patches. When you're going though them, can you make a decision whether you think they're safe enough to pull into 2.9? Thanks, --Justin ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] kernel crash bug caused by ixgbevf kernel module of centos-3.10.0-229.20.1.el7
On 1/29/2018 6:23 PM, Sam wrote: detail as below, bug is happened on bond of enp1s16 and enp1s16f1 [huanghuai-test@yf-mos-test-net14 ~]$ sudo /usr/local/share/openvswitch/scripts/dpdk_nic_bind --status Network devices using DPDK-compatible driver :01:00.0 'Ethernet Controller 10-Gigabit X540-AT2' drv=igb_uio unused=ixgbe :01:00.1 'Ethernet Controller 10-Gigabit X540-AT2' drv=igb_uio unused=ixgbe Network devices using kernel driver === :01:10.0 'X540 Ethernet Controller Virtual Function' if=enp1s16 drv=ixgbevf unused=bak,igb_uio :01:10.1 'X540 Ethernet Controller Virtual Function' if=enp1s16f1 drv=ixgbevf unused=bak,igb_uio :08:00.0 'I350 Gigabit Network Connection' if=eth2 drv=igb unused=igb_uio :08:00.1 'I350 Gigabit Network Connection' if=eth3 drv=igb unused=igb_uio Other network devices = I'd try reporting it to Intel. https://sourceforge.net/p/e1000/bugs/ - Greg 2018-01-30 10:19 GMT+08:00 Sam : I found a bug about ixgbevf kernel module in centos-3.10.0-229.20.1.el7. And this bug is also in 3.10.0-514.10.2.el7. How to produce this bug: use SRIOV first, then add lots of network traffic on vf port, and then ifdow/ifup vf port, after many times, this bug happens. BUG: [308026.586026] ixgbevf :01:10.0: NIC Link is Down [308026.586037] ixgbevf :01:10.1: NIC Link is Down [308026.683724] bonding: bond1: link status definitely down for interface enp1s16, disabling it [308026.683728] bonding: bond1: now running without any active interface ! [308026.683729] bonding: bond1: link status definitely down for interface enp1s16f1, disabling it [308028.266060] bonding: bond1: Removing slave enp1s16. [308028.266135] bonding: bond1: Warning: the permanent HWaddr of enp1s16 - 4e:cd:a6:59:26:2c - is still in use by bond1. Set the HWaddr of enp1s16 to a different address to avoid conflicts. [308028.266139] bonding: bond1: releasing active interface enp1s16 [308028.359872] BUG: unable to handle kernel NULL pointer dereference at 0008 [308028.361319] IP: [] ixgbevf_alloc_rx_buffers+0x60/0x160 [ixgbevf] [308028.362049] PGD 0 [308028.362777] Oops: [#1] SMP [308028.363481] Modules linked in: ixgbevf(OF) igb_uio(OF) iptable_mangle iptable_nat nf_conntrack_ipv4 nf_defrag_ipv4 nf_nat_ipv4 nf_nat nf_conntrack iptable_filter nbd(OF) vhost_net macvtap macvlan udp_diag unix_diag af_packet_diag netlink_diag tun tcp_diag inet_diag uio bonding ext4 mbcache jbd2 intel_powerclamp coretemp kvm_intel kvm crct10dif_pclmul crc32_pclmul crc32c_intel ghash_clmulni_intel mgag200 aesni_intel iTCO_wdt lrw dcdbas gf128mul syscopyarea sysfillrect iTCO_vendor_support glue_helper sysimgblt ablk_helper ttm cryptd ipmi_devintf igb ixgbe drm_kms_helper drm i2c_algo_bit ptp i2c_core ipmi_si pps_core sg mdio ipmi_msghandler dca sb_edac mei_me mei shpchp lpc_ich pcspkr mfd_core edac_core wmi acpi_power_meter acpi_pad ip_tables xfs libcrc32c sd_mod crc_t10dif crct10dif_common ahci libahci [308028.368487] libata megaraid_sas [last unloaded: ixgbevf] [308028.369345] CPU: 0 PID: 21971 Comm: kworker/0:1 Tainted: GF W O-- 3.10.0-229.el7.x86_64 #1 [308028.370226] Hardware name: Dell Inc. PowerEdge R720/068CDY, BIOS 2.5.2 01/28/2015 [308028.371132] Workqueue: events ixgbevf_service_task [ixgbevf] [308028.372038] task: 88022b0dad80 ti: 88010905c000 task.ti: 88010905c000 [308028.372965] RIP: 0010:[] [] ixgbevf_alloc_rx_buffers+0x60/0x160 [ixgbevf] [308028.373949] RSP: 0018:88010905fd10 EFLAGS: 00010287 [308028.374900] RAX: 0200 RBX: RCX: [308028.375895] RDX: RSI: 01ff RDI: 8800b82061c0 [308028.376841] RBP: 88010905fd48 R08: 0282 R09: 0001 [308028.377780] R10: 0004 R11: 0005 R12: [308028.378702] R13: fe00 R14: 01ff R15: 8800b82061c0 [308028.379628] FS: () GS:882f7fa0() knlGS: [308028.380540] CS: 0010 DS: ES: CR0: 80050033 [308028.381471] CR2: 0008 CR3: 0190a000 CR4: 001427f0 [308028.382376] DR0: DR1: DR2: [308028.383291] DR3: DR6: 0ff0 DR7: 0400 [308028.384180] Stack: [308028.385051] 8832d1b58bc0 88010905fd28 8832d1b588c0 0009 [308028.385933] 8832d1b58bc0 8800b82061c0 1028 88010905fdb8 [308028.386804] a0496ba3 8832d1b58e58 00022b1e2000 819e2108 [308028.387693] Call Trace: [308028.388520] [] ixgbevf_configure+0x5d3/0x7d0 [ixgbevf] [308028.389363] [] ixgbevf_reinit_locked+0x65/0x90 [ixgbevf] [308028.390213] [] ixgbevf_service_task+0x324/0x420 [ixgbevf] [308028.391043] [] process_one_work+0x17b/0x470 [3080
Re: [ovs-dev] [PATCH V2] rhel: Fix support for root user using DPDK
Marcos Felipe Schwarz writes:
> Since 2.8.0 OVS runs as non-root user on rhel distros, but the current
> implementation breaks the ability to run as root with DPDK and as a
> consequence there is no way possible to use UIO drivers on kernel 4.0 and
> newer [1, 2].
> [1] http://dpdk.org/browse/dpdk/commit/?id=cdc242f260e766bd95a658b5e0686a
> 62ec04f5b0
> [2] https://www.kernel.org/doc/Documentation/vm/pagemap.txt
>
> Fixes: e3e738a3d058 ("redhat: allow dpdk to also run as non-root user")
> Signed-off-by: Marcos Schwarz
> ---
> lib/daemon-unix.c | 3 ++-
> rhel/usr_lib_systemd_system_ovs-vswitchd.service.in | 2 +-
> 2 files changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/lib/daemon-unix.c b/lib/daemon-unix.c
> index adb549c98..06528e9ab 100644
> --- a/lib/daemon-unix.c
> +++ b/lib/daemon-unix.c
> @@ -1047,5 +1047,6 @@ daemon_set_new_user(const char *user_spec)
> }
> }
>
> -switch_user = true;
> +if (!uid_verify(uid) || !gid_verify(gid))
> +switch_user = true;
> }
> diff --git a/rhel/usr_lib_systemd_system_ovs-vswitchd.service.in b/rhel/
> usr_lib_systemd_system_ovs-vswitchd.service.in
> index c6d9aa1b8..e8b81e707 100644
> --- a/rhel/usr_lib_systemd_system_ovs-vswitchd.service.in
> +++ b/rhel/usr_lib_systemd_system_ovs-vswitchd.service.in
> @@ -14,7 +14,7 @@ Environment=HOME=/var/run/openvswitch
> EnvironmentFile=/etc/openvswitch/default.conf
> EnvironmentFile=-/etc/sysconfig/openvswitch
> @begin_dpdk@
> -ExecStartPre=-/usr/bin/chown :hugetlbfs /dev/hugepages
> +ExecStartPre=-/bin/sh -c '/usr/bin/chown :${OVS_USER_ID##*:}
I think part of this hunk was lost when moving to v2.
If you post a v3, add my Acked-by.
Thanks, Marcos!
> ExecStartPre=-/usr/bin/chmod 0775 /dev/hugepages
> @end_dpdk@
> ExecStart=/usr/share/openvswitch/scripts/ovs-ctl \
> --
> 2.14.3
> ___
> 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 0/5] datapath: enable NSH support in kernel compat mode
On 1/31/2018 6:12 AM, Yang, Yi Y wrote:
Hi, Eric
My kernel is 4.13.0-rc6, so I'm very sure it can support vxlan-gpe, I can add
vxlan-gpe port without any issue by command line, so I don't know what happened.
Greg, I checked unit tests in tests/system-traffic.at and
tests/system-layer3-tunnels.at for vxlan and vxlan-gpe, I think they are very
tricky, for NSH, they won't work, current test infrastructure can't handle NSH
unit test in kernel data path, so I don't think I can add it. I have posted v2
patch series, I have sfc test environment in my machine and have verified
kernel data path, my test environment has two vagrant VMs, each one VM starts
two containers which play SF roles, end-to-end ping and http are both ok.
Could you provide instructions on how to configure your setup so I can
replicate? That would provide a lot of
confidence.
Thanks,
- Greg
-Original Message-
From: Eric Garver [mailto:e...@erig.me]
Sent: Tuesday, January 30, 2018 9:53 PM
To: Yang, Yi Y
Cc: Gregory Rose ; d...@openvswitch.org
Subject: Re: [ovs-dev] [PATCH v1 0/5] datapath: enable NSH support in kernel
compat mode
On Tue, Jan 30, 2018 at 11:33:55AM +, Yang, Yi Y wrote:
Hi, Greg
I installed linux 3.10.107 in Ubuntu 14.04 and fixed
skb_gso_error_unwind issue, but for unit test,
tests/system-layer3-tunnels.at is a good reference for it because we
used vxlan-gpe for nsh, I ran unit test 90, but it always fails (I
have installed and used net-next kernel and the latest iproute2)
Here is error log
./system-layer3-tunnels.at:25: ip netns exec at_ns0 sh <<
NS_EXEC_HEREDOC ip route add 10.1.1.2/32 encap ip id 0 dst
172.31.1.100 dev at_vxlan1 NS_EXEC_HEREDOC
--- /dev/null 2018-01-30 02:18:43.272647961 +
+++
/home/vagrant/ovs-nsh-test/tests/system-kmod-testsuite.dir/at-groups/90/stderr
2018-01-30 09:45:15.415934534 +
@@ -0,0 +1 @@
+RTNETLINK answers: Operation not supported
./system-layer3-tunnels.at:25: exit code was 2, expected 0
I think my system missed something so “ip route add 10.1.1.2/32 encap
ip id 0 dst 172.31.1.100 dev at_vxlan1 “ failed, Eric, what linux distribution
do you know I can run “ping over VXLAN-GPE” successfully, I’ll use it as
baseline to add NSH unit test for kernel data path.
When I added the tests it was on RHEL-7.4 with a net-next kernel. It should
skip the tests if the installed iproute2 does not have the options for GPE.
The error you're seeing likely means your kernel does not have GPE support.
VXLAN-GPE first appeared in kernel 4.7.
e1e5314de08b ("vxlan: implement GPE")
As such, I think the VXLAN-GPE test case should pass on any distro with a 4.7+
kernel.
From: Gregory Rose [mailto:gvrose8...@gmail.com]
Sent: Tuesday, January 30, 2018 1:51 AM
To: Yang, Yi Y ; d...@openvswitch.org
Cc: b...@ovn.org; jan.scheur...@ericsson.com
Subject: Re: [PATCH v1 0/5] datapath: enable NSH support in kernel
compat mode
On 1/10/2018 11:53 PM, Yi Yang wrote:
This patch series is to backport NSH support patches in Linux net-next
tree
to OVS in order that it can support NSH in kernel compat mode.
Yi Yang (5):
datapath: ether: add NSH ethertype
datapath: vxlan: factor out VXLAN-GPE next protocol
datapath: net: add NSH header structures and helpers
datapath: nsh: add GSO support
datapath: enable NSH support
NEWS | 1 +
datapath/Modules.mk | 4 +-
datapath/actions.c| 116
datapath/datapath.c | 4 +
datapath/flow.c | 51
datapath/flow.h | 7 +
datapath/flow_netlink.c | 343 +-
datapath/flow_netlink.h | 5 +
datapath/linux/Modules.mk | 2 +
datapath/linux/compat/include/linux/if_ether.h| 4 +
datapath/linux/compat/include/linux/openvswitch.h | 6 +-
datapath/linux/compat/include/net/nsh.h | 313
datapath/linux/compat/include/net/tun_proto.h | 49
datapath/linux/compat/include/net/vxlan.h | 6 -
datapath/linux/compat/vxlan.c | 32 +-
datapath/nsh.c| 142 +
16 files changed, 1048 insertions(+), 37 deletions(-)
create mode 100644 datapath/linux/compat/include/net/nsh.h
create mode 100644 datapath/linux/compat/include/net/tun_proto.h
create mode 100644 datapath/nsh.c
Hi Yi,
My apologies for the delay in reviewing this series.
I've finished up my review and I think it mostly looks pretty good but I did
find an issue compiling on a 3.10.107 kernel build:
CC [M]
/home/travis/build/gvrose8192/ovs-experimental/datapath/linux/vport-ne
tdev.o
/home/travis/build/gvrose8192/ovs-experimental/datapath/linux/nsh.c:108:17:
error: undefined identifier 'skb_gso_er
Re: [ovs-dev] OVS-DPDK public meeting
31th January 2018 Attendees: Flavio, Johann, Olga, Aaron, Ben, Michael, Jan, Ian, Frikkie, Sugesh, Yipeng === GENERAL === OVS 2.9 Release - New features deadline for 2.9: 31/Jan. - Patches on DPDK_MERGE_2_9 -- VHost Zero Copy (Experimental) Potential patches for DPDK_MERGE_2_9 Outstanding bug fixes rhel: Fix support for root user using DPDK. Docs: mempool requirements & limitations. DPDK mempool model OVS 2.9 introduces mempool per port model. Issues raised in relation to memory requirements when upgrading from previous OVS versions https://mail.openvswitch.org/pipermail/ovs-discuss/2018-January/046083.html Next steps -- Flavio ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] openvswitch: meter: Use 64-bit arithmetic instead of 32-bit
From: "Gustavo A. R. Silva"
Date: Tue, 30 Jan 2018 22:55:33 -0600
> Add suffix LL to constant 1000 in order to give the compiler
> complete information about the proper arithmetic to use. Notice
> that this constant is used in a context that expects an expression
> of type long long int (64 bits, signed).
>
> The expression (band->burst_size + band->rate) * 1000 is currently
> being evaluated using 32-bit arithmetic.
>
> Addresses-Coverity-ID: 1461563 ("Unintentional integer overflow")
> Signed-off-by: Gustavo A. R. Silva
Applied.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v13] netdev-dpdk: Add support for vHost dequeue zero copy (experimental)
> -Original Message- > From: Loftus, Ciara > Sent: Wednesday, January 31, 2018 10:45 AM > To: d...@openvswitch.org > Cc: Loftus, Ciara ; Stokes, Ian > > Subject: [PATCH v13] netdev-dpdk: Add support for vHost dequeue zero copy > (experimental) > > Zero copy is disabled by default. To enable it, set the 'dq-zero-copy' > option to 'true' when configuring the Interface: > > ovs-vsctl set Interface dpdkvhostuserclient0 > options:vhost-server-path=/tmp/dpdkvhostuserclient0 > options:dq-zero-copy=true > > When packets from a vHost device with zero copy enabled are destined for a > single 'dpdk' port, the number of tx descriptors on that 'dpdk' port must > be set to a smaller value. 128 is recommended. This can be achieved like > so: > > ovs-vsctl set Interface dpdkport options:n_txq_desc=128 > > Note: The sum of the tx descriptors of all 'dpdk' ports the VM will send > to should not exceed 128. Due to this requirement, the feature is > considered 'experimental'. > > Testing of the patch showed a ~8% improvement when switching 512B packets > between vHost devices on different VMs on the same host when zero copy was > enabled on the transmitting device. > > Signed-off-by: Ciara Loftus > Acked-by: Ilya Maximets Thanks all for the reviews and work on this. I pushed this to DPDK_MERGE and DPDK_MERGE_2_9. Thanks Ian > --- > v13: > * Rebase > * Update commit message with recent performance measurement > > Documentation/intro/install/dpdk.rst | 2 + > Documentation/topics/dpdk/vhost-user.rst | 73 > > NEWS | 1 + > lib/netdev-dpdk.c| 18 > vswitchd/vswitch.xml | 11 + > 5 files changed, 105 insertions(+) > > diff --git a/Documentation/intro/install/dpdk.rst > b/Documentation/intro/install/dpdk.rst > index 8d5ee56..ed358d5 100644 > --- a/Documentation/intro/install/dpdk.rst > +++ b/Documentation/intro/install/dpdk.rst > @@ -518,6 +518,8 @@ The above command sets the number of rx queues for > DPDK physical interface. > The rx queues are assigned to pmd threads on the same NUMA node in a > round-robin fashion. > > +.. _dpdk-queues-sizes: > + > DPDK Physical Port Queue Sizes > ~~~ > > diff --git a/Documentation/topics/dpdk/vhost-user.rst > b/Documentation/topics/dpdk/vhost-user.rst > index 8447e2d..95517a6 100644 > --- a/Documentation/topics/dpdk/vhost-user.rst > +++ b/Documentation/topics/dpdk/vhost-user.rst > @@ -458,3 +458,76 @@ Sample XML > > > .. _QEMU documentation: http://git.qemu- > project.org/?p=qemu.git;a=blob;f=docs/specs/vhost- > user.txt;h=7890d7169;hb=HEAD > + > +vhost-user Dequeue Zero Copy (experimental) > +--- > + > +Normally when dequeuing a packet from a vHost User device, a memcpy > +operation must be used to copy that packet from guest address space to > +host address space. This memcpy can be removed by enabling dequeue zero- > copy like so:: > + > +$ ovs-vsctl add-port br0 dpdkvhostuserclient0 -- set Interface \ > +dpdkvhostuserclient0 type=dpdkvhostuserclient \ > +options:vhost-server-path=/tmp/dpdkvhostclient0 \ > +options:dq-zero-copy=true > + > +With this feature enabled, a reference (pointer) to the packet is > +passed to the host, instead of a copy of the packet. Removing this > +memcpy can give a performance improvement for some use cases, for > +example switching large packets between different VMs. However additional > packet loss may be observed. > + > +Note that the feature is disabled by default and must be explicitly > +enabled by setting the ``dq-zero-copy`` option to ``true`` while > +specifying the ``vhost-server-path`` option as above. If you wish to > +split out the command into multiple commands as below, ensure > +``dq-zero-copy`` is set before > +``vhost-server-path``:: > + > +$ ovs-vsctl set Interface dpdkvhostuserclient0 options:dq-zero- > copy=true > +$ ovs-vsctl set Interface dpdkvhostuserclient0 \ > +options:vhost-server-path=/tmp/dpdkvhostclient0 > + > +The feature is only available to ``dpdkvhostuserclient`` port types. > + > +A limitation exists whereby if packets from a vHost port with > +``dq-zero-copy=true`` are destined for a ``dpdk`` type port, the number > +of tx descriptors (``n_txq_desc``) for that port must be reduced to a > +smaller number, > +128 being the recommended value. This can be achieved by issuing the > +following > +command:: > + > +$ ovs-vsctl set Interface dpdkport options:n_txq_desc=128 > + > +Note: The sum of the tx descriptors of all ``dpdk`` ports the VM will > +send to should not exceed 128. For example, in case of a bond over two > +physical ports in balance-tcp mode, one must divide 128 by the number of > links in the bond. > + > +Refer to :ref:`dpdk-queues-sizes` for more information. > + > +The reason for this limitation is due to how the zero copy > +functiona
Re: [ovs-dev] [PATCH v1 0/5] datapath: enable NSH support in kernel compat mode
Hi, Eric
My kernel is 4.13.0-rc6, so I'm very sure it can support vxlan-gpe, I can add
vxlan-gpe port without any issue by command line, so I don't know what happened.
Greg, I checked unit tests in tests/system-traffic.at and
tests/system-layer3-tunnels.at for vxlan and vxlan-gpe, I think they are very
tricky, for NSH, they won't work, current test infrastructure can't handle NSH
unit test in kernel data path, so I don't think I can add it. I have posted v2
patch series, I have sfc test environment in my machine and have verified
kernel data path, my test environment has two vagrant VMs, each one VM starts
two containers which play SF roles, end-to-end ping and http are both ok.
-Original Message-
From: Eric Garver [mailto:e...@erig.me]
Sent: Tuesday, January 30, 2018 9:53 PM
To: Yang, Yi Y
Cc: Gregory Rose ; d...@openvswitch.org
Subject: Re: [ovs-dev] [PATCH v1 0/5] datapath: enable NSH support in kernel
compat mode
On Tue, Jan 30, 2018 at 11:33:55AM +, Yang, Yi Y wrote:
> Hi, Greg
>
> I installed linux 3.10.107 in Ubuntu 14.04 and fixed
> skb_gso_error_unwind issue, but for unit test,
> tests/system-layer3-tunnels.at is a good reference for it because we
> used vxlan-gpe for nsh, I ran unit test 90, but it always fails (I
> have installed and used net-next kernel and the latest iproute2)
>
> Here is error log
>
> ./system-layer3-tunnels.at:25: ip netns exec at_ns0 sh <<
> NS_EXEC_HEREDOC ip route add 10.1.1.2/32 encap ip id 0 dst
> 172.31.1.100 dev at_vxlan1 NS_EXEC_HEREDOC
> --- /dev/null 2018-01-30 02:18:43.272647961 +
> +++
> /home/vagrant/ovs-nsh-test/tests/system-kmod-testsuite.dir/at-groups/90/stderr
> 2018-01-30 09:45:15.415934534 +
> @@ -0,0 +1 @@
> +RTNETLINK answers: Operation not supported
> ./system-layer3-tunnels.at:25: exit code was 2, expected 0
>
> I think my system missed something so “ip route add 10.1.1.2/32 encap
> ip id 0 dst 172.31.1.100 dev at_vxlan1 “ failed, Eric, what linux
> distribution do you know I can run “ping over VXLAN-GPE” successfully, I’ll
> use it as baseline to add NSH unit test for kernel data path.
When I added the tests it was on RHEL-7.4 with a net-next kernel. It should
skip the tests if the installed iproute2 does not have the options for GPE.
The error you're seeing likely means your kernel does not have GPE support.
VXLAN-GPE first appeared in kernel 4.7.
e1e5314de08b ("vxlan: implement GPE")
As such, I think the VXLAN-GPE test case should pass on any distro with a 4.7+
kernel.
>
> From: Gregory Rose [mailto:gvrose8...@gmail.com]
> Sent: Tuesday, January 30, 2018 1:51 AM
> To: Yang, Yi Y ; d...@openvswitch.org
> Cc: b...@ovn.org; jan.scheur...@ericsson.com
> Subject: Re: [PATCH v1 0/5] datapath: enable NSH support in kernel
> compat mode
>
> On 1/10/2018 11:53 PM, Yi Yang wrote:
>
>
> This patch series is to backport NSH support patches in Linux net-next
> tree
>
> to OVS in order that it can support NSH in kernel compat mode.
>
>
>
> Yi Yang (5):
>
> datapath: ether: add NSH ethertype
>
> datapath: vxlan: factor out VXLAN-GPE next protocol
>
> datapath: net: add NSH header structures and helpers
>
> datapath: nsh: add GSO support
>
> datapath: enable NSH support
>
>
>
> NEWS | 1 +
>
> datapath/Modules.mk | 4 +-
>
> datapath/actions.c| 116
>
> datapath/datapath.c | 4 +
>
> datapath/flow.c | 51
>
> datapath/flow.h | 7 +
>
> datapath/flow_netlink.c | 343
> +-
>
> datapath/flow_netlink.h | 5 +
>
> datapath/linux/Modules.mk | 2 +
>
> datapath/linux/compat/include/linux/if_ether.h| 4 +
>
> datapath/linux/compat/include/linux/openvswitch.h | 6 +-
>
> datapath/linux/compat/include/net/nsh.h | 313
>
> datapath/linux/compat/include/net/tun_proto.h | 49
>
> datapath/linux/compat/include/net/vxlan.h | 6 -
>
> datapath/linux/compat/vxlan.c | 32 +-
>
> datapath/nsh.c| 142 +
>
> 16 files changed, 1048 insertions(+), 37 deletions(-)
>
> create mode 100644 datapath/linux/compat/include/net/nsh.h
>
> create mode 100644 datapath/linux/compat/include/net/tun_proto.h
>
> create mode 100644 datapath/nsh.c
>
>
>
> Hi Yi,
>
> My apologies for the delay in reviewing this series.
>
> I've finished up my review and I think it mostly looks pretty good but I did
> find an issue compiling on a 3.10.107 kernel build:
> CC [M]
> /home/travis/build/gvrose8192/ovs-experimental/datapath/linux/vport-ne
> tdev.o
> /home/travis/build/gvrose8192/ovs-experimental/datapath/linux/nsh.c:108:17:
> error: undefined ide
[ovs-dev] [PATCH v2 5/5] datapath: enable NSH support
Upstream commit:
commit b2d0f5d5dc53532e6f07bc546a476a55ebdfe0f3
Author: Yi Yang
Date: Tue Nov 7 21:07:02 2017 +0800
openvswitch: enable NSH support
OVS master and 2.8 branch has merged NSH userspace
patch series, this patch is to enable NSH support
in kernel data path in order that OVS can support
NSH in compat mode by porting this.
Signed-off-by: Yi Yang
Acked-by: Jiri Benc
Acked-by: Eric Garver
Acked-by: Pravin Shelar
Signed-off-by: David S. Miller
Signed-off-by: Yi Yang
---
NEWS | 1 +
acinclude.m4 | 1 +
datapath/actions.c| 116
datapath/flow.c | 51
datapath/flow.h | 7 +
datapath/flow_netlink.c | 343 +-
datapath/flow_netlink.h | 5 +
datapath/linux/compat/include/linux/netdevice.h | 14 +
datapath/linux/compat/include/linux/openvswitch.h | 6 +-
datapath/linux/compat/include/net/nsh.h | 3 +
datapath/nsh.c| 142 +
11 files changed, 684 insertions(+), 5 deletions(-)
create mode 100644 datapath/nsh.c
diff --git a/NEWS b/NEWS
index 726589c..1b4795a 100644
--- a/NEWS
+++ b/NEWS
@@ -9,6 +9,7 @@ v2.9.0 - xx xxx
- NSH implementation now conforms to latest draft (draft-ietf-sfc-nsh-28).
* Add ttl field.
* Add a new action dec_nsh_ttl.
+ * Enable NSH support in kernel datapath.
- OVSDB:
* New high-level documentation in ovsdb(7).
* New file format documentation for developers in ovsdb(5).
diff --git a/acinclude.m4 b/acinclude.m4
index c04c2c6..bc8be46 100644
--- a/acinclude.m4
+++ b/acinclude.m4
@@ -543,6 +543,7 @@ AC_DEFUN([OVS_CHECK_LINUX_COMPAT], [
OVS_GREP_IFELSE([$KSRC/include/linux/netdevice.h], [dev_get_by_index_rcu])
OVS_GREP_IFELSE([$KSRC/include/linux/netdevice.h], [dev_recursion_level])
OVS_GREP_IFELSE([$KSRC/include/linux/netdevice.h], [__skb_gso_segment])
+ OVS_GREP_IFELSE([$KSRC/include/linux/netdevice.h], [skb_gso_error_unwind])
OVS_GREP_IFELSE([$KSRC/include/linux/netdevice.h], [can_checksum_protocol])
OVS_GREP_IFELSE([$KSRC/include/linux/netdevice.h], [ndo_get_iflink])
OVS_GREP_IFELSE([$KSRC/include/linux/netdevice.h], [ndo_features_check],
diff --git a/datapath/actions.c b/datapath/actions.c
index 1840fe5..eab1476 100644
--- a/datapath/actions.c
+++ b/datapath/actions.c
@@ -42,6 +42,7 @@
#include "conntrack.h"
#include "gso.h"
#include "vport.h"
+#include "flow_netlink.h"
static int do_execute_actions(struct datapath *dp, struct sk_buff *skb,
struct sw_flow_key *key,
@@ -385,6 +386,38 @@ static int push_eth(struct sk_buff *skb, struct
sw_flow_key *key,
return 0;
}
+static int push_nsh(struct sk_buff *skb, struct sw_flow_key *key,
+ const struct nshhdr *nh)
+{
+ int err;
+
+ err = ovs_nsh_push(skb, nh);
+ if (err)
+ return err;
+
+ /* safe right before invalidate_flow_key */
+ key->mac_proto = MAC_PROTO_NONE;
+ invalidate_flow_key(key);
+ return 0;
+}
+
+static int pop_nsh(struct sk_buff *skb, struct sw_flow_key *key)
+{
+ int err;
+
+ err = ovs_nsh_pop(skb);
+ if (err)
+ return err;
+
+ /* safe right before invalidate_flow_key */
+ if (skb->protocol == htons(ETH_P_TEB))
+ key->mac_proto = MAC_PROTO_ETHERNET;
+ else
+ key->mac_proto = MAC_PROTO_NONE;
+ invalidate_flow_key(key);
+ return 0;
+}
+
static void update_ip_l4_checksum(struct sk_buff *skb, struct iphdr *nh,
__be32 addr, __be32 new_addr)
{
@@ -608,6 +641,69 @@ static int set_ipv6(struct sk_buff *skb, struct
sw_flow_key *flow_key,
return 0;
}
+static int set_nsh(struct sk_buff *skb, struct sw_flow_key *flow_key,
+ const struct nlattr *a)
+{
+ struct nshhdr *nh;
+ size_t length;
+ int err;
+ u8 flags;
+ u8 ttl;
+ int i;
+
+ struct ovs_key_nsh key;
+ struct ovs_key_nsh mask;
+
+ err = nsh_key_from_nlattr(a, &key, &mask);
+ if (err)
+ return err;
+
+ /* Make sure the NSH base header is there */
+ if (!pskb_may_pull(skb, skb_network_offset(skb) + NSH_BASE_HDR_LEN))
+ return -ENOMEM;
+
+ nh = nsh_hdr(skb);
+ length = nsh_hdr_len(nh);
+
+ /* Make sure the whole NSH header is there */
+ err = skb_ensure_writable(skb, skb_network_offset(skb) +
+ length);
+ if (unlikely(err))
+ return err;
+
+ nh = nsh_hdr(skb);
+ skb_postpull_rcsum(skb, nh, length);
+ flags = nsh_get_flags(nh);
+ flags = OVS_MASKED(flags, key.base.fla
[ovs-dev] [PATCH v2 4/5] datapath: nsh: add GSO support
Upstream commit:
commit c411ed854584a71b0e86ac3019b60e4789d88086
Author: Jiri Benc
Date: Mon Aug 28 21:43:24 2017 +0200
nsh: add GSO support
Add a new nsh/ directory. It currently holds only GSO functions but more
will come: in particular, code shared by openvswitch and tc to manipulate
NSH headers.
For now, assume there's no hardware support for NSH segmentation. We can
always introduce netdev->nsh_features later.
Signed-off-by: Jiri Benc
Signed-off-by: David S. Miller
Signed-off-by: Yi Yang
---
datapath/Modules.mk | 4 +++-
datapath/datapath.c | 4
datapath/linux/compat/include/net/nsh.h | 3 +++
3 files changed, 10 insertions(+), 1 deletion(-)
diff --git a/datapath/Modules.mk b/datapath/Modules.mk
index 21f04a0..3643da4 100644
--- a/datapath/Modules.mk
+++ b/datapath/Modules.mk
@@ -26,13 +26,15 @@ openvswitch_sources = \
flow_table.c \
vport.c \
vport-internal_dev.c \
- vport-netdev.c
+ vport-netdev.c \
+ nsh.c
vport_geneve_sources = vport-geneve.c
vport_vxlan_sources = vport-vxlan.c
vport_gre_sources = vport-gre.c
vport_lisp_sources = vport-lisp.c
vport_stt_sources = vport-stt.c
+nsh_sources = nsh.c
openvswitch_headers = \
compat.h \
diff --git a/datapath/datapath.c b/datapath/datapath.c
index 1780819..4272227 100644
--- a/datapath/datapath.c
+++ b/datapath/datapath.c
@@ -51,6 +51,7 @@
#include
#include
#include
+#include
#include "datapath.h"
#include "conntrack.h"
@@ -2408,6 +2409,7 @@ static int __init dp_init(void)
pr_info("Open vSwitch switching datapath %s\n", VERSION);
+ ovs_nsh_init();
err = action_fifos_init();
if (err)
goto error;
@@ -2463,6 +2465,7 @@ error_unreg_rtnl_link:
error_action_fifos_exit:
action_fifos_exit();
error:
+ ovs_nsh_cleanup();
return err;
}
@@ -2478,6 +2481,7 @@ static void dp_cleanup(void)
ovs_flow_exit();
ovs_internal_dev_rtnl_link_unregister();
action_fifos_exit();
+ ovs_nsh_cleanup();
}
module_init(dp_init);
diff --git a/datapath/linux/compat/include/net/nsh.h
b/datapath/linux/compat/include/net/nsh.h
index a1eaea2..c9c30e0 100644
--- a/datapath/linux/compat/include/net/nsh.h
+++ b/datapath/linux/compat/include/net/nsh.h
@@ -304,4 +304,7 @@ static inline void nsh_set_flags_ttl_len(struct nshhdr
*nsh, u8 flags,
NSH_FLAGS_MASK | NSH_TTL_MASK | NSH_LEN_MASK);
}
+int ovs_nsh_init(void);
+void ovs_nsh_cleanup(void);
+
#endif /* __NET_NSH_H */
--
2.1.0
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH v2 3/5] datapath: net: add NSH header structures and helpers
Upstream commit: commit 1f0b7744c50573df464ca33d8e5275be509f852b Author: Yi Yang Date: Mon Aug 28 21:43:23 2017 +0200 net: add NSH header structures and helpers NSH (Network Service Header)[1] is a new protocol for service function chaining, it can be handled as a L3 protocol like IPv4 and IPv6, Eth + NSH + Inner packet or VxLAN-gpe + NSH + Inner packet are two typical use cases. This patch adds NSH header structures and helpers for NSH GSO support and Open vSwitch NSH support. [1] https://datatracker.ietf.org/doc/draft-ietf-sfc-nsh/ [Jiri: added nsh_hdr() helper and renamed the header struct to "struct nshhdr" to match the usual pattern. Removed packet type defines, these are now shared with VXLAN-GPE.] Signed-off-by: Yi Yang Signed-off-by: Jiri Benc Signed-off-by: David S. Miller Signed-off-by: Yi Yang --- datapath/linux/Modules.mk | 1 + datapath/linux/compat/include/net/nsh.h | 307 2 files changed, 308 insertions(+) create mode 100644 datapath/linux/compat/include/net/nsh.h diff --git a/datapath/linux/Modules.mk b/datapath/linux/Modules.mk index ac6d4f2..a9955e2 100644 --- a/datapath/linux/Modules.mk +++ b/datapath/linux/Modules.mk @@ -92,6 +92,7 @@ openvswitch_headers += \ linux/compat/include/net/stt.h \ linux/compat/include/net/vrf.h \ linux/compat/include/net/tun_proto.h \ + linux/compat/include/net/nsh.h \ linux/compat/include/net/vxlan.h \ linux/compat/include/net/netfilter/nf_conntrack.h \ linux/compat/include/net/netfilter/nf_conntrack_core.h \ diff --git a/datapath/linux/compat/include/net/nsh.h b/datapath/linux/compat/include/net/nsh.h new file mode 100644 index 000..a1eaea2 --- /dev/null +++ b/datapath/linux/compat/include/net/nsh.h @@ -0,0 +1,307 @@ +#ifndef __NET_NSH_H +#define __NET_NSH_H 1 + +#include + +/* + * Network Service Header: + * 0 1 2 3 + * 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 + * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ + * |Ver|O|U|TTL| Length |U|U|U|U|MD Type| Next Protocol | + * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ + * | Service Path Identifier (SPI)| Service Index | + * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ + * | | + * ~ Mandatory/Optional Context Headers ~ + * | | + * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ + * + * Version: The version field is used to ensure backward compatibility + * going forward with future NSH specification updates. It MUST be set + * to 0x0 by the sender, in this first revision of NSH. Given the + * widespread implementation of existing hardware that uses the first + * nibble after an MPLS label stack for ECMP decision processing, this + * document reserves version 01b and this value MUST NOT be used in + * future versions of the protocol. Please see [RFC7325] for further + * discussion of MPLS-related forwarding requirements. + * + * O bit: Setting this bit indicates an Operations, Administration, and + * Maintenance (OAM) packet. The actual format and processing of SFC + * OAM packets is outside the scope of this specification (see for + * example [I-D.ietf-sfc-oam-framework] for one approach). + * + * The O bit MUST be set for OAM packets and MUST NOT be set for non-OAM + * packets. The O bit MUST NOT be modified along the SFP. + * + * SF/SFF/SFC Proxy/Classifier implementations that do not support SFC + * OAM procedures SHOULD discard packets with O bit set, but MAY support + * a configurable parameter to enable forwarding received SFC OAM + * packets unmodified to the next element in the chain. Forwarding OAM + * packets unmodified by SFC elements that do not support SFC OAM + * procedures may be acceptable for a subset of OAM functions, but can + * result in unexpected outcomes for others, thus it is recommended to + * analyze the impact of forwarding an OAM packet for all OAM functions + * prior to enabling this behavior. The configurable parameter MUST be + * disabled by default. + * + * TTL: Indicates the maximum SFF hops for an SFP. This field is used + * for service plane loop detection. The initial TTL value SHOULD be + * configurable via the control plane; the configured initial value can + * be specific to one or more SFPs. If no initial value is explicitly + * provided, the default initial TTL value of 63 MUST be used. Each SFF + * involved in forwarding an NSH packet MUST decrement the TTL value by + * 1 prior to NSH forwarding lookup. Decrementing by 1 from an incoming + * value of 0 shall result in a TTL value of 63. The packet MUST NOT be + * forwarded if TTL is, after
[ovs-dev] [PATCH v2 2/5] datapath: vxlan: factor out VXLAN-GPE next protocol
Upstream commit:
commit fa20e0e32cb3dfc1760b6254b64977f2fb5bd851
Author: Jiri Benc
Date: Mon Aug 28 21:43:22 2017 +0200
vxlan: factor out VXLAN-GPE next protocol
The values are shared between VXLAN-GPE and NSH. Originally probably by
coincidence but I notified both working groups about this last year and they
seem to keep the values in sync since then.
Hopefully they'll get a single IANA registry for the values, too. (I asked
them for that.)
Factor out the code to be shared by the NSH implementation.
NSH and MPLS values are added in this patch, too. For MPLS, the drafts
incorrectly assign only a single value, while we have two MPLS ethertypes.
I raised the problem with both groups. For now, I assume the value is for
unicast.
Signed-off-by: Jiri Benc
Signed-off-by: David S. Miller
Signed-off-by: Yi Yang
---
datapath/linux/Modules.mk | 1 +
datapath/linux/compat/include/net/tun_proto.h | 49 +++
datapath/linux/compat/include/net/vxlan.h | 6
datapath/linux/compat/vxlan.c | 32 -
4 files changed, 57 insertions(+), 31 deletions(-)
create mode 100644 datapath/linux/compat/include/net/tun_proto.h
diff --git a/datapath/linux/Modules.mk b/datapath/linux/Modules.mk
index eec9f23..ac6d4f2 100644
--- a/datapath/linux/Modules.mk
+++ b/datapath/linux/Modules.mk
@@ -91,6 +91,7 @@ openvswitch_headers += \
linux/compat/include/net/sock.h \
linux/compat/include/net/stt.h \
linux/compat/include/net/vrf.h \
+ linux/compat/include/net/tun_proto.h \
linux/compat/include/net/vxlan.h \
linux/compat/include/net/netfilter/nf_conntrack.h \
linux/compat/include/net/netfilter/nf_conntrack_core.h \
diff --git a/datapath/linux/compat/include/net/tun_proto.h
b/datapath/linux/compat/include/net/tun_proto.h
new file mode 100644
index 000..2ea3deb
--- /dev/null
+++ b/datapath/linux/compat/include/net/tun_proto.h
@@ -0,0 +1,49 @@
+#ifndef __NET_TUN_PROTO_H
+#define __NET_TUN_PROTO_H
+
+#include
+
+/* One byte protocol values as defined by VXLAN-GPE and NSH. These will
+ * hopefully get a shared IANA registry.
+ */
+#define TUN_P_IPV4 0x01
+#define TUN_P_IPV6 0x02
+#define TUN_P_ETHERNET 0x03
+#define TUN_P_NSH 0x04
+#define TUN_P_MPLS_UC 0x05
+
+static inline __be16 tun_p_to_eth_p(u8 proto)
+{
+ switch (proto) {
+ case TUN_P_IPV4:
+ return htons(ETH_P_IP);
+ case TUN_P_IPV6:
+ return htons(ETH_P_IPV6);
+ case TUN_P_ETHERNET:
+ return htons(ETH_P_TEB);
+ case TUN_P_NSH:
+ return htons(ETH_P_NSH);
+ case TUN_P_MPLS_UC:
+ return htons(ETH_P_MPLS_UC);
+ }
+ return 0;
+}
+
+static inline u8 tun_p_from_eth_p(__be16 proto)
+{
+ switch (proto) {
+ case htons(ETH_P_IP):
+ return TUN_P_IPV4;
+ case htons(ETH_P_IPV6):
+ return TUN_P_IPV6;
+ case htons(ETH_P_TEB):
+ return TUN_P_ETHERNET;
+ case htons(ETH_P_NSH):
+ return TUN_P_NSH;
+ case htons(ETH_P_MPLS_UC):
+ return TUN_P_MPLS_UC;
+ }
+ return 0;
+}
+
+#endif
diff --git a/datapath/linux/compat/include/net/vxlan.h
b/datapath/linux/compat/include/net/vxlan.h
index 460fbf2..18f5474 100644
--- a/datapath/linux/compat/include/net/vxlan.h
+++ b/datapath/linux/compat/include/net/vxlan.h
@@ -204,12 +204,6 @@ reserved_flags2:2;
#define VXLAN_GPE_USED_BITS (VXLAN_HF_VER | VXLAN_HF_NP | VXLAN_HF_OAM | \
cpu_to_be32(0xff))
-/* VXLAN-GPE header Next Protocol. */
-#define VXLAN_GPE_NP_IPV4 0x01
-#define VXLAN_GPE_NP_IPV6 0x02
-#define VXLAN_GPE_NP_ETHERNET 0x03
-#define VXLAN_GPE_NP_NSH 0x04
-
struct vxlan_metadata {
u32 gbp;
};
diff --git a/datapath/linux/compat/vxlan.c b/datapath/linux/compat/vxlan.c
index d27a5e2..54b9534 100644
--- a/datapath/linux/compat/vxlan.c
+++ b/datapath/linux/compat/vxlan.c
@@ -52,6 +52,7 @@
#include
#endif
+#include
#include
#include "gso.h"
#include "vport-netdev.h"
@@ -607,19 +608,9 @@ static bool vxlan_parse_gpe_hdr(struct vxlanhdr *unparsed,
if (gpe->oam_flag)
return false;
- switch (gpe->next_protocol) {
- case VXLAN_GPE_NP_IPV4:
- *protocol = htons(ETH_P_IP);
- break;
- case VXLAN_GPE_NP_IPV6:
- *protocol = htons(ETH_P_IPV6);
- break;
- case VXLAN_GPE_NP_ETHERNET:
- *protocol = htons(ETH_P_TEB);
- break;
- default:
+ *protocol = tun_p_to_eth_p(gpe->next_protocol);
+ if (!*protocol)
return false;
- }
unparsed->vx_flags &= ~VXLAN_GPE_USED_BITS;
return true;
@@ -815,19 +806,10 @@ static int vxlan_build_gpe_hdr(struct vxlanhdr *vxh, u32
vxflags
[ovs-dev] [PATCH v2 1/5] datapath: ether: add NSH ethertype
Upstream commit:
commit 155e6f649757c902901e599c268f8b575ddac1f8
Author: Jiri Benc
Date: Mon Aug 28 21:43:21 2017 +0200
ether: add NSH ethertype
The NSH draft says:
An IEEE EtherType, 0x894F, has been allocated for NSH.
Signed-off-by: Jiri Benc
Signed-off-by: David S. Miller
Signed-off-by: Yi Yang
---
datapath/linux/compat/include/linux/if_ether.h | 4
1 file changed, 4 insertions(+)
diff --git a/datapath/linux/compat/include/linux/if_ether.h
b/datapath/linux/compat/include/linux/if_ether.h
index 5eb99bc..c989c6e 100644
--- a/datapath/linux/compat/include/linux/if_ether.h
+++ b/datapath/linux/compat/include/linux/if_ether.h
@@ -19,6 +19,10 @@
#define ETH_P_8021AD0x88A8 /* 802.1ad Service VLAN */
#endif
+#ifndef ETH_P_NSH
+#define ETH_P_NSH 0x894F /* Network Service Header */
+#endif
+
#define inner_eth_hdr rpl_inner_eth_hdr
static inline struct ethhdr *inner_eth_hdr(const struct sk_buff *skb)
{
--
2.1.0
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH v2 0/5] datapath: enable NSH support in kernel compat mode
v1->v2 - Fix compilation error in linux-3.10.107 This patch series is to backport NSH support patches in Linux net-next tree to OVS in order that it can support NSH in kernel compat mode. Yi Yang (5): datapath: ether: add NSH ethertype datapath: vxlan: factor out VXLAN-GPE next protocol datapath: net: add NSH header structures and helpers datapath: nsh: add GSO support datapath: enable NSH support NEWS | 1 + acinclude.m4 | 1 + datapath/Modules.mk | 4 +- datapath/actions.c| 116 datapath/datapath.c | 4 + datapath/flow.c | 51 datapath/flow.h | 7 + datapath/flow_netlink.c | 343 +- datapath/flow_netlink.h | 5 + datapath/linux/Modules.mk | 2 + datapath/linux/compat/include/linux/if_ether.h| 4 + datapath/linux/compat/include/linux/netdevice.h | 14 + datapath/linux/compat/include/linux/openvswitch.h | 6 +- datapath/linux/compat/include/net/nsh.h | 313 datapath/linux/compat/include/net/tun_proto.h | 49 datapath/linux/compat/include/net/vxlan.h | 6 - datapath/linux/compat/vxlan.c | 32 +- datapath/nsh.c| 142 + 18 files changed, 1063 insertions(+), 37 deletions(-) create mode 100644 datapath/linux/compat/include/net/nsh.h create mode 100644 datapath/linux/compat/include/net/tun_proto.h create mode 100644 datapath/nsh.c -- 2.1.0 ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] Tagging OVS 2.7.4
Does it make sense tagging ovs 2.7.4 stable?. There are several patches which are important for ovn/ networking-ovn since the release of ovs 2.7.3. And I'd be glad to have them available on the upstream builds, so people can start doing more serious testing over RDO/openstack/networking-ovn . Best regards, Miguel Ángel ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH V2] OF1.5/EXT-334 OXS/Extensible Flow Entry Statistics Support
Hi Ben, Are you also agreeing with the Jan's comments. Thanks & Regards Satya Valli Tata Consultancy Services Mailto: satyavalli.r...@tcs.com Website: http://www.tcs.com Experience certainty. IT Services Business Solutions Consulting -Jan Scheurich wrote: - To: Satyavalli Rama , Ben Pfaff From: Jan Scheurich Date: 01/08/2018 06:31PM Cc: SatyaValli , "d...@openvswitch.org" , "manasa.cherukupa...@tcs.com" , "p.pava...@tcs.com" , Harivelam Lavanya , "muttamsetty.su...@tcs.com" Subject: RE: [ovs-dev] [PATCH V2] OF1.5/EXT-334 OXS/Extensible Flow Entry Statistics Support Hi Satyavalli, Please find my responses below. Regards, Jan From: Satyavalli Rama [mailto:satyavalli.r...@tcs.com] Sent: Friday, 05 January, 2018 12:25 To: Ben Pfaff ; Jan Scheurich Cc: SatyaValli ; d...@openvswitch.org; manasa.cherukupa...@tcs.com; p.pava...@tcs.com; Harivelam Lavanya ; muttamsetty.su...@tcs.com Subject: Re: [ovs-dev] [PATCH V2] OF1.5/EXT-334 OXS/Extensible Flow Entry Statistics Support Hi Jan and Ben, Please find the inline responses. -Ben Pfaff wrote: - To: Jan Scheurich From: Ben Pfaff Date: 01/05/2018 02:35AM Cc: SatyaValli , "d...@openvswitch.org" , Manasa Cherukupally , Pavani Panthagada , Lavanya Harivelam , Surya Muttamsetty , SatyaValli Subject: Re: [ovs-dev] [PATCH V2] OF1.5/EXT-334 OXS/Extensible Flow Entry Statistics Support On Wed, Jan 03, 2018 at 04:24:06PM +, Jan Scheurich wrote: > > > > > > > > This Patch provides implementation Existing flow entry statistics are > > > > redefined as standard OXS(OpenFlow Extensible Statistics) fields for > > > > displaying the arbitrary flow stats.The existing Flow Stats were renamed > > > > as Flow Description. > > > > > > > > To support this implementation below messages are newly added > > > > > > > > OFPRAW_OFPT15_FLOW_REMOVED, > > > > OFPRAW_OFPST15_FLOW_REQUEST, > > > > OFPRAW_OFPST15_FLOW_DESC_REQUEST, > > > > OFPRAW_OFPST15_AGGREGATE_REQUEST, > > > > OFPRAW_OFPST15_FLOW_REPLY, > > > > OFPRAW_OFPST15_FLOW_DESC_REPLY, > > > > OFPRAW_OFPST15_AGGREGATE_REPLY, > > > > > > > > The current commit adds support for the new feature in flow statistics > > > > multipart messages,aggregate multipart messages and OXS support for flow > > > > removal message, individual flow description messages. > > > > > > > > "ovs-ofctl dump-flows" needs to be provided with the arbitrary OXS > > > > fields > > > > for displaying the desired flow stats. > > > > > > > > Below are Commands to display OXS stats field wise > > > > > > > > Flow Statistics Multipart > > > > ovs-ofctl dump-flows -O OpenFlow15 idle_time > > > > ovs-ofctl dump-flows -O OpenFlow15 packet_count > > > > ovs-ofctl dump-flows -O OpenFlow15 byte_count > > > > > > This would break backward compatibility for one of the most frequently > > > used OVS CLI commands. Why don't you introduce a new > > command such as "ovs-ofctl dump-flow-stats" for the new OXS stats? > > > > I think you might be misinterpreting the meaning here. It doesn't > > appear to break compatibility, at least not in a major way, since it > > doesn't do a lot of updates to the tests that would otherwise be > > required. > > Perhaps I am missing the point of some of these changes. I understand that > OVS needs to support the new extensible OXS flow stats syntax in OpenFlow 1.5 > and the differentiated MP request/reply pairs OFPMP_FLOW_DESC (replacing the > former OFPMP_FLOW) and OFPMP_FLOW_STATS (just fetching flow stats per flow > w/o the rest of the flow data). > > But I don't understand why this should have any impact on the existing CLI > command "ovs-ofctl dump-flows" and its output. This tool expressly fetches > and displays the complete flow dump from OVS, including match, > instructions/actions and statistics. When using OF 1.5 it should > transparently apply OFPMP_FLOW_DESC MP request/reply to fetch the data, up to > OF 1.4 it should use the original OFPMP_FLOW. > > I can't see any ovs-ofctl use case that would justify the use of the new > OFPMP_FLOW_STATS request/reply. The removed data in the reply compared to the > full flow description are mainly the instructions, the full match is still > there to identify each flow. So cutting down the transferred data volume can > hardly be the reason (Note, this may still be different for real OF 1.5 > controllers). > > If you believe we should have an ovs-ofctl command anyhow, e.g. for testing > purposes, I suggest to introduce a new command or add an option to dump-flows > to force use of this particular MP message. The output would be limited to > flow match and stats in that case. > As per our understanding and from previous review comments we treated OF1.5+ has two different ways to request and get replies for Flow Stats: FLOW_DESC and FLOW_STATS (which will be even used for Flow Stats Trigger). And we've supported this with t
[ovs-dev] [PATCH v13] netdev-dpdk: Add support for vHost dequeue zero copy (experimental)
Zero copy is disabled by default. To enable it, set the 'dq-zero-copy' option to 'true' when configuring the Interface: ovs-vsctl set Interface dpdkvhostuserclient0 options:vhost-server-path=/tmp/dpdkvhostuserclient0 options:dq-zero-copy=true When packets from a vHost device with zero copy enabled are destined for a single 'dpdk' port, the number of tx descriptors on that 'dpdk' port must be set to a smaller value. 128 is recommended. This can be achieved like so: ovs-vsctl set Interface dpdkport options:n_txq_desc=128 Note: The sum of the tx descriptors of all 'dpdk' ports the VM will send to should not exceed 128. Due to this requirement, the feature is considered 'experimental'. Testing of the patch showed a ~8% improvement when switching 512B packets between vHost devices on different VMs on the same host when zero copy was enabled on the transmitting device. Signed-off-by: Ciara Loftus Acked-by: Ilya Maximets --- v13: * Rebase * Update commit message with recent performance measurement Documentation/intro/install/dpdk.rst | 2 + Documentation/topics/dpdk/vhost-user.rst | 73 NEWS | 1 + lib/netdev-dpdk.c| 18 vswitchd/vswitch.xml | 11 + 5 files changed, 105 insertions(+) diff --git a/Documentation/intro/install/dpdk.rst b/Documentation/intro/install/dpdk.rst index 8d5ee56..ed358d5 100644 --- a/Documentation/intro/install/dpdk.rst +++ b/Documentation/intro/install/dpdk.rst @@ -518,6 +518,8 @@ The above command sets the number of rx queues for DPDK physical interface. The rx queues are assigned to pmd threads on the same NUMA node in a round-robin fashion. +.. _dpdk-queues-sizes: + DPDK Physical Port Queue Sizes ~~~ diff --git a/Documentation/topics/dpdk/vhost-user.rst b/Documentation/topics/dpdk/vhost-user.rst index 8447e2d..95517a6 100644 --- a/Documentation/topics/dpdk/vhost-user.rst +++ b/Documentation/topics/dpdk/vhost-user.rst @@ -458,3 +458,76 @@ Sample XML .. _QEMU documentation: http://git.qemu-project.org/?p=qemu.git;a=blob;f=docs/specs/vhost-user.txt;h=7890d7169;hb=HEAD + +vhost-user Dequeue Zero Copy (experimental) +--- + +Normally when dequeuing a packet from a vHost User device, a memcpy operation +must be used to copy that packet from guest address space to host address +space. This memcpy can be removed by enabling dequeue zero-copy like so:: + +$ ovs-vsctl add-port br0 dpdkvhostuserclient0 -- set Interface \ +dpdkvhostuserclient0 type=dpdkvhostuserclient \ +options:vhost-server-path=/tmp/dpdkvhostclient0 \ +options:dq-zero-copy=true + +With this feature enabled, a reference (pointer) to the packet is passed to +the host, instead of a copy of the packet. Removing this memcpy can give a +performance improvement for some use cases, for example switching large packets +between different VMs. However additional packet loss may be observed. + +Note that the feature is disabled by default and must be explicitly enabled +by setting the ``dq-zero-copy`` option to ``true`` while specifying the +``vhost-server-path`` option as above. If you wish to split out the command +into multiple commands as below, ensure ``dq-zero-copy`` is set before +``vhost-server-path``:: + +$ ovs-vsctl set Interface dpdkvhostuserclient0 options:dq-zero-copy=true +$ ovs-vsctl set Interface dpdkvhostuserclient0 \ +options:vhost-server-path=/tmp/dpdkvhostclient0 + +The feature is only available to ``dpdkvhostuserclient`` port types. + +A limitation exists whereby if packets from a vHost port with +``dq-zero-copy=true`` are destined for a ``dpdk`` type port, the number of tx +descriptors (``n_txq_desc``) for that port must be reduced to a smaller number, +128 being the recommended value. This can be achieved by issuing the following +command:: + +$ ovs-vsctl set Interface dpdkport options:n_txq_desc=128 + +Note: The sum of the tx descriptors of all ``dpdk`` ports the VM will send to +should not exceed 128. For example, in case of a bond over two physical ports +in balance-tcp mode, one must divide 128 by the number of links in the bond. + +Refer to :ref:`dpdk-queues-sizes` for more information. + +The reason for this limitation is due to how the zero copy functionality is +implemented. The vHost device's 'tx used vring', a virtio structure used for +tracking used ie. sent descriptors, will only be updated when the NIC frees +the corresponding mbuf. If we don't free the mbufs frequently enough, that +vring will be starved and packets will no longer be processed. One way to +ensure we don't encounter this scenario, is to configure ``n_txq_desc`` to a +small enough number such that the 'mbuf free threshold' for the NIC will be hit +more often and thus free mbufs more frequently. The value of 128 is suggested, +but values of 64 and 256 have been tested
Re: [ovs-dev] [PATCH v2] tests: Add system-dpdk-testsuite
> -Original Message- > From: ovs-dev-boun...@openvswitch.org [mailto:ovs-dev- > boun...@openvswitch.org] On Behalf Of Marcin Rybka > Sent: Tuesday, January 2, 2018 2:36 PM > To: d...@openvswitch.org > Cc: Rybka, MarcinX > Subject: [ovs-dev] [PATCH v2] tests: Add system-dpdk-testsuite > > New OVS-DPDK testsuite, which can be launched via `make check-dpdk`, tests > OVS using a DPDK datapath. The testsuite contains already initial tests: > 1. EAL init > 2. Add standard DPDK PHY port > 3. Add vhost-user-client port > > Signed-off-by: Marcin Rybka > --- > Documentation/topics/testing.rst | 19 > tests/automake.mk| 17 +++ > tests/system-dpdk-macros.at | 54 + > tests/system-dpdk-testsuite.at | 25 > tests/system-dpdk.at | 65 > > 5 files changed, 180 insertions(+) > create mode 100644 tests/system-dpdk-macros.at create mode 100644 > tests/system-dpdk-testsuite.at create mode 100644 tests/system-dpdk.at > > diff --git a/Documentation/topics/testing.rst > b/Documentation/topics/testing.rst > index a49336b..74e0d3f 100644 > --- a/Documentation/topics/testing.rst > +++ b/Documentation/topics/testing.rst > @@ -297,6 +297,25 @@ To invoke the datapath testsuite with the userspace > datapath, run:: > > The results of the testsuite are in ``tests/system-userspace- > testsuite.dir``. > > +DPDK datapath > +' > + > +To test :doc:`/intro/install/dpdk` (i.e., the build was configured with > +``--with-dpdk``,the ``DPDK`` is installed), run the testsuite and > +generate a report by using the ``check-dpdk`` target:: > + > +$ make check-dpdk > + > +To see a list of all the available tests, run:: > + > +$ make check-dpdk TESTSUITEFLAGS=--list > + > +These tests require a `DPDK supported NIC`_ and proper DPDK variables > +(``DPDK_DIR`` and ``DPDK_BUILD``). Moreover you need to load the > +required modules and bind the NIC to the DPDK-compatible driver. > + > +.. _DPDK supported NIC: http://dpdk.org/doc/nics > + Hi Marcin, Thanks for working on this, a few comments inline below. These tests will require elevated privileges (root or sudo) on systems to be run, otherwise they will fail as memory will not be allocated etc. by ovs-dpdk. I'd like to see this called out explicitly in the documentation along with a short explanation (it's obvious to those familiar with DPDK but we must assume a user may not be, also existing ovs unit test can be run without these privileges so someone might work from that assumption). In testing I also see the following error although it does not stop the test from running. set /bin/sh './tests/system-dpdk-testsuite' -C tests AUTOTEST_PATH='utilities:vswitchd:ovsdb:vtep:tests::ovn/controller-vtep:ovn/northd:ovn/utilities:ovn/controller' -j1; \ "$@" || (test X'' = Xyes && "$@" --recheck) Traceback (most recent call last): File "", line 3, in File "/usr/lib64/python2.7/socket.py", line 228, in meth return getattr(self._sock,name)(*args) socket.error: [Errno 99] Cannot assign requested address > Kernel datapath > ''' > > diff --git a/tests/automake.mk b/tests/automake.mk index 8157641..7be5712 > 100644 > --- a/tests/automake.mk > +++ b/tests/automake.mk > @@ -5,10 +5,12 @@ EXTRA_DIST += \ > $(SYSTEM_KMOD_TESTSUITE_AT) \ > $(SYSTEM_USERSPACE_TESTSUITE_AT) \ > $(SYSTEM_OFFLOADS_TESTSUITE_AT) \ > + $(SYSTEM_DPDK_TESTSUITE_AT) \ > $(TESTSUITE) \ > $(SYSTEM_KMOD_TESTSUITE) \ > $(SYSTEM_USERSPACE_TESTSUITE) \ > $(SYSTEM_OFFLOADS_TESTSUITE) \ > + $(SYSTEM_DPDK_TESTSUITE) \ > tests/atlocal.in \ > $(srcdir)/package.m4 \ > $(srcdir)/tests/testsuite \ > @@ -126,6 +128,12 @@ SYSTEM_OFFLOADS_TESTSUITE_AT = \ > tests/system-offloads-traffic.at \ > tests/system-offloads-testsuite.at > > +SYSTEM_DPDK_TESTSUITE_AT = \ > + tests/system-common-macros.at \ > + tests/system-dpdk-macros.at \ > + tests/system-dpdk-testsuite.at \ > + tests/system-dpdk.at > + > check_SCRIPTS += tests/atlocal > > TESTSUITE = $(srcdir)/tests/testsuite > @@ -133,6 +141,7 @@ TESTSUITE_PATCH = $(srcdir)/tests/testsuite.patch > SYSTEM_KMOD_TESTSUITE = $(srcdir)/tests/system-kmod-testsuite > SYSTEM_USERSPACE_TESTSUITE = $(srcdir)/tests/system-userspace-testsuite > SYSTEM_OFFLOADS_TESTSUITE = $(srcdir)/tests/system-offloads-testsuite > +SYSTEM_DPDK_TESTSUITE = $(srcdir)/tests/system-dpdk-testsuite > DISTCLEANFILES += tests/atconfig tests/atlocal > > AUTOTEST_PATH = > utilities:vswitchd:ovsdb:vtep:tests:$(PTHREAD_WIN32_DIR_DLL):ovn/controlle > r-vtep:ovn/northd:ovn/utilities:ovn/controller > @@ -256,6 +265,10 @@ check-offloads: all > set $(SHELL) '$(SYSTEM_OFFLOADS_TESTSUITE)' -C tests > AUTOTEST_PATH='$(AUTOTEST_PATH)' $(TESTSUITEFLAGS) -j1; \ > "$$@" || (test X'$(RECHECK)' = Xyes && "$$@" --recheck) > > +check-dpdk:
Re: [ovs-dev] [PATCH V2] rhel: Fix support for root user using DPDK
Hello,
On 31/01/18 00:07, Marcos Felipe Schwarz wrote:
> diff --git a/lib/daemon-unix.c b/lib/daemon-unix.c
> index adb549c98..06528e9ab 100644
> --- a/lib/daemon-unix.c
> +++ b/lib/daemon-unix.c
> @@ -1047,5 +1047,6 @@ daemon_set_new_user(const char *user_spec)
> }
> }
>
> -switch_user = true;
> +if (!uid_verify(uid) || !gid_verify(gid))
> +switch_user = true;
> }
> diff --git a/rhel/usr_lib_systemd_system_ovs-vswitchd.service.in b/rhel/
> usr_lib_systemd_system_ovs-vswitchd.service.in
> index c6d9aa1b8..e8b81e707 100644
> --- a/rhel/usr_lib_systemd_system_ovs-vswitchd.service.in
> +++ b/rhel/usr_lib_systemd_system_ovs-vswitchd.service.in
> @@ -14,7 +14,7 @@ Environment=HOME=/var/run/openvswitch
> EnvironmentFile=/etc/openvswitch/default.conf
> EnvironmentFile=-/etc/sysconfig/openvswitch
> @begin_dpdk@
> -ExecStartPre=-/usr/bin/chown :hugetlbfs /dev/hugepages
> +ExecStartPre=-/bin/sh -c '/usr/bin/chown :${OVS_USER_ID##*:}
There is something missing^^ here. Where do you apply the chown command?
--
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
