[ovs-dev] GOOGLE Official Winning Notification Letter
Dear Google User,
This is to officially inform you that you have been selected as a winner for
using Google services, we wait
your urgent respond for our official notification letter for the claim of your
winning prize GBP 950,000.00
{Nine Hundred and Fifty Thousand Great British Pounds Sterling}..
You are to contact our Google Coordinator as follows by email for your claim.
Name: Mrs. Patricia Coleman
Email: patriciacolema...@gmail.com
Sincerely.
Larry Page
CEO & Co-founder of Google
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [branch-2.11 1/2] Set release date for 2.11.2.
Hi Justin: Please back-port the above patches to branch-2.10 too as it fails there too. On Thu, Sep 5, 2019 at 4:24 PM Justin Pettit wrote: > > > On Sep 5, 2019, at 4:08 PM, Han Zhou wrote: > > > > Shall older branches support latest kernels? If so, some kernel patches > need to be backported to avoid compile failure. For example, on 2.11, below > patch is required for kernel module compile to pass on Ubuntu with kernel > version 4.15.0-60: > > - datapath: Use new header file net/ipv6_frag.h > > > > For the above patch to be backported without conflict, it requires: > > - datapath: Pass nf_hook_state to nf_conntrack_in() > > - datapath: Handle removal of nf_conntrack_l3proto.h > > - datapath: Fix compiling error for 4.14.111+ kernel > > - datapath: meter: Use struct_size() in kzalloc() > > We support the kernels listed in our releases FAQ for a particular OVS > version: > > http://docs.openvswitch.org/en/latest/faq/releases/ > > 2.11 does indicate that kernel 4.15 should be supported. It looks like > OVS compiles fine against 4.15.0-58, but something changed in 4.15.0-60. I > went ahead and pulled the patches you recommended and pushed them to > branch-2.11. Thanks for figuring it out. > > I'll skip the 2.11.2 release announcement and just jump to 2.11.3. > > Thanks again! > > --Justin > > > ___ > 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] [branch-2.11 1/2] Set release date for 2.11.2.
> On Sep 5, 2019, at 4:08 PM, Han Zhou wrote: > > Shall older branches support latest kernels? If so, some kernel patches need > to be backported to avoid compile failure. For example, on 2.11, below patch > is required for kernel module compile to pass on Ubuntu with kernel version > 4.15.0-60: > - datapath: Use new header file net/ipv6_frag.h > > For the above patch to be backported without conflict, it requires: > - datapath: Pass nf_hook_state to nf_conntrack_in() > - datapath: Handle removal of nf_conntrack_l3proto.h > - datapath: Fix compiling error for 4.14.111+ kernel > - datapath: meter: Use struct_size() in kzalloc() We support the kernels listed in our releases FAQ for a particular OVS version: http://docs.openvswitch.org/en/latest/faq/releases/ 2.11 does indicate that kernel 4.15 should be supported. It looks like OVS compiles fine against 4.15.0-58, but something changed in 4.15.0-60. I went ahead and pulled the patches you recommended and pushed them to branch-2.11. Thanks for figuring it out. I'll skip the 2.11.2 release announcement and just jump to 2.11.3. Thanks again! --Justin ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [branch-2.11 1/2] Set release date for 2.11.2.
On Tue, Sep 3, 2019 at 3:10 PM Justin Pettit wrote: > > Signed-off-by: Justin Pettit > --- > NEWS | 3 ++- > debian/changelog | 2 +- > 2 files changed, 3 insertions(+), 2 deletions(-) > > diff --git a/NEWS b/NEWS > index ff2647739375..f2d44e4630bb 100644 > --- a/NEWS > +++ b/NEWS > @@ -1,5 +1,6 @@ > -v2.11.2 - xx xxx > +v2.11.2 - 03 Sep 2019 > > + - Bug fixes > - DPDK > * OVS validated with DPDK 18.11.2 which is recommended to be used. > > diff --git a/debian/changelog b/debian/changelog > index 1c3f40bad58d..25763e831b68 100644 > --- a/debian/changelog > +++ b/debian/changelog > @@ -2,7 +2,7 @@ openvswitch (2.11.2-1) unstable; urgency=low > [ Open vSwitch team ] > * New upstream version > > - -- Open vSwitch team Sat, 30 Mar 2019 10:46:10 -0700 > + -- Open vSwitch team Tue, 03 Sep 2019 15:05:42 -0700 > > openvswitch (2.11.1-1) unstable; urgency=low > [ Open vSwitch team ] > -- > 2.17.1 > > ___ > dev mailing list > d...@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev Hi Justin, Shall older branches support latest kernels? If so, some kernel patches need to be backported to avoid compile failure. For example, on 2.11, below patch is required for kernel module compile to pass on Ubuntu with kernel version 4.15.0-60: - datapath: Use new header file net/ipv6_frag.h For the above patch to be backported without conflict, it requires: - datapath: Pass nf_hook_state to nf_conntrack_in() - datapath: Handle removal of nf_conntrack_l3proto.h - datapath: Fix compiling error for 4.14.111+ kernel - datapath: meter: Use struct_size() in kzalloc() Thanks, Han ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH 2/2] netdev-afxdp: Add need_wakeup supprt.
On Wed, Sep 4, 2019 at 7:10 AM Ilya Maximets wrote: > > > Hi Eelco, > > > > Thanks for your testing and review. > > > > On Wed, Sep 4, 2019 at 1:04 AM Eelco Chaudron > > wrote: > >> > >> > >> > >> On 27 Aug 2019, at 1:02, William Tu wrote: > >> > >> > The patch adds support for using need_wakeup flag in AF_XDP rings. > >> > When this flag is used, it means that OVS has to explicitly wake > >> > up the kernel RX, using poll() syscall and wake up TX, using sendto() > >> > syscall. This feature improves the performance by avoiding unnecessary > >> > syscalls, so keeping more CPU time in userspace to process packets. > >> > > >> > On Intel Xeon E5-2620 v3 2.4GHz system, performance of physical port > >> > to physical port improves from 6.1Mpps to 7.3Mpps. > >> > >> Did some more testing and with PVP I see a performance decrease, with > >> physical to physical I see an increase. > >> Tests are performed with a port redirect open flow rule on an ixgbe > >> (Xeon E5-2690 v4 2.60GHz): > >> > >> +---+-+-+-+-+-+-+-++ > >> | PVP | Number of flows | 64| 128 | 256 | 512 | > >> 768 | 1024 | 1514 | > >> +---+-+-+-+-+-+-+-++ > >> | master|1000 | 737896 | 700643 | 682915 | 648386 | > >> 621792 | 582821 | 527899 | > >> | Patch |1000 | 734179 | 696515 | 676712 | 646536 | > >> 619600 | 578546 | 519965 | > >> +---+-+-+-+-+-+-+-++ > >> > >> +---+-+-+-+-+-+-+-++ > >> | Port2Port | Number of flows | 64| 128 | 256 | 512 | > >> 768 | 1024 | 1514 | > >> +---+-+-+-+-+-+-+-++ > >> | master|1000 | 3351114 | 3236581 | 3143710 | 2349598 | > >> 1586276 | 1197304 | 814854 | > >> | Patch |1000 | 3571733 | 3488697 | 3448908 | 2349593 | > >> 1586276 | 1197304 | 814854 | > >> +---+-+-+-+-+-+-+-++ > >> > >> Did not research why PVP is slower, maybe related to the TAP interface > >> with AF_XDP? > >> > > I haven't tried PVP with this feature. > > Maybe for virtual device, we don't need "need_wakeup" feature. > > Let me investigate more. > > For virtual devices xmit is synchronous, so it always needs wakeup, i.e. > flag is always set. In my spare time I'm working on kernel thread to > poll tx queue in SKB mode by the analogue with SQ_POLL in io_uring, but > I'm not sure if it will have good performance impact. > Cool. If there is a kthread polling the SKB mode tx queue, then need_wakeup feature can be enabled for virtual device. This will save sendto syscall overhead and improve performance. William ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCHv2] netdev-afxdp: Add need_wakeup supprt.
The patch adds support for using need_wakeup flag in AF_XDP rings.
A new option, use_need_wakeup, is added. When this option is used,
it means that OVS has to explicitly wake up the kernel RX, using poll()
syscall and wake up TX, using sendto() syscall. This feature improves
the performance by avoiding unnecessary sendto syscalls for TX.
For RX, instead of kernel always busy-spinning on fille queue, OVS wakes
up the kernel RX processing when fill queue is replenished.
The need_wakeup feature is merged into Linux kernel 5.3.0-rc1 and OVS
enables it by default. Running the feature before this version causes
xsk bind fails, please use options:use_need_wakeup=false to disable it.
For virtual interface, it's better set use_need_wakeup=false, since
the virtual device's AF_XDP xmit is synchronous: the sendto syscall
enters kernel and process the TX packet on tx queue directly.
I tested on kernel 5.3.0-rc3 using its libbpf. On Intel Xeon E5-2620
v3 2.4GHz system, performance of physical port to physical port improves
from 6.1Mpps to 7.3Mpps. Testing on 5.2.0-rc6 using libbpf from 5.3.0-rc3
does not work due to libbpf API change. Users have to use the older
libbpf for older kernel.
Suggested-by: Ilya Maximets
Signed-off-by: William Tu
---
v2:
- address feedbacks from Ilya and Eelco
- add options:use_need_wakeup, default to true
- remove poll timeout=1sec, make poll() return immediately
- naming change: rename to xsk_rx_wakeup_if_needing
- fix indents and return value for errno
---
Documentation/intro/install/afxdp.rst | 14 --
acinclude.m4 | 5 ++
lib/netdev-afxdp.c| 94 +++
lib/netdev-linux-private.h| 2 +
vswitchd/vswitch.xml | 13 +
5 files changed, 114 insertions(+), 14 deletions(-)
diff --git a/Documentation/intro/install/afxdp.rst
b/Documentation/intro/install/afxdp.rst
index 820e9d993d8f..b7a3d0b7f57b 100644
--- a/Documentation/intro/install/afxdp.rst
+++ b/Documentation/intro/install/afxdp.rst
@@ -176,9 +176,17 @@ in :doc:`general`::
ovs-vswitchd ...
ovs-vsctl -- add-br br0 -- set Bridge br0 datapath_type=netdev
-Make sure your device driver support AF_XDP, and to use 1 PMD (on core 4)
-on 1 queue (queue 0) device, configure these options: **pmd-cpu-mask,
-pmd-rxq-affinity, and n_rxq**. The **xdpmode** can be "drv" or "skb"::
+Make sure your device driver support AF_XDP, netdev-afxdp supports
+the following additional options (see man ovs-vswitchd.conf.db for
+more details):
+
+ * **xdpmode**: use "drv" for driver mode, or "skb" for skb mode.
+
+ * **use_need_wakeup**: enable by setting to "true", otherwise "false"
+
+For example, to use 1 PMD (on core 4) on 1 queue (queue 0) device,
+configure these options: **pmd-cpu-mask, pmd-rxq-affinity, and n_rxq**.
+The **xdpmode** can be "drv" or "skb"::
ethtool -L enp2s0 combined 1
ovs-vsctl set Open_vSwitch . other_config:pmd-cpu-mask=0x10
diff --git a/acinclude.m4 b/acinclude.m4
index f0e38898b17a..3f24475c96b5 100644
--- a/acinclude.m4
+++ b/acinclude.m4
@@ -276,6 +276,11 @@ AC_DEFUN([OVS_CHECK_LINUX_AF_XDP], [
[Define to 1 if AF_XDP support is available and enabled.])
LIBBPF_LDADD=" -lbpf -lelf"
AC_SUBST([LIBBPF_LDADD])
+
+AC_CHECK_DECL([xsk_ring_prod__needs_wakeup], [
+ AC_DEFINE([HAVE_XDP_NEED_WAKEUP], [1],
+[XDP need wakeup support detected in xsk.h.])
+], [], [#include ])
fi
AM_CONDITIONAL([HAVE_AF_XDP], test "$AF_XDP_ENABLE" = true)
])
diff --git a/lib/netdev-afxdp.c b/lib/netdev-afxdp.c
index e5b058d08a09..5169ed3ff801 100644
--- a/lib/netdev-afxdp.c
+++ b/lib/netdev-afxdp.c
@@ -26,6 +26,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -82,7 +83,7 @@ BUILD_ASSERT_DECL(PROD_NUM_DESCS == CONS_NUM_DESCS);
#define UMEM2DESC(elem, base) ((uint64_t)((char *)elem - (char *)base))
static struct xsk_socket_info *xsk_configure(int ifindex, int xdp_queue_id,
- int mode);
+ int mode, bool use_need_wakeup);
static void xsk_remove_xdp_program(uint32_t ifindex, int xdpmode);
static void xsk_destroy(struct xsk_socket_info *xsk);
static int xsk_configure_all(struct netdev *netdev);
@@ -117,6 +118,49 @@ struct xsk_socket_info {
atomic_uint64_t tx_dropped;
};
+#ifdef HAVE_XDP_NEED_WAKEUP
+static inline void
+xsk_rx_wakeup_if_needed(struct xsk_umem_info *umem,
+struct netdev *netdev, int fd)
+{
+struct pollfd pfd;
+int ret;
+
+if (xsk_ring_prod__needs_wakeup(>fq)) {
+pfd.fd = fd;
+pfd.events = POLLIN;
+
+ret = poll(, 1, 0);
+if (OVS_UNLIKELY(ret < 0)) {
+VLOG_WARN_RL(, "%s: error polling rx fd: %s.",
+ netdev_get_name(netdev),
+ ovs_strerror(errno));
+}
+}
+}
+
+static inline bool
+xsk_tx_need_wakeup(struct
Re: [ovs-dev] [branch-2.5 1/2] Set release date for 2.5.9.
All of these 2.5 to 2.8 release series look good to me. Acked-by: Ben Pfaff ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v1] ofproto: Fix OVS crash when packets hit Openflow rules with certain combinations of nested actions
Hi Ben, Please consider this as a gentle remainder and provide your inputs on the below issue. This issue has been discussed earlier in the following thread. https://mail.openvswitch.org/pipermail/ovs-discuss/2019-April/048494.html Best Regards, Anil Kumar. -Original Message- From: Anil Kumar Koli Sent: Tuesday, 3 September, 2019 11:35 AM To: 'Ilya Maximets' ; 'ovs-dev@openvswitch.org' Cc: 'Ben Pfaff' Subject: RE: [ovs-dev] [PATCH v1] ofproto: Fix OVS crash when packets hit Openflow rules with certain combinations of nested actions Hi Ilya, Sorry for late reply and thanks for the review comments. Yes, the issue happens only in the userspace datapath and not in kernel datapath. I have tested it on kernel datapath and could not see this issue for the same build. Regarding the suggested solution, I still feel like going with recursive mutex approach is more cleaner then releasing the lock for ofproto_packet_out_finish(). We have the ofproto_dpif_xcache_execute() which may lead to race when a flow mod learn is called from another thread. May be it could also be possible that in certain combination we may result the crash in ofproto_packet_out_start(). May be Ben can share his inputs as well. Best Regards,Anil Kumar -Original Message- From: Ilya Maximets Sent: Thursday, 29 August, 2019 05:10 PM To: Anil Kumar Koli ; ovs-dev@openvswitch.org Cc: 'Ben Pfaff' Subject: Re: [ovs-dev] [PATCH v1] ofproto: Fix OVS crash when packets hit Openflow rules with certain combinations of nested actions On 29.08.2019 12:52, Anil Kumar Koli wrote: > Hi Ilya, > > Thanks for the review. > Please find below the stack trace of the crash > > (gdb) bt > #0 0x7f0a3da46c37 in __GI_raise (sig=sig@entry=6) at > ../nptl/sysdeps/unix/sysv/linux/raise.c:56 > #1 0x7f0a3da4a028 in __GI_abort () at abort.c:89 > #2 0x0094262e in ovs_abort_valist (err_no=, > format=, args=args@entry=0x7fffaf59e308) at > lib/util.c:419 > #3 0x009426b7 in ovs_abort (err_no=, > format=format@entry=0xb0289f "%s: pthread_%s_%s failed") at > lib/util.c:411 > #4 0x009104f9 in ovs_mutex_lock_at (l_=l_@entry=0xe47cc0 > , where=where@entry=0xad4199 "ofproto/ofproto.c:5391") > at lib/ovs-thread.c:75 > #5 0x0083fb1f in ofproto_flow_mod_learn > (ofm=ofm@entry=0x7fffaf59e620, keep_ref=, > limit=, below_limitp=below_limitp@entry=0x7fffaf59e540) > at ofproto/ofproto.c:5391 > #6 0x0085e5d0 in xlate_learn_action > (ctx=ctx@entry=0x7fffaf5a02e0, learn=learn@entry=0x2b18308) at > ofproto/ofproto-dpif-xlate.c:5378 > #7 0x008615c3 in do_xlate_actions (ofpacts=, > ofpacts_len=, ctx=, > is_last_action=, group_bucket_action=) > at ofproto/ofproto-dpif-xlate.c:6893 > #8 0x0085edba in xlate_recursively (actions_xlator=0x860730 > , is_last_action=false, deepens=, > rule=0x2b8e440, ctx=0x7fffaf5a02e0) at > ofproto/ofproto-dpif-xlate.c:4233 > #9 xlate_table_action (ctx=0x7fffaf5a02e0, in_port=, > table_id=, may_packet_in=, > honor_table_miss=, with_ct_orig=, > is_last_action=false, > xlator=0x860730 ) at > ofproto/ofproto-dpif-xlate.c:4361 > #10 0x00861aaa in xlate_ofpact_resubmit (resubmit= out>, resubmit=, resubmit=, > is_last_action=, ctx=0x7fffaf5a02e0) at > ofproto/ofproto-dpif-xlate.c:4672 > #11 do_xlate_actions (ofpacts=ofpacts@entry=0x2b654e8, > ofpacts_len=ofpacts_len@entry=48, ctx=ctx@entry=0x7fffaf5a02e0, > is_last_action=is_last_action@entry=true, > group_bucket_action=group_bucket_action@entry=false) > at ofproto/ofproto-dpif-xlate.c:6773 > #12 0x008696d6 in xlate_actions (xin=xin@entry=0x7fffaf5a0da0, > xout=xout@entry=0x7fffaf5a11b0) at ofproto/ofproto-dpif-xlate.c:7570 > #13 0x00859b56 in upcall_xlate (wc=0x7fffaf5a23f0, > odp_actions=0x7fffaf5a1bc0, upcall=0x7fffaf5a1150, udpif=0x2b10670) at > ofproto/ofproto-dpif-upcall.c:1197 > #14 process_upcall (udpif=udpif@entry=0x2b10670, > upcall=upcall@entry=0x7fffaf5a1150, > odp_actions=odp_actions@entry=0x7fffaf5a1bc0, > wc=wc@entry=0x7fffaf5a23f0) at ofproto/ofproto-dpif-upcall.c:1413 > #15 0x0085a9eb in upcall_cb (packet=, > flow=0x7fffaf5a2150, ufid=, pmd_id=, > type=, userdata=, actions=0x7fffaf5a1bc0, > wc=0x7fffaf5a23f0, > put_actions=0x7fffaf5a1c00, aux=0x2b10670) at > ofproto/ofproto-dpif-upcall.c:1315 > #16 0x0088419c in dp_netdev_upcall (pmd=pmd@entry=0x7f0a35f34010, > packet_=packet_@entry=0x2b68930, flow=flow@entry=0x7fffaf5a2150, > wc=wc@entry=0x7fffaf5a23f0, ufid=ufid@entry=0x7fffaf5a1ba0, > type=type@entry=DPIF_UC_MISS, > userdata=userdata@entry=0x0, actions=actions@entry=0x7fffaf5a1bc0, > put_actions=put_actions@entry=0x7fffaf5a1c00) at > lib/dpif-netdev.c:6236 > #17 0x0088d053 in handle_packet_upcall > (put_actions=0x7fffaf5a1c00, actions=0x7fffaf5a1bc0, > key=0x7fffaf5a3080, packet=0x2b68930, pmd=0x7f0a35f34010) at > lib/dpif-netdev.c:6591 > #18 fast_path_processing (pmd=pmd@entry=0x7f0a35f34010, > packets_=packets_@entry=0x7fffaf5a35f0,
[ovs-dev] [PATCH v4] userspace: Enable non-bridge port as tunnel endpoint.
For userspace datapath, currently only the bridge itself, the LOCAL port,
can be the tunnel endpoint to encap/decap tunnel packets. This patch
enables non-bridge port as tunnel endpoint. One use case is for users to
create a bridge and a vtep port as tap, and configure underlay IP at vtep
port as the tunnel endpoint.
This patch causes failure for test "ptap - L3 over patch port". This is
because this test is already using non-bridge port gre1 as tunnel endpoint.
In this test, a flow is added to redirect tunnel packets to gre1 port,
as shown below:
ovs-ofctl add-flow br1 in_port=p1,actions=output=gre1
It later generates a datapath flow which matches an extra eth field:
- recirc_id(0),...,eth_type(0x0800),...
+ recirc_id(0),...,eth(dst=1e:2c:e9:2a:66:9e),eth_type(0x0800),...
With this patch, this flow need only a NORMAL action.
Signed-off-by: William Tu
Co-authored-by: William Tu
Signed-off-by: Yifeng Sun
---
v1->v2: Fixed an error pointed out by Ben.
v2->v3: Fixed a test failure, thanks Ben for review and testing!
v3->v4: When creating v3, a code rebase lead to test number change and
we tested the wrong test. Thanks Ben for catching it. In addition to
this fix, this version also added system test, as suggested by Darrell.
ofproto/ofproto-dpif-xlate.c | 56 +-
tests/packet-type-aware.at | 8 +++---
tests/system-layer3-tunnels.at | 55 +
3 files changed, 104 insertions(+), 15 deletions(-)
diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index 17800f3c8a3f..9c2f44784161 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -3410,6 +3410,19 @@ tnl_route_lookup_flow(const struct xlate_ctx *ctx,
}
}
}
+
+/* If tunnel IP isn't configured on bridges, then we search all ports. */
+HMAP_FOR_EACH (xbridge, hmap_node, >xcfg->xbridges) {
+struct xport *port;
+
+HMAP_FOR_EACH (port, ofp_node, >xports) {
+if (!strncmp(netdev_get_name(port->netdev),
+ out_dev, IFNAMSIZ)) {
+*out_port = port;
+return 0;
+}
+}
+}
return -ENOENT;
}
@@ -3972,6 +3985,16 @@ is_nd_dst_correct(const struct flow *flow, const struct
in6_addr *ipv6_addr)
IN6_ARE_ADDR_EQUAL(>ipv6_dst, ipv6_addr);
}
+static bool
+is_neighbor_reply_matched(const struct flow *flow, struct in6_addr *ip_addr)
+{
+return ((IN6_IS_ADDR_V4MAPPED(ip_addr) &&
+ flow->dl_type == htons(ETH_TYPE_ARP) &&
+ in6_addr_get_mapped_ipv4(ip_addr) == flow->nw_dst) ||
+(!IN6_IS_ADDR_V4MAPPED(ip_addr) &&
+ is_nd_dst_correct(flow, ip_addr)));
+}
+
/* Function verifies if the ARP reply or Neighbor Advertisement represented by
* 'flow' addresses the 'xbridge' of 'ctx'. Returns true if the ARP TA or
* neighbor discovery destination is in the list of configured IP addresses of
@@ -3986,11 +4009,7 @@ is_neighbor_reply_correct(const struct xlate_ctx *ctx,
const struct flow *flow)
/* Verify if 'nw_dst' of ARP or 'ipv6_dst' of ICMPV6 is in the list. */
for (i = 0; xbridge_addr && i < xbridge_addr->n_addr; i++) {
struct in6_addr *ip_addr = _addr->addr[i];
-if ((IN6_IS_ADDR_V4MAPPED(ip_addr) &&
- flow->dl_type == htons(ETH_TYPE_ARP) &&
- in6_addr_get_mapped_ipv4(ip_addr) == flow->nw_dst) ||
-(!IN6_IS_ADDR_V4MAPPED(ip_addr) &&
- is_nd_dst_correct(flow, ip_addr))) {
+if (is_neighbor_reply_matched(flow, ip_addr)) {
/* Found a match. */
ret = true;
break;
@@ -3998,20 +4017,35 @@ is_neighbor_reply_correct(const struct xlate_ctx *ctx,
const struct flow *flow)
}
xbridge_addr_unref(xbridge_addr);
+
+/* If not found in bridge's IPs, search in its ports. */
+if (!ret) {
+struct in6_addr *ip_addr, *mask;
+struct xport *port;
+int error, n_in6;
+
+HMAP_FOR_EACH (port, ofp_node, >xbridge->xports) {
+error = netdev_get_addr_list(port->netdev, _addr,
+ , _in6);
+if (!error && is_neighbor_reply_matched(flow, ip_addr)) {
+/* Found a match. */
+ret = true;
+break;
+}
+}
+}
return ret;
}
static bool
-terminate_native_tunnel(struct xlate_ctx *ctx, ofp_port_t ofp_port,
-struct flow *flow, struct flow_wildcards *wc,
-odp_port_t *tnl_port)
+terminate_native_tunnel(struct xlate_ctx *ctx, struct flow *flow,
+struct flow_wildcards *wc, odp_port_t *tnl_port)
{
*tnl_port = ODPP_NONE;
/* XXX: Write better Filter for tunnel port. We can use in_port
* in tunnel-port flow to avoid these checks completely. */
-if (ofp_port == OFPP_LOCAL &&
-
Re: [ovs-dev] [PATCH 2/2] netdev-afxdp: Add need_wakeup supprt.
> Did some more testing and with PVP I see a performance decrease, with > physical to physical I see an increase. > Tests are performed with a port redirect open flow rule on an ixgbe > (Xeon E5-2690 v4 2.60GHz): > > +---+-+-+-+-+-+-+-++ > | PVP | Number of flows | 64| 128 | 256 | 512 | > 768 | 1024 | 1514 | > +---+-+-+-+-+-+-+-++ > | master|1000 | 737896 | 700643 | 682915 | 648386 | > 621792 | 582821 | 527899 | > | Patch |1000 | 734179 | 696515 | 676712 | 646536 | > 619600 | 578546 | 519965 | > +---+-+-+-+-+-+-+-++ > > +---+-+-+-+-+-+-+-++ > | Port2Port | Number of flows | 64| 128 | 256 | 512 | > 768 | 1024 | 1514 | > +---+-+-+-+-+-+-+-++ > | master|1000 | 3351114 | 3236581 | 3143710 | 2349598 | > 1586276 | 1197304 | 814854 | > | Patch |1000 | 3571733 | 3488697 | 3448908 | 2349593 | > 1586276 | 1197304 | 814854 | > +---+-+-+-+-+-+-+-++ Hi Eleco, I'm wondering why you get only ~3.3Mpps for P2P test. Using ixgbe, I usually get at least 6Mpps using single flow (a single 64B UDP packet hitting single OpenFlow rule) Do you think it's due to have 1000 flows in your setup? Another possible overhead is due to no rxhash in AF_XDP, so there is extra overhead doing flow_hash_5tuple and dp_packet_set_rss_hash Regards, William ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] tests: Add track-origins flag to valgrind.
On Tue, Sep 03, 2019 at 06:07:26PM +0300, Ilya Maximets wrote: > Useful for tracking where the uninitialized memory came from. > Report example: > > Thread 13 revalidator11: > Conditional jump or move depends on uninitialised value(s) > at 0x4C35D96: __memcmp_sse4_1 (in vgpreload_memcheck.so) > by 0x9D4404: ofpbuf_equal (ofpbuf.h:273) > by 0x9D4404: revalidate_ukey__ (ofproto-dpif-upcall.c:2219) > <...> > by 0x6AF488E: clone (clone.S:95) >Uninitialised value was created by a stack allocation > at 0x9D4450: compose_slow_path (ofproto-dpif-upcall.c:1062) > > Signed-off-by: Ilya Maximets Acked-by: Ben Pfaff ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH V2 branch-2.8 1/2] datapath: Properly set L4 keys on "later" IP fragments
On 9/4/2019 10:37 PM, Justin Pettit wrote:
Thanks, Greg. I pushed the patches you sent for all the appropriate branches
(2.5-2.8). I sent out patches to do bug patch releases for all those branches,
too, that will include these. Hopefully we'll get those merged and all the
releases out tomorrow.
--Justin
Awesome! Thanks!
On Sep 4, 2019, at 10:07 AM, Greg Rose wrote:
Upstream commit:
commit ad06a566e118e57b852cab5933dbbbaebb141de3
Author: Greg Rose
Date: Tue Aug 27 07:58:09 2019 -0700
openvswitch: Properly set L4 keys on "later" IP fragments
When IP fragments are reassembled before being sent to conntrack, the
key from the last fragment is used. Unless there are reordering
issues, the last fragment received will not contain the L4 ports, so the
key for the reassembled datagram won't contain them. This patch updates
the key once we have a reassembled datagram.
The handle_fragments() function works on L3 headers so we pull the L3/L4
flow key update code from key_extract into a new function
'key_extract_l3l4'. Then we add a another new function
ovs_flow_key_update_l3l4() and export it so that it is accessible by
handle_fragments() for conntrack packet reassembly.
Co-authored-by: Justin Pettit
Signed-off-by: Greg Rose
Acked-by: Pravin B Shelar
Signed-off-by: David S. Miller
Signed-off-by: Greg Rose
---
V2 - Fix compile errors
---
datapath/conntrack.c | 5 ++
datapath/flow.c | 157 +--
datapath/flow.h | 1 +
3 files changed, 95 insertions(+), 68 deletions(-)
diff --git a/datapath/conntrack.c b/datapath/conntrack.c
index 1990984..2c3e441 100644
--- a/datapath/conntrack.c
+++ b/datapath/conntrack.c
@@ -537,6 +537,11 @@ static int handle_fragments(struct net *net, struct
sw_flow_key *key,
return -EPFNOSUPPORT;
}
+ /* The key extracted from the fragment that completed this datagram
+* likely didn't have an L4 header, so regenerate it.
+*/
+ ovs_flow_key_update_l3l4(skb, key);
+
key->ip.frag = OVS_FRAG_TYPE_NONE;
skb_clear_hash(skb);
skb->ignore_df = 1;
diff --git a/datapath/flow.c b/datapath/flow.c
index 99dae79..083288f 100644
--- a/datapath/flow.c
+++ b/datapath/flow.c
@@ -493,79 +493,14 @@ invalid:
}
/**
- * key_extract - extracts a flow key from an Ethernet frame.
+ * key_extract_l3l4 - extracts L3/L4 header information.
* @skb: sk_buff that contains the frame, with skb->data pointing to the
- * Ethernet header
+ * L3 header
* @key: output flow key
- *
- * The caller must ensure that skb->len >= ETH_HLEN.
- *
- * Returns 0 if successful, otherwise a negative errno value.
- *
- * Initializes @skb header fields as follows:
- *
- *- skb->mac_header: the L2 header.
- *
- *- skb->network_header: just past the L2 header, or just past the
- * VLAN header, to the first byte of the L2 payload.
- *
- *- skb->transport_header: If key->eth.type is ETH_P_IP or ETH_P_IPV6
- * on output, then just past the IP header, if one is present and
- * of a correct length, otherwise the same as skb->network_header.
- * For other key->eth.type values it is left untouched.
- *
- *- skb->protocol: the type of the data starting at skb->network_header.
- * Equals to key->eth.type.
*/
-static int key_extract(struct sk_buff *skb, struct sw_flow_key *key)
+static int key_extract_l3l4(struct sk_buff *skb, struct sw_flow_key *key)
{
int error;
- struct ethhdr *eth;
-
- /* Flags are always used as part of stats */
- key->tp.flags = 0;
-
- skb_reset_mac_header(skb);
-
- /* Link layer. */
- clear_vlan(key);
- if (ovs_key_mac_proto(key) == MAC_PROTO_NONE) {
- if (unlikely(eth_type_vlan(skb->protocol)))
- return -EINVAL;
-
- skb_reset_network_header(skb);
- key->eth.type = skb->protocol;
- } else {
- eth = eth_hdr(skb);
- ether_addr_copy(key->eth.src, eth->h_source);
- ether_addr_copy(key->eth.dst, eth->h_dest);
-
- __skb_pull(skb, 2 * ETH_ALEN);
- /* We are going to push all headers that we pull, so no need to
-* update skb->csum here.
-*/
-
- if (unlikely(parse_vlan(skb, key)))
- return -ENOMEM;
-
- key->eth.type = parse_ethertype(skb);
- if (unlikely(key->eth.type == htons(0)))
- return -ENOMEM;
-
- /* Multiple tagged packets need to retain TPID to satisfy
-* skb_vlan_pop(), which will later shift the ethertype into
-* skb->protocol.
-*/
- if (key->eth.cvlan.tci & htons(VLAN_TAG_PRESENT))
- skb->protocol = key->eth.cvlan.tpid;
- else
-
Re: [ovs-dev] [PATCH] netdev-dpdk: add support for the RTE_ETH_EVENT_INTR_RESET event
See inlines below, and will sent a v2 early next week.
On 5 Sep 2019, at 14:40, Ilya Maximets wrote:
Hi Eelco,
Thanks for the patch! Looks reasonable.
One comment is that it's better to explicitly initialize the
flag in common_construct. I see that we doesn't initialize
'started' flag, but this might be fixed too.
More comments inline.
Best regards, Ilya Maximets.
On 05.09.2019 14:48, Eelco Chaudron wrote:
Currently, OVS does not register and therefore not handle the
interface reset event from the DPDK framework. This would cause a
problem in cases where a VF is used as an interface, and its
configuration changes.
As an example in the following scenario the MAC change is not
detected/acted upon until OVS is restarted without the patch applied:
$ echo 1 > /sys/bus/pci/devices/:05:00.1/sriov_numvfs
$ ovs-vsctl add-port ovs_pvp_br0 dpdk0 -- \
set Interface dpdk0 type=dpdk -- \
set Interface dpdk0 options:dpdk-devargs=:05:0a.0
$ ip link set p5p2 vf 0 mac 52:54:00:92:d3:33
Signed-off-by: Eelco Chaudron
---
lib/netdev-dpdk.c | 49
+--
1 file changed, 47 insertions(+), 2 deletions(-)
diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index bc20d6843..a23150387 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -362,6 +362,7 @@ struct netdev_dpdk {
bool attached;
/* If true, rte_eth_dev_start() was successfully called */
bool started;
+bool reset_needed;
This will produce a hole in the structure as members will no longer
fit into a single cacheline. And cacheline markers will be
misaligned.
See details for similar issue here:
https://mail.openvswitch.org/pipermail/ovs-dev/2019-September/362349.html
Possible solution is to turn existing flags into bit fields or stop
avoiding the problem and drop most of padding in the structure.
Will change this…
struct eth_addr hwaddr;
int mtu;
int socket_id;
@@ -1735,6 +1736,34 @@ netdev_dpdk_process_devargs(struct netdev_dpdk
*dev,
return new_port_id;
}
+static int
+dpdk_eth_event_callback(uint16_t port_id, enum rte_eth_event_type
type,
s/uint16_t/dpdk_port_t/
+void *param OVS_UNUSED, void *ret_param
OVS_UNUSED)
+{
+struct netdev_dpdk *dev;
+
+switch ((int)type) {
Do we need this cast?
In my environment this flag is enabled, not sure if I should change this
some where else…
lib/netdev-dpdk.c:1747:5: error: enumeration value
'RTE_ETH_EVENT_UNKNOWN' not handled in switch [-Werror=switch-enum]
switch (type) {
+
Empty line not needed.
ack
+case RTE_ETH_EVENT_INTR_RESET:
+
Empty line not needed.
ack
+ovs_mutex_lock(_mutex);
+dev = netdev_dpdk_lookup_by_port_id(port_id);
+if (dev) {
+ovs_mutex_lock(>mutex);
+dev->reset_needed = true;
+netdev_request_reconfigure(>up);> +
ovs_mutex_unlock(>mutex);
Maybe it'll be good to add at leas debug logs here?
Will ad a log message
+}
+ovs_mutex_unlock(_mutex);
+break;
+
+default:
+/* Ignore all other types */
+break;
+ }
+ return 0;
+}
+
static void
dpdk_set_rxq_config(struct netdev_dpdk *dev, const struct smap
*args)
OVS_REQUIRES(dev->mutex)
@@ -3777,6 +3806,8 @@ netdev_dpdk_class_init(void)
/* This function can be called for different classes. The
initialization
* needs to be done only once */
if (ovsthread_once_start()) {
+int ret;
+
ovs_thread_create("dpdk_watchdog", dpdk_watchdog, NULL);
unixctl_command_register("netdev-dpdk/set-admin-state",
"[netdev] up|down", 1, 2,
@@ -3790,6 +3821,14 @@ netdev_dpdk_class_init(void)
"[netdev]", 0, 1,
netdev_dpdk_get_mempool_info,
NULL);
+ret = rte_eth_dev_callback_register(RTE_ETH_ALL,
+
RTE_ETH_EVENT_INTR_RESET,
+dpdk_eth_event_callback,
NULL);
+
+if (ret != 0) {
+VLOG_ERR("Ethernet device callback register error %d",
ret);
It's better to print rte_strerror(-ret).
ack
+}
+
ovsthread_once_done();
}
@@ -4150,13 +4189,19 @@ netdev_dpdk_reconfigure(struct netdev
*netdev)
&& dev->rxq_size == dev->requested_rxq_size
&& dev->txq_size == dev->requested_txq_size
&& dev->socket_id == dev->requested_socket_id
-&& dev->started) {
+&& dev->started && !dev->reset_needed) {
/* Reconfiguration is unnecessary */
goto out;
}
-rte_eth_dev_stop(dev->port_id);
+if (dev->reset_needed) {
+rte_eth_dev_reset(dev->port_id);
+dev->reset_needed = false;
+} else {
+rte_eth_dev_stop(dev->port_id);
+}
+
dev->started = false;
err =
[ovs-dev] [PATCH ovn] ovn-openstack.rst: Add check for Gateway_Chassis table
This is related to the section called "Adding a Gateway".
Added workaround command for a known issue in networking-ovn.
Even after the issue is resolved, it may be useful to have this
in the tutorial, so folks have a feel for how OVN keeps track
of chassis gateway.
Also, removed redundant route command when assigning an address
to the br-ex interface.
Signed-off-by: Flavio Fernandes
---
Documentation/tutorials/ovn-openstack.rst | 15 ++-
1 file changed, 14 insertions(+), 1 deletion(-)
diff --git a/Documentation/tutorials/ovn-openstack.rst
b/Documentation/tutorials/ovn-openstack.rst
index 353ef209e..ed30e3044 100644
--- a/Documentation/tutorials/ovn-openstack.rst
+++ b/Documentation/tutorials/ovn-openstack.rst
@@ -1331,6 +1331,20 @@ with an IP address from the "private" network, then we
create a
floating IP address on the "public" network, then we associate the
port with the floating IP address.
+As of this writing, you may need to run the following to fix a
+problem with associating a logical port of router with the external
+gateway::
+
+ $ CHASSIS=$(ovn-nbctl --bare --columns="_uuid" find gateway_chassis) ; \
+[ -z "${CHASSIS}" ] && PORT_NAME='' || \
+PORT_NAME=$(ovn-nbctl --bare --columns=name \
+find logical_router_port gateway_chassis="${CHASSIS}")
+
+ $ [ -z "${PORT_NAME}" ] && {
+ openstack router unset --external-gateway router1 && \
+ openstack router set --external-gateway public router1
+} || echo logical port \"${PORT_NAME}\" in chassis \"${CHASSIS}\"
+
Let's add a new VM ``d`` with a floating IP::
$ openstack server create --nic net-id=private --flavor m1.nano --image
$IMAGE_ID --key-name demo d
@@ -1347,7 +1361,6 @@ It's also necessary to configure the "public" network
because DevStack
does not do it automatically::
$ sudo ip link set br-ex up
- $ sudo ip route add 172.24.4.0/24 dev br-ex
$ sudo ip addr add 172.24.4.1/24 dev br-ex
Now you should be able to "ping" VM ``d`` from the OpenStack host::
--
2.17.1
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [branch-2.12 PATCH] netdev-dpdk: Fix padding info comment.
The comment was incorrectly updated. Fix it to the
correct value of 32 pad bytes.
/* --- cacheline 5 boundary (320 bytes) --- */
union {
struct {
struct netdev_stats stats; /* 320 336 */
/* --- cacheline 5 boundary (320 bytes) was 16 bytes ago --- */
uint64_t tx_retries; /* 656 8 */
rte_spinlock_t stats_lock; /* 664 4 */
}; /* 352 */
uint8_tpad52[384]; /* 384 */
}; /* 320 384 */
Fixes: c161357d5d96 ("netdev-dpdk: Add custom stat for vhost tx retries.")
Reported-by: Ilya Maximets
Signed-off-by: Kevin Traynor
---
lib/netdev-dpdk.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index 48057835f..a53db0eed 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -452,5 +452,5 @@ struct netdev_dpdk {
/* Protects stats */
rte_spinlock_t stats_lock;
-/* 4 pad bytes here. */
+/* 32 pad bytes here. */
);
--
2.20.1
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] netdev-dpdk: add support for the RTE_ETH_EVENT_INTR_RESET event
Hi Eelco,
Thanks for the patch! Looks reasonable.
One comment is that it's better to explicitly initialize the
flag in common_construct. I see that we doesn't initialize
'started' flag, but this might be fixed too.
More comments inline.
Best regards, Ilya Maximets.
On 05.09.2019 14:48, Eelco Chaudron wrote:
> Currently, OVS does not register and therefore not handle the
> interface reset event from the DPDK framework. This would cause a
> problem in cases where a VF is used as an interface, and its
> configuration changes.
>
> As an example in the following scenario the MAC change is not
> detected/acted upon until OVS is restarted without the patch applied:
>
> $ echo 1 > /sys/bus/pci/devices/:05:00.1/sriov_numvfs
> $ ovs-vsctl add-port ovs_pvp_br0 dpdk0 -- \
> set Interface dpdk0 type=dpdk -- \
> set Interface dpdk0 options:dpdk-devargs=:05:0a.0
>
> $ ip link set p5p2 vf 0 mac 52:54:00:92:d3:33
>
> Signed-off-by: Eelco Chaudron
> ---
> lib/netdev-dpdk.c | 49 +--
> 1 file changed, 47 insertions(+), 2 deletions(-)
>
> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
> index bc20d6843..a23150387 100644
> --- a/lib/netdev-dpdk.c
> +++ b/lib/netdev-dpdk.c
> @@ -362,6 +362,7 @@ struct netdev_dpdk {
> bool attached;
> /* If true, rte_eth_dev_start() was successfully called */
> bool started;
> +bool reset_needed;
This will produce a hole in the structure as members will no longer
fit into a single cacheline. And cacheline markers will be misaligned.
See details for similar issue here:
https://mail.openvswitch.org/pipermail/ovs-dev/2019-September/362349.html
Possible solution is to turn existing flags into bit fields or stop
avoiding the problem and drop most of padding in the structure.
> struct eth_addr hwaddr;
> int mtu;
> int socket_id;
> @@ -1735,6 +1736,34 @@ netdev_dpdk_process_devargs(struct netdev_dpdk *dev,
> return new_port_id;
> }
>
> +static int
> +dpdk_eth_event_callback(uint16_t port_id, enum rte_eth_event_type type,
s/uint16_t/dpdk_port_t/
> +void *param OVS_UNUSED, void *ret_param OVS_UNUSED)
> +{
> +struct netdev_dpdk *dev;
> +
> +switch ((int)type) {
Do we need this cast?
> +
Empty line not needed.
> +case RTE_ETH_EVENT_INTR_RESET:
> +
Empty line not needed.
> +ovs_mutex_lock(_mutex);
> +dev = netdev_dpdk_lookup_by_port_id(port_id);
> +if (dev) {
> +ovs_mutex_lock(>mutex);
> +dev->reset_needed = true;
> +netdev_request_reconfigure(>up);> +
> ovs_mutex_unlock(>mutex);
Maybe it'll be good to add at leas debug logs here?
> +}
> +ovs_mutex_unlock(_mutex);
> +break;
> +
> +default:
> +/* Ignore all other types */
> +break;
> + }
> + return 0;
> +}
> +
> static void
> dpdk_set_rxq_config(struct netdev_dpdk *dev, const struct smap *args)
> OVS_REQUIRES(dev->mutex)
> @@ -3777,6 +3806,8 @@ netdev_dpdk_class_init(void)
> /* This function can be called for different classes. The initialization
> * needs to be done only once */
> if (ovsthread_once_start()) {
> +int ret;
> +
> ovs_thread_create("dpdk_watchdog", dpdk_watchdog, NULL);
> unixctl_command_register("netdev-dpdk/set-admin-state",
> "[netdev] up|down", 1, 2,
> @@ -3790,6 +3821,14 @@ netdev_dpdk_class_init(void)
> "[netdev]", 0, 1,
> netdev_dpdk_get_mempool_info, NULL);
>
> +ret = rte_eth_dev_callback_register(RTE_ETH_ALL,
> +RTE_ETH_EVENT_INTR_RESET,
> +dpdk_eth_event_callback, NULL);
> +
> +if (ret != 0) {
> +VLOG_ERR("Ethernet device callback register error %d", ret);
It's better to print rte_strerror(-ret).
> +}
> +
> ovsthread_once_done();
> }
>
> @@ -4150,13 +4189,19 @@ netdev_dpdk_reconfigure(struct netdev *netdev)
> && dev->rxq_size == dev->requested_rxq_size
> && dev->txq_size == dev->requested_txq_size
> && dev->socket_id == dev->requested_socket_id
> -&& dev->started) {
> +&& dev->started && !dev->reset_needed) {
> /* Reconfiguration is unnecessary */
>
> goto out;
> }
>
> -rte_eth_dev_stop(dev->port_id);
> +if (dev->reset_needed) {
> +rte_eth_dev_reset(dev->port_id);
> +dev->reset_needed = false;
> +} else {
> +rte_eth_dev_stop(dev->port_id);
> +}
> +
> dev->started = false;
>
> err = netdev_dpdk_mempool_configure(dev);
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] Question about robot mail reply
"txfh2007" writes: > Hi Aaron: > > Sorry to bother, I have submit a patch via ovs-dev, but as I > changed the signoff mail address and got the robot errors(see > below). This new signoff mail address is my new working mail which is > not in the author list(txfh2...@aliyun.com is in the author list, but > for some reason I can't use this mail as the signoff address). So > what should i do to pass the robot check ? Strange that you cannot use the author's mail as the signoff mail. Try adding: > From: Liu Chang to the start of your commit message (omit the '> '). You can test it yourself by applying your patch and looking at the 'Author:' field. If that doesn't work, I think it's still probably okay to accept. > Thanks very much ! > > Timo > > [PATCH] show "rx_missed_errors" counter in interface statisics > > ERROR: Author txfh2007 needs to sign off. > WARNING: Unexpected sign-offs from developers who are not authors or > co-authors or committers: Liu Chang > Lines checked: 31, Warnings: 1, Errors: 1 ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH] netdev-dpdk: add support for the RTE_ETH_EVENT_INTR_RESET event
Currently, OVS does not register and therefore not handle the
interface reset event from the DPDK framework. This would cause a
problem in cases where a VF is used as an interface, and its
configuration changes.
As an example in the following scenario the MAC change is not
detected/acted upon until OVS is restarted without the patch applied:
$ echo 1 > /sys/bus/pci/devices/:05:00.1/sriov_numvfs
$ ovs-vsctl add-port ovs_pvp_br0 dpdk0 -- \
set Interface dpdk0 type=dpdk -- \
set Interface dpdk0 options:dpdk-devargs=:05:0a.0
$ ip link set p5p2 vf 0 mac 52:54:00:92:d3:33
Signed-off-by: Eelco Chaudron
---
lib/netdev-dpdk.c | 49 +--
1 file changed, 47 insertions(+), 2 deletions(-)
diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index bc20d6843..a23150387 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -362,6 +362,7 @@ struct netdev_dpdk {
bool attached;
/* If true, rte_eth_dev_start() was successfully called */
bool started;
+bool reset_needed;
struct eth_addr hwaddr;
int mtu;
int socket_id;
@@ -1735,6 +1736,34 @@ netdev_dpdk_process_devargs(struct netdev_dpdk *dev,
return new_port_id;
}
+static int
+dpdk_eth_event_callback(uint16_t port_id, enum rte_eth_event_type type,
+void *param OVS_UNUSED, void *ret_param OVS_UNUSED)
+{
+struct netdev_dpdk *dev;
+
+switch ((int)type) {
+
+case RTE_ETH_EVENT_INTR_RESET:
+
+ovs_mutex_lock(_mutex);
+dev = netdev_dpdk_lookup_by_port_id(port_id);
+if (dev) {
+ovs_mutex_lock(>mutex);
+dev->reset_needed = true;
+netdev_request_reconfigure(>up);
+ovs_mutex_unlock(>mutex);
+}
+ovs_mutex_unlock(_mutex);
+break;
+
+default:
+/* Ignore all other types */
+break;
+ }
+ return 0;
+}
+
static void
dpdk_set_rxq_config(struct netdev_dpdk *dev, const struct smap *args)
OVS_REQUIRES(dev->mutex)
@@ -3777,6 +3806,8 @@ netdev_dpdk_class_init(void)
/* This function can be called for different classes. The initialization
* needs to be done only once */
if (ovsthread_once_start()) {
+int ret;
+
ovs_thread_create("dpdk_watchdog", dpdk_watchdog, NULL);
unixctl_command_register("netdev-dpdk/set-admin-state",
"[netdev] up|down", 1, 2,
@@ -3790,6 +3821,14 @@ netdev_dpdk_class_init(void)
"[netdev]", 0, 1,
netdev_dpdk_get_mempool_info, NULL);
+ret = rte_eth_dev_callback_register(RTE_ETH_ALL,
+RTE_ETH_EVENT_INTR_RESET,
+dpdk_eth_event_callback, NULL);
+
+if (ret != 0) {
+VLOG_ERR("Ethernet device callback register error %d", ret);
+}
+
ovsthread_once_done();
}
@@ -4150,13 +4189,19 @@ netdev_dpdk_reconfigure(struct netdev *netdev)
&& dev->rxq_size == dev->requested_rxq_size
&& dev->txq_size == dev->requested_txq_size
&& dev->socket_id == dev->requested_socket_id
-&& dev->started) {
+&& dev->started && !dev->reset_needed) {
/* Reconfiguration is unnecessary */
goto out;
}
-rte_eth_dev_stop(dev->port_id);
+if (dev->reset_needed) {
+rte_eth_dev_reset(dev->port_id);
+dev->reset_needed = false;
+} else {
+rte_eth_dev_stop(dev->port_id);
+}
+
dev->started = false;
err = netdev_dpdk_mempool_configure(dev);
--
2.20.1
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH ovn] Handle GARP reply packets from provider networks only on gateway chassis
On Thu, Sep 5, 2019 at 3:19 AM Han Zhou wrote:
>
>
> On Mon, Sep 2, 2019 at 9:32 AM wrote:
> >
> > From: Numan Siddique
> >
> > Suppose there is a provider network (with localnet port) and it is
> > connected to a logical router via a distributed gateway router port.
> > When an external switch sends GARP request packets, these packets
> > enter the br-int (via the patch port from the provider bridge) and
> > the action - put_arp is applied in the router pipeline only on the
> > gateway chassis where the distributed gateway router port is scheduled.
> > Other chassis even if they receive don't apply this action. This
> functionality
> > was added by the commit [1].
> >
> > But that is not the case with GARP reply packets sent by the external
> > switch. ovn-controllers running on all the chassis configured with
> > ovn-bridge-mappings receive these packets as packet-ins because
> > of put_arp action. This results in unnecessary processing of these
> > packets only to be ignored. pinctrl thread wakes up the main
> ovn-controller
> > thread wasting few CPU cycles. It is enough for the gateway chassis to
> > handle these packets where the distributed router gateway port is
> scheduled.
> >
> > This patch achieves the same - similar to GARP request packets. The below
> > logical flows are added
> > - table=1 (lr_in_ip_input), priority=92,
> >match=(inport == "lrp" &&
> > !is_chassis_resident("cr-lrp") && eth.bcast &&
> > arp.op == 2 && arp.spa == PROVIDER_NETWORK_CIDR),
> action=(drop;)
> >
> > - table=1 (lr_in_ip_input), priority=92
> >match=(inport == "lr0-public" && is_chassis_resident("cr-lr0-public")
> > && eth.bcast && arp.op == 2 && arp.spa ==
> PROVIDER_NETWORK_CIDR),
> >action=(put_arp(inport, arp.spa, arp.sha);)
> >
> > This patch doesn't affect the arp replies from the overlay networks in
> the
> > router pipeline. This only handles GARP replies from the provider
> networks.
> >
> > In the present master this is not much of any issue even if
> ovn-controller
> > wakes up, thanks to incremental processing engine. But in the older
> > versions - 2.11, 2.10 and 2.9, this results in complete flow calculations
> > resulting in much more CPU cyles. This patch will definitely help in
> saving
> > these CPU cyles if backported.
> >
> > [1] - 3dd9febe953f("ovn-northd: Support learning neighbor from ARP
> request.")
> >
> > CC: Han ZHou
> > Signed-off-by: Numan Siddique
> > ---
> > northd/ovn-northd.8.xml | 31 +++
> > northd/ovn-northd.c | 35 +++
> > 2 files changed, 62 insertions(+), 4 deletions(-)
> >
> > diff --git a/northd/ovn-northd.8.xml b/northd/ovn-northd.8.xml
> > index 442e899bc..a06e404dd 100644
> > --- a/northd/ovn-northd.8.xml
> > +++ b/northd/ovn-northd.8.xml
> > @@ -1442,10 +1442,33 @@ arp.sha = external_mac;
> >
> >
> >
> > -ARP reply handling. This flow uses ARP replies to populate the
> > -logical router's ARP table. A priority-90 flow with match
> arp.op
> > -== 2 has actions put_arp(inport, arp.spa,
> > -arp.sha);.
> > +
> > + ARP reply handling. Following flows are added to handle ARP
> replies.
> > +
> > +
> > +
> > + For each distributed gateway logical router port a
> priority-92 flow
> > + with match inport == P
> > + is_chassis_resident(cr-P) eth.bcast
>
> > + arp.op == 2 arp.spa == I with the
> > + action put_arp(inport, arp.spa, arp.sha); so
> that the
> > + resident gateway chassis can learn the GARP reply, where
> > + P is the distributed gateway router port name,
> > + I is the logical router port's network address.
> > +
> > +
> > +
> > + For each distributed gateway logical router port a
> priority-92 flow
> > + with match inport == P
> > + !is_chassis_resident(cr-P) eth.bcast
>
> > + arp.op == 2 arp.spa == I with
> the action
> > + drop; so that other chassis drop this packet.
> > +
> > +
> > +
> > + A priority-90 flow with match arp.op == 2 has
> actions
> > + put_arp(inport, arp.spa, arp.sha);.
> > +
> >
> >
> >
> > diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
> > index 9a282..a83b56362 100644
> > --- a/northd/ovn-northd.c
> > +++ b/northd/ovn-northd.c
> > @@ -6552,6 +6552,41 @@ build_lrouter_flows(struct hmap *datapaths,
> struct hmap *ports,
> >
> > }
> >
> > +/* Handle GARP reply packets received on a distributed router
> gateway
> > + * port. GARP reply broadcast packets could be sent by external
> > + * switches. We don't want them to be handled by all the
> > + * ovn-controllers if they receive it. So add a priority-92
> flow to
> > + * apply the put_arp action on a redirect chassis and drop it on
> > + *
Re: [ovs-dev] SIGILL ovs branch-2.12/arm64/DPDK
Thanks James, I can understand the root cause now. Best Regards, Wei Yanqin From: James Page Sent: Thursday, September 5, 2019 3:42 PM To: Yanqin Wei (Arm Technology China) Cc: Ilya Maximets ; ovs-dev@openvswitch.org Subject: Re: [ovs-dev] SIGILL ovs branch-2.12/arm64/DPDK On Thu, Sep 5, 2019 at 8:34 AM Yanqin Wei (Arm Technology China) mailto:yanqin@arm.com>> wrote: Is it possible a configuration issue for building OVS userspace program , which should not enable crc for xgene cpu? DPDK library is linked with OVS program during OVS building. It should not import compiling configuration from DPDK, right? Compiler flags will be generated by the pkg-config provided by DPDK - the CPU flags exceed the features of the xgene CPU. That said, in the context of compiling a DPDK enabled version of OVS for arm64 for general consumption, having a practical baseline for CPU features that are actually useful to arm64 users is important. Canonical have some fairly earlier arm64 hardware in the Ubuntu build infrastructure which is probably below the sensible baseline for DPDK support. IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] SIGILL ovs branch-2.12/arm64/DPDK
Hi Ilya, Thanks for the clarification. It looks application is strong couple with DPDK. I will follow your discussion under this bug. Best Regards, Wei Yanqin -Original Message- From: Ilya Maximets Sent: Thursday, September 5, 2019 3:41 PM To: Yanqin Wei (Arm Technology China) ; James Page Cc: ovs-dev@openvswitch.org Subject: Re: [ovs-dev] SIGILL ovs branch-2.12/arm64/DPDK On 05.09.2019 10:34, Yanqin Wei (Arm Technology China) wrote: > Is it possible a configuration issue for building OVS userspace program , > which should not enable crc for xgene cpu? There is a arm64-xgene1-linux-gcc target that can be used for a legacy make-based build system. New meson based build doesn't support that. > DPDK library is linked with OVS program during OVS building. It should not > import compiling configuration from DPDK, right? DPDK exports its machine flags, extra defines and libraries to the application build process because big and important parts of DPDK code are in header inlines and built within the application build process. > > Best Regards, > Wei Yanqin > > -Original Message- > From: Ilya Maximets > Sent: Wednesday, September 4, 2019 8:46 PM > To: Yanqin Wei (Arm Technology China) ; James Page > > Cc: ovs-dev@openvswitch.org > Subject: Re: [ovs-dev] SIGILL ovs branch-2.12/arm64/DPDK > > BTW, I submitted a bug to DPDK: > https://bugs.dpdk.org/show_bug.cgi?id=344 > > Best regards, Ilya Maximets. > > On 04.09.2019 12:53, Yanqin Wei (Arm Technology China) wrote: >> Understood. Thanks for the information. >> >> >> >> Best Regards, >> >> Wei Yanqin >> >> >> >> *From:* James Page >> *Sent:* Wednesday, September 4, 2019 5:45 PM >> *To:* Yanqin Wei (Arm Technology China) >> *Cc:* Ilya Maximets ; ovs-dev@openvswitch.org >> *Subject:* Re: [ovs-dev] SIGILL ovs branch-2.12/arm64/DPDK >> >> >> >> Hi >> >> >> >> On Wed, Sep 4, 2019 at 10:30 AM Yanqin Wei (Arm Technology China) >> mailto:yanqin@arm.com>> wrote: >> >> >> CRC32 is not a mandatory feature for Armv8.0. What is the arm64 CPU used >> in your platform? >> I'm not sure if it's necessary to use mandatory feature (neon) to >> optimize hash libraries, because I thought most arm64 CPUs supported CRC32. >> >> >> >> The builders we use in Launchpad for package builds are fully virtualized >> via OpenStack; I checked and we have some older X-gene CPU's which don't >> have crc32 support. >> >> >> >> IMPORTANT NOTICE: The contents of this email and any attachments are >> confidential and may also be privileged. If you are not the intended >> recipient, please notify the sender immediately and do not disclose the >> contents to any other person, use it for any purpose, or store or copy the >> information in any medium. Thank you. > IMPORTANT NOTICE: The contents of this email and any attachments are > confidential and may also be privileged. If you are not the intended > recipient, please notify the sender immediately and do not disclose the > contents to any other person, use it for any purpose, or store or copy the > information in any medium. Thank you. > IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] ovs-vswitchd exited silently
Hi All I’m encountered a very strange problem that ovs-vswitchd exited silently without any exceptions while ovsdb-server running normally, I’m using openvswitch 2.9.2 I didn’t find any valuable log except that ovs|1|unixctl|WARN|failed to connect to /var/run/openvswitch/ovs-vswitchd.5168.ctl ovs-ctl[303320]: 2019-09-05T03:12:28Z|1|unixctl|WARN|failed to connect to /var/run/openvswitch/ovs-vswitchd.5168.ctl ovs-ctl[303320]: ovs-appctl: cannot connect to "/var/run/openvswitch/ovs-vswitchd.5168.ctl" (Connection refused) See the details below. [root@node-34435-75174 ~]# systemctl status ovs-vswitchd -l ● ovs-vswitchd.service - Open vSwitch Forwarding Unit Loaded: loaded (/usr/lib/systemd/system/ovs-vswitchd.service; static; vendor preset: disabled) Active: inactive (dead) since Thu 2019-09-05 11:12:28 CST; 4h 4min ago Process: 303320 ExecStop=/usr/share/openvswitch/scripts/ovs-ctl --no-ovsdb-server stop (code=exited, status=0/SUCCESS) Aug 27 11:09:13 node-34435-75174 systemd[1]: Starting Open vSwitch Forwarding Unit... Aug 27 11:09:13 node-34435-75174 ovs-ctl[5105]: Inserting openvswitch module [ OK ] Aug 27 11:09:13 node-34435-75174 ovs-ctl[5105]: Starting ovs-vswitchd [ OK ] Aug 27 11:09:13 node-34435-75174 ovs-vsctl[5257]: ovs|1|vsctl|INFO|Called as ovs-vsctl --no-wait set Open_vSwitch . external-ids:hostname=node-34435-75174 Aug 27 11:09:13 node-34435-75174 ovs-ctl[5105]: Enabling remote OVSDB managers [ OK ] Aug 27 11:09:13 node-34435-75174 systemd[1]: Started Open vSwitch Forwarding Unit. Sep 05 11:12:28 node-34435-75174 ovs-appctl[30]: ovs|1|unixctl|WARN|failed to connect to /var/run/openvswitch/ovs-vswitchd.5168.ctl Sep 05 11:12:28 node-34435-75174 ovs-ctl[303320]: 2019-09-05T03:12:28Z|1|unixctl|WARN|failed to connect to /var/run/openvswitch/ovs-vswitchd.5168.ctl Sep 05 11:12:28 node-34435-75174 ovs-ctl[303320]: ovs-appctl: cannot connect to "/var/run/openvswitch/ovs-vswitchd.5168.ctl" (Connection refused) [root@node-34435-75174 ~]# ls /var/run/openvswitch/ -l total 12 srwxr-x---. 1 openvswitch openvswitch 0 Aug 27 11:09 db.sock srwxr-x---. 1 openvswitch openvswitch 0 Aug 27 11:09 managevSwitch.mgmt srwxr-x---. 1 openvswitch openvswitch 0 Aug 27 11:09 managevSwitch.snoop srwxr-x---. 1 openvswitch openvswitch 0 Aug 27 11:09 ovsdb-server.5084.ctl -rw-r--r--. 1 openvswitch openvswitch 5 Aug 27 11:09 ovsdb-server.pid srwxr-x---. 1 openvswitch openvswitch 0 Aug 27 11:09 ovs-vswitchd.5168.ctl -rw-r--r--. 1 openvswitch openvswitch 5 Aug 27 11:09 ovs-vswitchd.pid srwxr-x---. 1 openvswitch openvswitch 0 Aug 27 11:09 sw-01.mgmt srwxr-x---. 1 openvswitch openvswitch 0 Aug 27 11:09 sw-01.snoop -rw-r--r--. 1 rootroot43 Aug 27 11:09 useropts cat /var/log/message Sep 5 11:00:01 node-34435-75174 systemd: Removed slice User Slice of root. Sep 5 11:01:01 node-34435-75174 systemd: Created slice User Slice of root. Sep 5 11:01:01 node-34435-75174 systemd: Started Session 1530 of user root. Sep 5 11:01:01 node-34435-75174 systemd: Removed slice User Slice of root. Sep 5 11:10:01 node-34435-75174 systemd: Created slice User Slice of root. Sep 5 11:10:01 node-34435-75174 systemd: Started Session 1531 of user root. Sep 5 11:10:01 node-34435-75174 systemd: Removed slice User Slice of root. Sep 5 11:12:28 node-34435-75174 ovs-appctl: ovs|1|unixctl|WARN|failed to connect to /var/run/openvswitch/ovs-vswitchd.5168.ctl Sep 5 11:12:28 node-34435-75174 ovs-ctl: 2019-09-05T03:12:28Z|1|unixctl|WARN|failed to connect to /var/run/openvswitch/ovs-vswitchd.5168.ctl Sep 5 11:12:28 node-34435-75174 ovs-ctl: ovs-appctl: cannot connect to "/var/run/openvswitch/ovs-vswitchd.5168.ctl" (Connection refused) Sep 5 11:20:01 node-34435-75174 systemd: Created slice User Slice of root. Sep 5 11:20:01 node-34435-75174 systemd: Started Session 1532 of user root. Sep 5 11:20:01 node-34435-75174 systemd: Removed slice User Slice of root. Sep 5 11:24:12 node-34435-75174 systemd: Starting Cleanup of Temporary Directories... Sep 5 11:24:12 node-34435-75174 systemd: Started Cleanup of Temporary Directories. Sep 5 11:30:01 node-34435-75174 systemd: Created slice User Slice of root. Sep 5 11:30:01 node-34435-75174 systemd: Started Session 1533 of user root. Sep 5 11:30:01 node-34435-75174 systemd: Removed slice User Slice of root. Sep 5 11:40:01 node-34435-7 [root@node-34435-75174 ~]# systemctl status ovsdb-server -l ● ovsdb-server.service - Open vSwitch Database Unit Loaded: loaded (/usr/lib/systemd/system/ovsdb-server.service; static; vendor preset: disabled) Active: active (running) since Tue 2019-08-27 11:09:13 CST; 1 weeks 2 days ago Tasks: 1 CGroup: /system.slice/ovsdb-server.service └─5084 ovsdb-server /etc/openvswitch/conf.db -vconsole:emer -vsyslog:err -vfile:info --remote=punix:/var/run/openvswitch/db.sock
Re: [ovs-dev] [PATCH v12] Improved Packet Drop Statistics in OVS
Not a code review. Just a few notes about patch formatting. Best regards, Ilya Maximets. On 25.07.2019 15:16, Anju Thomas wrote: > Currently OVS maintains explicit packet drop/error counters only on > port level. Packets that are dropped as part of normal OpenFlow > processing are counted in flow stats of “drop” flows or as table > misses in table stats. These can only be interpreted by controllers > that know the semantics of the configured OpenFlow pipeline. > Without that knowledge, it is impossible for an OVS user to obtain > e.g. the total number of packets dropped due to OpenFlow rules. > > Furthermore, there are numerous other reasons for which packets can > be dropped by OVS slow path that are not related to the OpenFlow pipeline. > The generated datapath flow entries include a drop action to avoid > further expensive upcalls to the slow path, but subsequent packets > dropped by the datapath are not accounted anywhere. > > Finally, the datapath itself drops packets in certain error situations. > Also, these drops are today not accounted for.This makes it difficult > for OVS users to monitor packet drop in an OVS instance and to alert > a management system in case of a unexpected increase of such drops. > Also OVS trouble-shooters face difficulties in analysing packet drops. > > With this patch we implement following changes to address the issues > mentioned above. > > 1. Identify and account all the silent packet drop scenarios > > 2. Display these drops in ovs-appctl coverage/show > > v11->v12 Addresses comments from Ben Pfaff Above line should go after the '---' line as it's not useful in a git log. > > Co-authored-by: Rohith Basavaraja > Co-authored-by: Keshav Gupta > Signed-off-by: Anju Thomas > Signed-off-by: Rohith Basavaraja > Signed-off-by: Keshav Gupta > --- Version history should be here and it'll be good if it will be a bit more detailed, i.e. what really changed in this and previous versions. Like this: Version 13: * Implemented something. * Fixed something. Version 12: * ... Without history it's hard to track changes especially for long living or big patches. Same applicable to other patches on a mail-list. One more hint is that it's better to add relevant reviewers to CC list while sending patches. This could speed up review process as not everyone subscribed to all the messages from mail-list. > datapath/linux/compat/include/linux/openvswitch.h | 1 + > lib/dpif-netdev.c | 46 - > lib/dpif.c| 7 + > lib/dpif.h| 4 + > lib/odp-execute.c | 79 > lib/odp-util.c| 9 + > ofproto/ofproto-dpif-ipfix.c | 1 + > ofproto/ofproto-dpif-sflow.c | 1 + > ofproto/ofproto-dpif-xlate.c | 40 - > ofproto/ofproto-dpif-xlate.h | 3 + > ofproto/ofproto-dpif.c| 8 + > ofproto/ofproto-dpif.h| 8 +- > tests/automake.mk | 3 +- > tests/dpif-netdev.at | 8 + > tests/drop-stats.at | 210 > ++ > tests/ofproto-dpif.at | 2 +- > tests/tunnel-push-pop.at | 29 ++- > tests/tunnel.at | 16 +- > 18 files changed, 464 insertions(+), 11 deletions(-) > create mode 100644 tests/drop-stats.at ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] SIGILL ovs branch-2.12/arm64/DPDK
On Thu, Sep 5, 2019 at 8:34 AM Yanqin Wei (Arm Technology China) < yanqin@arm.com> wrote: > Is it possible a configuration issue for building OVS userspace program , > which should not enable crc for xgene cpu? DPDK library is linked with OVS > program during OVS building. It should not import compiling configuration > from DPDK, right? > Compiler flags will be generated by the pkg-config provided by DPDK - the CPU flags exceed the features of the xgene CPU. That said, in the context of compiling a DPDK enabled version of OVS for arm64 for general consumption, having a practical baseline for CPU features that are actually useful to arm64 users is important. Canonical have some fairly earlier arm64 hardware in the Ubuntu build infrastructure which is probably below the sensible baseline for DPDK support. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] SIGILL ovs branch-2.12/arm64/DPDK
On 05.09.2019 10:34, Yanqin Wei (Arm Technology China) wrote: > Is it possible a configuration issue for building OVS userspace program , > which should not enable crc for xgene cpu? There is a arm64-xgene1-linux-gcc target that can be used for a legacy make-based build system. New meson based build doesn't support that. > DPDK library is linked with OVS program during OVS building. It should not > import compiling configuration from DPDK, right? DPDK exports its machine flags, extra defines and libraries to the application build process because big and important parts of DPDK code are in header inlines and built within the application build process. > > Best Regards, > Wei Yanqin > > -Original Message- > From: Ilya Maximets > Sent: Wednesday, September 4, 2019 8:46 PM > To: Yanqin Wei (Arm Technology China) ; James Page > > Cc: ovs-dev@openvswitch.org > Subject: Re: [ovs-dev] SIGILL ovs branch-2.12/arm64/DPDK > > BTW, I submitted a bug to DPDK: > https://bugs.dpdk.org/show_bug.cgi?id=344 > > Best regards, Ilya Maximets. > > On 04.09.2019 12:53, Yanqin Wei (Arm Technology China) wrote: >> Understood. Thanks for the information. >> >> >> >> Best Regards, >> >> Wei Yanqin >> >> >> >> *From:* James Page >> *Sent:* Wednesday, September 4, 2019 5:45 PM >> *To:* Yanqin Wei (Arm Technology China) >> *Cc:* Ilya Maximets ; ovs-dev@openvswitch.org >> *Subject:* Re: [ovs-dev] SIGILL ovs branch-2.12/arm64/DPDK >> >> >> >> Hi >> >> >> >> On Wed, Sep 4, 2019 at 10:30 AM Yanqin Wei (Arm Technology China) >> mailto:yanqin@arm.com>> wrote: >> >> >> CRC32 is not a mandatory feature for Armv8.0. What is the arm64 CPU used >> in your platform? >> I'm not sure if it's necessary to use mandatory feature (neon) to >> optimize hash libraries, because I thought most arm64 CPUs supported CRC32. >> >> >> >> The builders we use in Launchpad for package builds are fully virtualized >> via OpenStack; I checked and we have some older X-gene CPU's which don't >> have crc32 support. >> >> >> >> IMPORTANT NOTICE: The contents of this email and any attachments are >> confidential and may also be privileged. If you are not the intended >> recipient, please notify the sender immediately and do not disclose the >> contents to any other person, use it for any purpose, or store or copy the >> information in any medium. Thank you. > IMPORTANT NOTICE: The contents of this email and any attachments are > confidential and may also be privileged. If you are not the intended > recipient, please notify the sender immediately and do not disclose the > contents to any other person, use it for any purpose, or store or copy the > information in any medium. Thank you. > ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v12] Improved Packet Drop Statistics in OVS
Thanks, I’ve CC’ed Ben so he can look at the comments regarding his
earlier comment :)
Looking forward for the next version…
//Eelco
On 4 Sep 2019, at 11:24, Anju Thomas wrote:
Hi Eelco,
Since I have not received any more comments, let me respond to your
comments:
-Original Message-
From: Eelco Chaudron
Sent: Tuesday, September 3, 2019 7:27 PM
To: Anju Thomas
Cc: d...@openvswitch.org; Keshav Gupta
Subject: Re: [ovs-dev] [PATCH v12] Improved Packet Drop Statistics in
OVS
On 20 Aug 2019, at 11:57, Anju Thomas wrote:
Thanks for the comments Eelco. I will address them in the next patch.
On a similar note, are these the final set of comments for this patch
?
Did you get any update, and are you planning on sending out an update?
Thanks,
Eelco
Regards
Anju
-Original Message-
From: Eelco Chaudron
Sent: Monday, August 12, 2019 5:48 PM
To: Anju Thomas
Cc: d...@openvswitch.org; Keshav Gupta
Subject: Re: [ovs-dev] [PATCH v12] Improved Packet Drop Statistics in
OVS
Hi Anju,
See comments inline…
//Eelco
On 25 Jul 2019, at 14:16, Anju Thomas wrote:
Currently OVS maintains explicit packet drop/error counters only on
port level. Packets that are dropped as part of normal OpenFlow
processing are counted in flow stats of “drop” flows or as table
misses in table stats. These can only be interpreted by controllers
that know the semantics of the configured OpenFlow pipeline.
Without that knowledge, it is impossible for an OVS user to obtain
e.g. the total number of packets dropped due to OpenFlow rules.
Furthermore, there are numerous other reasons for which packets can
be dropped by OVS slow path that are not related to the OpenFlow
pipeline.
The generated datapath flow entries include a drop action to avoid
further expensive upcalls to the slow path, but subsequent packets
dropped by the datapath are not accounted anywhere.
Finally, the datapath itself drops packets in certain error
situations.
Also, these drops are today not accounted for.This makes it
difficult
for OVS users to monitor packet drop in an OVS instance and to alert
a management system in case of a unexpected increase of such drops.
Also OVS trouble-shooters face difficulties in analysing packet
drops.
With this patch we implement following changes to address the issues
mentioned above.
1. Identify and account all the silent packet drop scenarios
2. Display these drops in ovs-appctl coverage/show
v11->v12 Addresses comments from Ben Pfaff
Co-authored-by: Rohith Basavaraja
Co-authored-by: Keshav Gupta
Signed-off-by: Anju Thomas
Signed-off-by: Rohith Basavaraja
Signed-off-by: Keshav Gupta
---
datapath/linux/compat/include/linux/openvswitch.h | 1 +
lib/dpif-netdev.c | 46 -
lib/dpif.c| 7 +
lib/dpif.h| 4 +
lib/odp-execute.c | 79
lib/odp-util.c| 9 +
ofproto/ofproto-dpif-ipfix.c | 1 +
ofproto/ofproto-dpif-sflow.c | 1 +
ofproto/ofproto-dpif-xlate.c | 40 -
ofproto/ofproto-dpif-xlate.h | 3 +
ofproto/ofproto-dpif.c| 8 +
ofproto/ofproto-dpif.h| 8 +-
tests/automake.mk | 3 +-
tests/dpif-netdev.at | 8 +
tests/drop-stats.at | 210
++
tests/ofproto-dpif.at | 2 +-
tests/tunnel-push-pop.at | 29 ++-
tests/tunnel.at | 16 +-
18 files changed, 464 insertions(+), 11 deletions(-) create mode
100644 tests/drop-stats.at
diff --git a/datapath/linux/compat/include/linux/openvswitch.h
b/datapath/linux/compat/include/linux/openvswitch.h
index 65a003a..415c053 100644
--- a/datapath/linux/compat/include/linux/openvswitch.h
+++ b/datapath/linux/compat/include/linux/openvswitch.h
@@ -989,6 +989,7 @@ enum ovs_action_attr { #ifndef __KERNEL__
OVS_ACTION_ATTR_TUNNEL_PUSH, /* struct ovs_action_push_tnl*/
OVS_ACTION_ATTR_TUNNEL_POP,/* u32 port number. */
+ OVS_ACTION_ATTR_DROP,
#endif
__OVS_ACTION_ATTR_MAX,/* Nothing past this will be accepted
* from userspace. */
diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c index
6b99a3c..3878b8d 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -101,6 +101,17 @@ enum { MAX_METERS = 65536 };/* Maximum
number
of meters. */
enum { MAX_BANDS = 8 }; /* Maximum number of bands / meter.
*/
enum { N_METER_LOCKS = 64 };/* Maximum number of meters. */
+COVERAGE_DEFINE(datapath_drop_meter);
+COVERAGE_DEFINE(datapath_drop_upcall_error);
+COVERAGE_DEFINE(datapath_drop_lock_error);
Re: [ovs-dev] SIGILL ovs branch-2.12/arm64/DPDK
Is it possible a configuration issue for building OVS userspace program , which should not enable crc for xgene cpu? DPDK library is linked with OVS program during OVS building. It should not import compiling configuration from DPDK, right? Best Regards, Wei Yanqin -Original Message- From: Ilya Maximets Sent: Wednesday, September 4, 2019 8:46 PM To: Yanqin Wei (Arm Technology China) ; James Page Cc: ovs-dev@openvswitch.org Subject: Re: [ovs-dev] SIGILL ovs branch-2.12/arm64/DPDK BTW, I submitted a bug to DPDK: https://bugs.dpdk.org/show_bug.cgi?id=344 Best regards, Ilya Maximets. On 04.09.2019 12:53, Yanqin Wei (Arm Technology China) wrote: > Understood. Thanks for the information. > > > > Best Regards, > > Wei Yanqin > > > > *From:* James Page > *Sent:* Wednesday, September 4, 2019 5:45 PM > *To:* Yanqin Wei (Arm Technology China) > *Cc:* Ilya Maximets ; ovs-dev@openvswitch.org > *Subject:* Re: [ovs-dev] SIGILL ovs branch-2.12/arm64/DPDK > > > > Hi > > > > On Wed, Sep 4, 2019 at 10:30 AM Yanqin Wei (Arm Technology China) > mailto:yanqin@arm.com>> wrote: > > > CRC32 is not a mandatory feature for Armv8.0. What is the arm64 CPU used > in your platform? > I'm not sure if it's necessary to use mandatory feature (neon) to > optimize hash libraries, because I thought most arm64 CPUs supported CRC32. > > > > The builders we use in Launchpad for package builds are fully virtualized via > OpenStack; I checked and we have some older X-gene CPU's which don't have > crc32 support. > > > > IMPORTANT NOTICE: The contents of this email and any attachments are > confidential and may also be privileged. If you are not the intended > recipient, please notify the sender immediately and do not disclose the > contents to any other person, use it for any purpose, or store or copy the > information in any medium. Thank you. IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v7] Detailed packet drop statistics per dpdk and vhostuser ports
Hi Ilya,
Thanks a lot for the explanation. As per your suggestion, I will move all the
counters (including 'tx_retries')to some structure and place a pointer to it
in netdev_dpdk structure so that the padding size will not vary with the
introduction of new counters in future.
@Kevin Traynor : I will change the comment line for the number of padding
bytes in my next patch instead of you sending a new patch just for changing
the comment line.
Thanks & Regards,
Sriram.
-Original Message-
From: Ilya Maximets
Sent: 04 September 2019 19:34
To: Sriram Vatala ; Kevin Traynor
Cc: ovs-dev@openvswitch.org; Stokes, Ian
Subject: Re: [PATCH v7] Detailed packet drop statistics per dpdk and vhostuser
ports
On 04.09.2019 16:31, Sriram Vatala wrote:
> Hi Ilya,
> 1) I was working on addressing the comments provided by you. Had a
> small query on one of your comments.
> 2) I am trying to understand the problem of padding bytes in struct
> netdev_dpdk which you are referring to in your comment.
> 3) If I understand correctly, the macro "PADDED_MEMBERS(UNIT, MEMBERS)"
> ensures that the memory to be allocated for all 'MEMBERS' is rounded
> off to nearest multiple of CACHE_LINE_SIZE which is 64 in this case.
> This macro adds pad bytes to roundoff to multiple of 64.
> 4) At runtime, I checked the size of stats section of netdev_dpdk
> without new counters that I have introduced in my patch. It is 384
> bytes, out of which struct netdev_stats alone occupies 336 bytes and 8
> bytes for tx_retries counter. (I could see there is no padding between
> netdev_stats and tx_retries). Total of 344 is rounded to 384 [((344 + 64 -
> 1)/64) * 64].
> 5) With my new counters, the size remains same after padding.
> (336 + 8 + 8 +8 +8 +8 = 376) which is rounded to 384 bytes [((376 +64
> -1)/64) *64] at runtime.
>
> I want to check with you, if I have correctly understood the problem
> that you are referring in your comment. If you can explain a bit more
> on this, it would be helpful for me to understand the problem and think of
> possible solutions.
>
> Following is the snippet of memory layout of netdev_dpdk at runtime :
> union {
> OVS_CACHE_LINE_MARKER cacheline1;
> struct {...};
> uint8_t pad50[64];
> };
> union {
> struct {...};
> uint8_t pad51[192];
> };
> union {
> struct {...};
> uint8_t pad52[384];
> };
> union {
> struct {...};
> uint8_t pad53[128];
> };
> union {
> struct {...};
> uint8_t pad54[64];
> };
> }
Hi.
The code looks like this:
PADDED_MEMBERS(CACHE_LINE_SIZE,
struct netdev_stats stats;
/* Custom stat for retries when unable to transmit. */
uint64_t tx_retries;
/* Protects stats */
rte_spinlock_t stats_lock;
/* 4 pad bytes here. */<-- This comment I'm talking about.
);
The only thing you need to change is to update the comment while adding new
structure fields.
Your calculations are missing the size of 'stats_lock' which is 4 bytes.
So, on current master total size is:
336 bytes for stats + 8 bytes for tx_retries + 4 bytes for lock = 352 And
it's rounded up to 384 by padding, i.e. we have 384 - 352 = 32 pad bytes.
The comment on current master should be "/* 32 pad bytes here. */".
BTW, Kevin, how did you calculate 4 here? My pahole output is following:
union {
struct {
struct netdev_stats stats; /* 320 336 */
uint64_t tx_retries; /* 656 8 */
rte_spinlock_t stats_lock; /* 664 4 */
}; /* 352 */
uint8_tpad52[0]; /* 0 */
}; /* 320 384 */
Returning to the patch. After adding 4 more stats (4 * 8 bytes), the new
layout
takes:
336 bytes for stats + 8 bytes for tx_retries \
+ 4*8 bytes for new stats + 4 bytes for lock = 384 bytes.
Now the structure takes exactly 384 bytes and you need to remove the comment
or change it to "/* 0 pad bytes here. */".
Sorry, I didn't check the actual layout until now so I was confused a bit by
the comment on current master. Anyway, you need to update that comment.
However, It might be still useful to move these stats to a separate structure
to avoid big padding in case we'll want to add one more. And I'm still
thinking that we need to drop paddings at all for most of the structure
fields, but this should be a separate change.
Best regards, Ilya Maximets.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
