Re: [ovs-dev] [PATCH] ofp-group: Don't assert-fail decoding bad OF1.5 group mod type or command.
Thanks, applied to master and backported as far as branch-2.4. On Fri, Jul 06, 2018 at 04:24:35PM -0700, Yifeng Sun wrote: > Looks good to me. Thanks. > > Reviewed-by: Yifeng Sun > > On Thu, Jul 5, 2018 at 3:28 PM, Ben Pfaff wrote: > > > When decoding a group mod, the current code validates the group type and > > command after the whole group mod has been decoded. The OF1.5 decoder, > > however, tries to use the type and command earlier, when it might still be > > invalid. This caused an assertion failure (via OVS_NOT_REACHED). This > > commit fixes the problem. > > > > ovs-vswitchd does not enable support for OpenFlow 1.5 by default. > > > > Reported-at: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=9249 > > Signed-off-by: Ben Pfaff > > --- > > lib/ofp-group.c | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/lib/ofp-group.c b/lib/ofp-group.c > > index 697208f0e9f6..f6e5242f5244 100644 > > --- a/lib/ofp-group.c > > +++ b/lib/ofp-group.c > > @@ -1547,7 +1547,7 @@ parse_group_prop_ntr_selection_method(struct ofpbuf > > *payload, > > "only allowed for select groups"); > > return OFPERR_OFPBPC_BAD_VALUE; > > default: > > -OVS_NOT_REACHED(); > > +return OFPERR_OFPGMFC_BAD_TYPE; > > } > > > > switch (group_cmd) { > > @@ -1562,7 +1562,7 @@ parse_group_prop_ntr_selection_method(struct ofpbuf > > *payload, > > "only allowed for add and delete group > > modifications"); > > return OFPERR_OFPBPC_BAD_VALUE; > > default: > > -OVS_NOT_REACHED(); > > +return OFPERR_OFPGMFC_BAD_COMMAND; > > } > > > > if (payload->size < sizeof *prop) { > > -- > > 2.16.1 > > > > ___ > > 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] ofp-group: Don't assert-fail decoding bad OF1.5 group mod type or command.
Looks good to me. Thanks. Reviewed-by: Yifeng Sun On Thu, Jul 5, 2018 at 3:28 PM, Ben Pfaff wrote: > When decoding a group mod, the current code validates the group type and > command after the whole group mod has been decoded. The OF1.5 decoder, > however, tries to use the type and command earlier, when it might still be > invalid. This caused an assertion failure (via OVS_NOT_REACHED). This > commit fixes the problem. > > ovs-vswitchd does not enable support for OpenFlow 1.5 by default. > > Reported-at: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=9249 > Signed-off-by: Ben Pfaff > --- > lib/ofp-group.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/lib/ofp-group.c b/lib/ofp-group.c > index 697208f0e9f6..f6e5242f5244 100644 > --- a/lib/ofp-group.c > +++ b/lib/ofp-group.c > @@ -1547,7 +1547,7 @@ parse_group_prop_ntr_selection_method(struct ofpbuf > *payload, > "only allowed for select groups"); > return OFPERR_OFPBPC_BAD_VALUE; > default: > -OVS_NOT_REACHED(); > +return OFPERR_OFPGMFC_BAD_TYPE; > } > > switch (group_cmd) { > @@ -1562,7 +1562,7 @@ parse_group_prop_ntr_selection_method(struct ofpbuf > *payload, > "only allowed for add and delete group > modifications"); > return OFPERR_OFPBPC_BAD_VALUE; > default: > -OVS_NOT_REACHED(); > +return OFPERR_OFPGMFC_BAD_COMMAND; > } > > if (payload->size < sizeof *prop) { > -- > 2.16.1 > > ___ > 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] dpif-netlink-rtnl: Retry a smaller MTU for netdev when MAX_MTU is too large.
On 7/5/2018 11:55 AM, Yifeng Sun wrote: When MAX_MTU is larger than hw supported max MTU, dpif_netlink_rtnl_create will fail. This leads to testing failure '11: datapath - ping over gre tunnel' in 'make check-kmod'. This patch fixes this issue by retrying a smaller MTU when MAX_MTU is too large. Signed-off-by: Yifeng Sun --- lib/dpif-netlink-rtnl.c | 27 ++- 1 file changed, 18 insertions(+), 9 deletions(-) diff --git a/lib/dpif-netlink-rtnl.c b/lib/dpif-netlink-rtnl.c index bec3fce91e38..8304e7fb3826 100644 --- a/lib/dpif-netlink-rtnl.c +++ b/lib/dpif-netlink-rtnl.c @@ -282,6 +282,19 @@ dpif_netlink_rtnl_verify(const struct netdev_tunnel_config *tnl_cfg, } static int +rtnl_set_mtu(const char *name, uint32_t mtu, struct ofpbuf *request) +{ +ofpbuf_clear(request); +nl_msg_put_nlmsghdr(request, 0, RTM_SETLINK, +NLM_F_REQUEST | NLM_F_ACK); +ofpbuf_put_zeros(request, sizeof(struct ifinfomsg)); +nl_msg_put_string(request, IFLA_IFNAME, name); +nl_msg_put_u32(request, IFLA_MTU, mtu); + +return nl_transact(NETLINK_ROUTE, request, NULL); +} + +static int dpif_netlink_rtnl_create(const struct netdev_tunnel_config *tnl_cfg, const char *name, enum ovs_vport_type type, const char *kind, uint32_t flags) @@ -354,15 +367,11 @@ dpif_netlink_rtnl_create(const struct netdev_tunnel_config *tnl_cfg, type == OVS_VPORT_TYPE_IP6GRE)) { /* Work around a bug in kernel GRE driver, which ignores IFLA_MTU in * RTM_NEWLINK, by setting the MTU again. See - * https://bugzilla.redhat.com/show_bug.cgi?id=1488484. */ -ofpbuf_clear(); -nl_msg_put_nlmsghdr(, 0, RTM_SETLINK, -NLM_F_REQUEST | NLM_F_ACK); -ofpbuf_put_zeros(, sizeof(struct ifinfomsg)); -nl_msg_put_string(, IFLA_IFNAME, name); -nl_msg_put_u32(, IFLA_MTU, MAX_MTU); - -int err2 = nl_transact(NETLINK_ROUTE, , NULL); + * https://bugzilla.redhat.com/show_bug.cgi?id=1488484. + * + * In case of MAX_MTU exceeds hw max MTU, retry a smaller value. */ +int err2 = rtnl_set_mtu(name, MAX_MTU, ) && + rtnl_set_mtu(name, 1450, ); if (err2) { static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5); This has been bugging me forever and I just never get around to fixing it. Thanks!!! - Greg ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v2] ovn: Allow for automatic dynamic updates of IPAM
On Mon, Jun 25, 2018 at 04:09:53PM -0400, Mark Michelson wrote: > OVN offers a method of IP address management that allows for an IPv4 subnet or > IPv6 prefix to be specified on a logical switch. Then by specifying a > switch port's address as "dynamic" or " dynamic", OVN will > automatically assign addresses to the switch port. > > While this works great for initial assignment of addresses, addresses do > not automatically adjust when changes are made to the switch's > configuration. For instance: > * If the subnet, ipv6_prefix, or exclude_ips for a logical switch > changes, the affected switch ports are not updated. > * If a switch port with a static IP address is added to the switch, and > that address conflicts with a dynamically assigned IP address, the > dynamic address is not updated. > * If a MAC address switched from being statically assigned to > dynamically assigned, the MAC address would not be updated. > * If a statically assigned MAC address changed, then the IPv6 address > would not be updated. > > This patch solves all of the above issues by changing the algorithm for > IPAM assignment. There are essentially three steps. > 1) While joining logical ports, all statically-assigned addresses (i.e. > any ports without "dynamic" addresses) have their addresses registered > to IPAM. This gives them top priority. > 2) All logical ports with dynamic addresses are inspected. Any changes > that must be made to the addresses are collected to be made later. Any > addresses that do not require change are registered to IPAM. This allows > for previously assigned dynamic addresses to be kept. > 3) All gathered changes are enacted. > > The change contains new tests that ensure that dynamic addresses are > updated when appropriate. > > This patch also alters some existing IPAM tests. Those tests assumed > that dynamic addresses would not be updated automatically, so those > tests either had to be altered or removed. > > Signed-off-by: Mark Michelson > --- > v2->v3: > Fixed a checkpatch problem (line too long) > > v1->v2: > Rebased Thanks for the new version. I spent some time trying to understand this. I think one of my issues with it is that there is an unstated assumption that a logical switch port has at most one dynamic address, but nothing that checks for or enforces it. It's possible to request more than one if you put multiple "xx:xx:xx:xx:xx:xx dynamic" entries in an addresses column, but I don't think that the code really treats them properly since it tries to independently diff each one of them against the current set of dynamically assigned addresses. Is this all correct? If so, would you figure out some way to fix it? I'm appending some incrementals that make sense to me. I removed 'od' from the struct because it's redundant with op->od. I moved the ovs_list member to the top because that's where we usually put a member used to show membership in a larger structure, and renamed it from 'list' to 'node' because this ovs_list is used for membership not for containment. I also added a check in build_ipam() that op->nbsp == nbsp, which I think the code assumed and might be ovs_assert()'able but at least checking on it seems wise. If this makes sense, would you mind working on a v4? Thanks, Ben. --8<--cut here-->8-- diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c index 12c9929da039..80eb0e6bcb3a 100644 --- a/ovn/northd/ovn-northd.c +++ b/ovn/northd/ovn-northd.c @@ -1065,10 +1065,11 @@ enum dynamic_update_type { }; struct dynamic_address_update { +struct ovs_list node; /* In build_ipam()'s list of updates. */ + struct ovn_port *op; -struct ovn_datapath *od; + struct lport_addresses current_addresses; -struct ovs_list list; struct eth_addr static_mac; enum dynamic_update_type mac; enum dynamic_update_type ipv4; @@ -1102,7 +1103,7 @@ dynamic_mac_changed(const char *lsp_addresses, static enum dynamic_update_type dynamic_ip4_changed(struct dynamic_address_update *update) { -bool dynamic_ip4 = update->od->ipam_info.allocated_ipv4s != NULL; +bool dynamic_ip4 = update->op->od->ipam_info.allocated_ipv4s != NULL; if (!dynamic_ip4) { if (update->current_addresses.n_ipv4_addrs) { @@ -1118,13 +1119,13 @@ dynamic_ip4_changed(struct dynamic_address_update *update) } uint32_t ip4 = ntohl(update->current_addresses.ipv4_addrs[0].addr); -if (ip4 < update->od->ipam_info.start_ipv4) { +if (ip4 < update->op->od->ipam_info.start_ipv4) { return DYNAMIC; } -uint32_t index = ip4 - update->od->ipam_info.start_ipv4; -if (index > update->od->ipam_info.total_ipv4s || -bitmap_is_set(update->od->ipam_info.allocated_ipv4s, index)) { +uint32_t index = ip4 - update->op->od->ipam_info.start_ipv4; +if (index > update->op->od->ipam_info.total_ipv4s || +bitmap_is_set(update->op->od->ipam_info.allocated_ipv4s,
Re: [ovs-dev] [PATCH] oss-fuzz: Move oss-fuzz test harnesses and fuzzer configs to ovs source repo
> Is that the preferred way to do it? It seems a little ad hoc. Another > way would be to add a target to the OVS build tree, so that the script > could just become something like > > ./boot.sh && ./configure && make -j$(nproc) tests/oss-fuzz/fuzzer > > or whatever. This is cleaner and my preference as well. How do I go about doing this? > > But, maybe this ad hoc build method is conventional for oss-fuzz? I'm > not familiar with their usual processes. I don't think Google cares how targets are built although a lot of targets are currently built in an ad-hoc way. > Well, either they have to have #include or we need to > blacklist these files, one or the other. The former is probably > harmless and possibly helpful. Let's add a make target for oss-fuzz and and everything that is needed to get that done. I suppose this means I add a #include as the first line of each of the fuzz test harnesses? >>> The new files need to get mentioned in an automake.mk, at least in >>> EXTRA_DIST, to ensure that "make dist" will put them into the tarball: >>> >>> The following files are in git but not the distribution: >>> tests/oss-fuzz/config/flow_extract_fuzzer.options >>> tests/oss-fuzz/config/json_parser_fuzzer.options >>> tests/oss-fuzz/config/ofp_print_fuzzer.options >>> tests/oss-fuzz/config/ovs.dict >>> tests/oss-fuzz/flow_extract_target.c >>> tests/oss-fuzz/json_parser_target.c >>> tests/oss-fuzz/ofp_print_target.c >> >> There are several automake files to choose from. How do I do this? > > I'd add a new tests/oss-fuzz/automake.mk and then include that in > tests/automake.mk. I am not sure about this step. I will get back to you once we have a make target ready. Thanks. Bhargava ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH 2/2] ovs-sandbox: Fix ovs-appctl for ovn-northd and ovn-controller.
> On Jul 6, 2018, at 5:04 PM, Ben Pfaff wrote: > > On Sun, Jul 01, 2018 at 06:23:01PM -0700, Justin Pettit wrote: >> Commits 1e8eeb66db2e7 ("ovs-sandbox: Support starting multiple >> ovn-northds.") and 047458de40391 ("ovs-sandbox: Add option to support >> multiple ovn-controllers.") allowed starting multiple instances of >> ovn-northd and ovn-controller, respectively. It did this by assigning a >> sequence number to to the pidfile name. Unfortunately, this breaks the >> method ovs-appctl uses to determine to which process it should connect. >> This commit changes the behavior so that a sequence number is not added >> to the first instance, so ovs-appctl will connect to that be default. >> >> This commit also uses the same convention for naming the log file. >> >> Signed-off-by: Justin Pettit > > Acked-by: Ben Pfaff Thanks. I pushed the series to master. --Justin ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] DNS: Fix build issue with dns_resolve_init().
> On Jul 6, 2018, at 5:52 PM, Ben Pfaff wrote: > > On Fri, Jul 06, 2018 at 02:51:08PM -0700, Justin Pettit wrote: >> CC: Yifeng Sun >> Fixes: 771680d96f ("DNS: Add basic support for asynchronous DNS resolving") >> Signed-off-by: Justin Pettit > > Acked-by: Ben Pfaff Thanks. I pushed it with yours and Ben's Acked-by. --Justin ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] DNS: Fix build issue with dns_resolve_init().
On Fri, Jul 06, 2018 at 02:51:08PM -0700, Justin Pettit wrote: > CC: Yifeng Sun > Fixes: 771680d96f ("DNS: Add basic support for asynchronous DNS resolving") > Signed-off-by: Justin Pettit Acked-by: Ben Pfaff ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] DNS: Fix build issue with dns_resolve_init().
Good catch! Thanks. Reviewed-by: Yifeng Sun On Fri, Jul 6, 2018 at 2:51 PM, Justin Pettit wrote: > CC: Yifeng Sun > Fixes: 771680d96f ("DNS: Add basic support for asynchronous DNS resolving") > Signed-off-by: Justin Pettit > --- > lib/dns-resolve-stub.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/lib/dns-resolve-stub.c b/lib/dns-resolve-stub.c > index edf8337be1d9..859c76f6b869 100644 > --- a/lib/dns-resolve-stub.c > +++ b/lib/dns-resolve-stub.c > @@ -19,7 +19,7 @@ > #include "compiler.h" > > void > -dns_resolve_init(void) > +dns_resolve_init(bool is_daemon OVS_UNUSED) > { > } > > -- > 2.17.1 > > ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH] DNS: Fix build issue with dns_resolve_init().
CC: Yifeng Sun Fixes: 771680d96f ("DNS: Add basic support for asynchronous DNS resolving") Signed-off-by: Justin Pettit --- lib/dns-resolve-stub.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/dns-resolve-stub.c b/lib/dns-resolve-stub.c index edf8337be1d9..859c76f6b869 100644 --- a/lib/dns-resolve-stub.c +++ b/lib/dns-resolve-stub.c @@ -19,7 +19,7 @@ #include "compiler.h" void -dns_resolve_init(void) +dns_resolve_init(bool is_daemon OVS_UNUSED) { } -- 2.17.1 ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH 1/6] NEWS: Correct release date of OVN QoS meter support.
> On Jul 6, 2018, at 5:07 PM, Ben Pfaff wrote: > > On Fri, Jun 29, 2018 at 12:13:49PM -0700, Justin Pettit wrote: >> It occured in OVS 2.9.0, not OVS 2.8.0. >> >> Signed-off-by: Justin Pettit > > For the series: > Acked-by: Ben Pfaff Thanks! I pushed the series to master. I also applied the first four patches to branch-2.9. --Justin ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH 0/5] mac-learning: Add additional counters for troubleshooting aid
On Mon, Jun 25, 2018 at 12:57:08PM +0200, Eelco Chaudron wrote: > This series add some additional counters that make FDB debugging easier. > The following counters now exists on a per mac_learning instance, and > the last two have also been added as coverage counters. Thanks for making OVS easier to debug. I applied this series to master. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] utilities: Add upcall related commands to the GDB script
On Mon, Jul 02, 2018 at 02:00:10PM +0200, Eelco Chaudron wrote: > This commit adds ovs_dump_udpif_keys and ovs_show_upcall commands to the GDB > script. > Here are some examples of the output: > > (gdb) ovs_show_upcall > netdev@ovs-netdev: > flows : (current 0) (avg 0) (max 0) (limit 1) > dump duration : 1ms > ufid enabled : true > > 39: (keys 0) > 42: (keys 0) > 41: (keys 0) > 43: (keys 0) > 44: (keys 0) > 45: (keys 0) > > system@ovs-system: > flows : (current 4000) (avg 4031) (max 4064) (limit 139000) > dump duration : 4ms > ufid enabled : true > > 99: (keys 676) > 102: (keys 665) > 101: (keys 656) > 103: (keys 648) > 104: (keys 642) > 105: (keys 713) > > > (gdb) ovs_dump_udpif_keys > (struct udpif *) 0x1ebb830: name = netdev@ovs-netdev, total keys = 2 > (struct udpif *) 0x20c6f00: name = system@ovs-system, total keys = 0 > > (gdb) ovs_dump_udpif_keys 0x1ebb830 > (struct umap *) 0x1ef9328: > (struct udpif_key *) 0x7f36e0004e40: key_len = 132, mask_len = 144 >ufid = > 3e529416-83bf-bab4-5c6e-421127a9143a >hash = 0x3d96b11d, pmd_id = 1 >state = UKEY_OPERATIONAL >n_packets = 2, n_bytes = 68 >used = 1397047436, tcp_flags = 0x > (struct umap *) 0x1efb740: > (struct udpif_key *) 0x7f36dc004c20: key_len = 132, mask_len = 144 >ufid = > ee98d69f-8298-04dd-844a-4d2abee9f773 >hash = 0x2e8077c2, pmd_id = 15 >state = UKEY_OPERATIONAL >n_packets = 0, n_bytes = 0 >used = 0, tcp_flags = 0x > > (gdb) ovs_dump_udpif_keys 0x1ebb830 short > (struct umap *) 0x1ef9328: > (struct udpif_key *) 0x7f36e0004e40: > (struct umap *) 0x1efb740: > (struct udpif_key *) 0x7f36dc004c20: > > Signed-off-by: Eelco Chaudron Applied to master, thanks! ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH 1/4] ovs-dev: Detect Out-Of-Resource condition on a netdev
On Fri, Jul 06, 2018 at 04:10:47PM +0530, Sriharsha Basavapatna via dev wrote: > This is the first patch in the patch-set to support dynamic rebalancing > of offloaded flows. > > The patch detects OOR condition on a netdev port when ENOSPC error is > returned by TC-Flower while adding a flow rule. A new structure is added > to the netdev called "netdev_hw_info", to store OOR related information > required to perform dynamic offload-rebalancing. > > Signed-off-by: Sriharsha Basavapatna > Co-authored-by: Venkat Duvvuru > Signed-off-by: Venkat Duvvuru > Reviewed-by: Sathya Perla I see that the robot already pointed out build errors in the first two patches. Please do correct those and post v2. As a trivial additional note, "ovs-dev:" doesn't make sense as a prefix. Please choose a prefix for each patch that reflects the part of OVS that it primarily affects. Thanks, Ben. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH 1/6] NEWS: Correct release date of OVN QoS meter support.
On Fri, Jun 29, 2018 at 12:13:49PM -0700, Justin Pettit wrote: > It occured in OVS 2.9.0, not OVS 2.8.0. > > Signed-off-by: Justin Pettit For the series: Acked-by: Ben Pfaff ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH 2/2] ovs-sandbox: Fix ovs-appctl for ovn-northd and ovn-controller.
On Sun, Jul 01, 2018 at 06:23:01PM -0700, Justin Pettit wrote: > Commits 1e8eeb66db2e7 ("ovs-sandbox: Support starting multiple > ovn-northds.") and 047458de40391 ("ovs-sandbox: Add option to support > multiple ovn-controllers.") allowed starting multiple instances of > ovn-northd and ovn-controller, respectively. It did this by assigning a > sequence number to to the pidfile name. Unfortunately, this breaks the > method ovs-appctl uses to determine to which process it should connect. > This commit changes the behavior so that a sequence number is not added > to the first instance, so ovs-appctl will connect to that be default. > > This commit also uses the same convention for naming the log file. > > Signed-off-by: Justin Pettit Acked-by: Ben Pfaff ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH 1/2] ovs-sandbox: Use different log file names for ovn-controllers.
On Sun, Jul 01, 2018 at 06:23:00PM -0700, Justin Pettit wrote: > Commit 047458de40391 ("ovs-sandbox: Add option to support multiple > ovn-controllers.") allowed creating multiple instances of > ovn-controller. However, all instances would use the same log file > name. This commit uses the sequence number to name the log file. > > Signed-off-by: Justin Pettit Acked-by: Ben Pfaff ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] OVS DPDK: dpdk_merge pull request for master
On Fri, Jul 06, 2018 at 03:55:50PM +0100, Ian Stokes wrote: > The following changes since commit 23626bcf3bd987f7a5e03b93bec8450b10421c31: > > OVN: add ICMPv6 time exceeded support to OVN logical router (2018-07-05 > 15:46:09 -0700) > > are available in the git repository at: > > https://github.com/istokes/ovs dpdk_merge > > for you to fetch changes up to 43307ad0e2543c9c8443f3ab1150ab03f4eb551c: > > dpdk: Support both shared and per port mempools. (2018-07-06 12:46:26 > +0100) Thanks, merged into master. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] OVS DPDK: dpdk_merge pull request for branch-2.9
On Fri, Jul 06, 2018 at 03:56:15PM +0100, Ian Stokes wrote: > The following changes since commit 0ed5bf7577bec8ddbe551648f038ea1dec1ced96: > > ofp-actions: Fix undefined behavior shifting 'int' 16 places left. > (2018-07-05 15:10:44 -0700) > > are available in the git repository at: > > https://github.com/istokes/ovs dpdk_merge_2_9 > > for you to fetch changes up to ac2fb7d4fae5ad6d37dde4d1d3f0907e6f950329: > > dpdk: Use DPDK 17.11.3 release. (2018-07-06 10:52:54 +0100) Merged, thanks! ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCHv5] DNS: Add basic support for asynchronous DNS resolving
On Tue, Jun 26, 2018 at 02:06:21PM -0700, Yifeng Sun wrote: > This patch is a simple implementation for the proposal discussed in > > https://mail.openvswitch.org/pipermail/ovs-dev/2017-August/337038.html and > > https://mail.openvswitch.org/pipermail/ovs-dev/2017-October/340013.html. > > > > It enables ovs-vswitchd and other utilities to use DNS names when specifying > > OpenFlow and OVSDB remotes. > I applied this to master. Thanks! ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCHv2] dpif-netlink-rtnl: Retry smaller MTU when default MAX_MTU is too large.
On Fri, Jul 06, 2018 at 08:28:27AM -0700, Yifeng Sun wrote: > When MAX_MTU is larger than hw supported max MTU, > dpif_netlink_rtnl_create will fail. This leads to > testing failure '11: datapath - ping over gre tunnel' > in 'make check-kmod'. > > This patch fixes this issue by retrying a smaller MTU > when MAX_MTU is too large. > > Signed-off-by: Yifeng Sun > --- > v1 -> v2: Fixed a bug pointed out by Ben. (Thanks Ben). Thanks! Applied to master. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH 0/2] add IPv6 port unreachable support to OVN logical router
On Fri, Jul 06, 2018 at 06:12:16PM +0200, Lorenzo Bianconi wrote: > Add ICMPv6 port unreachable messages in reply to IPv6 packets (not TCP) > directed to the logical router's IP addresses Thanks, I applied this series to master. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCHv2] dpif-netlink-rtnl: Retry smaller MTU when default MAX_MTU is too large.
When MAX_MTU is larger than hw supported max MTU, dpif_netlink_rtnl_create will fail. This leads to testing failure '11: datapath - ping over gre tunnel' in 'make check-kmod'. This patch fixes this issue by retrying a smaller MTU when MAX_MTU is too large. Signed-off-by: Yifeng Sun --- v1 -> v2: Fixed a bug pointed out by Ben. (Thanks Ben). lib/dpif-netlink-rtnl.c | 29 - 1 file changed, 20 insertions(+), 9 deletions(-) diff --git a/lib/dpif-netlink-rtnl.c b/lib/dpif-netlink-rtnl.c index bec3fce91e38..2e23a8c14fcf 100644 --- a/lib/dpif-netlink-rtnl.c +++ b/lib/dpif-netlink-rtnl.c @@ -282,6 +282,19 @@ dpif_netlink_rtnl_verify(const struct netdev_tunnel_config *tnl_cfg, } static int +rtnl_set_mtu(const char *name, uint32_t mtu, struct ofpbuf *request) +{ +ofpbuf_clear(request); +nl_msg_put_nlmsghdr(request, 0, RTM_SETLINK, +NLM_F_REQUEST | NLM_F_ACK); +ofpbuf_put_zeros(request, sizeof(struct ifinfomsg)); +nl_msg_put_string(request, IFLA_IFNAME, name); +nl_msg_put_u32(request, IFLA_MTU, mtu); + +return nl_transact(NETLINK_ROUTE, request, NULL); +} + +static int dpif_netlink_rtnl_create(const struct netdev_tunnel_config *tnl_cfg, const char *name, enum ovs_vport_type type, const char *kind, uint32_t flags) @@ -354,15 +367,13 @@ dpif_netlink_rtnl_create(const struct netdev_tunnel_config *tnl_cfg, type == OVS_VPORT_TYPE_IP6GRE)) { /* Work around a bug in kernel GRE driver, which ignores IFLA_MTU in * RTM_NEWLINK, by setting the MTU again. See - * https://bugzilla.redhat.com/show_bug.cgi?id=1488484. */ -ofpbuf_clear(); -nl_msg_put_nlmsghdr(, 0, RTM_SETLINK, -NLM_F_REQUEST | NLM_F_ACK); -ofpbuf_put_zeros(, sizeof(struct ifinfomsg)); -nl_msg_put_string(, IFLA_IFNAME, name); -nl_msg_put_u32(, IFLA_MTU, MAX_MTU); - -int err2 = nl_transact(NETLINK_ROUTE, , NULL); + * https://bugzilla.redhat.com/show_bug.cgi?id=1488484. + * + * In case of MAX_MTU exceeds hw max MTU, retry a smaller value. */ +int err2 = rtnl_set_mtu(name, MAX_MTU, ); +if (err2) { +err2 = rtnl_set_mtu(name, 1450, ); +} if (err2) { static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5); -- 2.7.4 ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] OVN: add unit test for ICMPv6 TTL exceeded
On Fri, Jul 06, 2018 at 07:07:32PM +0200, Lorenzo Bianconi wrote: > Add unit test for the ICMPv6 TTL exceeded packet sent by OVN > logical router when it receives an IPv6 packet whose TTL has > expired (ip.ttl == {0, 1}) > > Signed-off-by: Lorenzo Bianconi Thanks, applied to master. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] dpif-netlink-rtnl: Retry a smaller MTU for netdev when MAX_MTU is too large.
Oh yes, I missed that when I tried to make code look compact. Thanks for pointing it out. Yifeng On Fri, Jul 6, 2018 at 1:17 PM, Ben Pfaff wrote: > On Thu, Jul 05, 2018 at 11:55:46AM -0700, Yifeng Sun wrote: > > When MAX_MTU is larger than hw supported max MTU, > > dpif_netlink_rtnl_create will fail. This leads to > > testing failure '11: datapath - ping over gre tunnel' > > in 'make check-kmod'. > > > > This patch fixes this issue by retrying a smaller MTU > > when MAX_MTU is too large. > > > > Signed-off-by: Yifeng Sun > > ... > > > -int err2 = nl_transact(NETLINK_ROUTE, , NULL); > > + * https://bugzilla.redhat.com/show_bug.cgi?id=1488484. > > + * > > + * In case of MAX_MTU exceeds hw max MTU, retry a smaller > value. */ > > +int err2 = rtnl_set_mtu(name, MAX_MTU, ) && > > + rtnl_set_mtu(name, 1450, ); > > Probably, this should be more like: > > int err2 = rtnl_set_mtu(name, MAX_MTU, ); > if (err2) { > err2 = rtnl_set_mtu(name, 1450, ); > } > > because if you use && then the result is 0 or 1, not 0 or an errno > value. > > Thanks, > > Ben. > ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] dpif-netlink-rtnl: Retry a smaller MTU for netdev when MAX_MTU is too large.
On Thu, Jul 05, 2018 at 11:55:46AM -0700, Yifeng Sun wrote: > When MAX_MTU is larger than hw supported max MTU, > dpif_netlink_rtnl_create will fail. This leads to > testing failure '11: datapath - ping over gre tunnel' > in 'make check-kmod'. > > This patch fixes this issue by retrying a smaller MTU > when MAX_MTU is too large. > > Signed-off-by: Yifeng Sun ... > -int err2 = nl_transact(NETLINK_ROUTE, , NULL); > + * https://bugzilla.redhat.com/show_bug.cgi?id=1488484. > + * > + * In case of MAX_MTU exceeds hw max MTU, retry a smaller value. */ > +int err2 = rtnl_set_mtu(name, MAX_MTU, ) && > + rtnl_set_mtu(name, 1450, ); Probably, this should be more like: int err2 = rtnl_set_mtu(name, MAX_MTU, ); if (err2) { err2 = rtnl_set_mtu(name, 1450, ); } because if you use && then the result is 0 or 1, not 0 or an errno value. Thanks, Ben. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v2] ofproto-dpif-xlate: Fix packet_in reason for Table-miss rule
On Wed, Jul 04, 2018 at 08:40:53PM +0530, Keshav Gupta wrote: > Currently in OvS if we hit "Table-miss" rules (associated with Controller > action) then we send PACKET_IN message to controller with reason as > OFPR_NO_MATCH. > > “Table-miss” rule is one whose priority is 0 and its catch all rule. > > But if we hit same "Table-miss" rule after executing group entry we will > send the reason as OFPR_ACTION (for OF1.3 and below) and OFPR_GROUP > (for OF1.4 and above). > > This is because once we execute group entry we set ctx->in_group and later > when we hit the "Table-miss" rule, Since ctx->in_group is set we send > reason as OFPR_ACTION (for OF1.3) and OFPR_GROUP (for OF1.4 and above). > > For eg: for the following pipeline, we will send the reason as OFPR_ACTION > even if we hit The “Table-miss” rule. > > cookie=0x800, duration=761.189s, table=0, n_packets=1401, n_bytes=67954, > priority=4,in_port=9,vlan_tci=0x/0x1fff > actions=write_metadata:0x678700/0xff01,goto_table:17 > > cookie=0x681, duration=768.848s, table=17, n_packets=1418, n_bytes=68776, > priority=10,metadata=0x678700/0xff00 > actions=write_metadata:0xe0678700/0xfffe,goto_table:60 > > cookie=0x680, duration=24944.312s, table=60, n_packets=58244, > n_bytes=2519520, priority=0 actions=resubmit(,17) > > cookie=0x804, duration=785.733s, table=17, n_packets=1450, n_bytes=69724, > priority=10,metadata=0xe0678700/0xff00 > actions=write_metadata:0x67871d4d00/0xfffe,goto_table:43 > > cookie=0x822002d, duration=24960.795s, table=43, n_packets=53097, > n_bytes=2230074, priority=100,arp,arp_op=1 actions=group:6000 > > group_id=6000,type=all,bucket=actions=CONTROLLER:65535, > bucket=actions=resubmit(,48), bucket=actions=resubmit(,81) > > cookie=0x850, duration=24977.323s, table=48, n_packets=58309, > n_bytes=2522634, > priority=0 actions=resubmit(,49),resubmit(,50) > > cookie=0x805, duration=24984.679s, table=50, n_packets=6, n_bytes=264, > priority=0 actions=CONTROLLER:65535 > > Currently we are sending table_id as 50 and packet_in reason as OFPR_ACTION. > Instead of sending packet_in reason as OFPR_NO_MATCH. > > Signed-off-by: Keshav Gupta > Co-authored-by: Rohith Basavaraja > Signed-off-by: Rohith Basavaraja Thanks! I applied this to master. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] ovs-kmod spec files
Hi Vlad, We’re actually using the rhel6 spec file to build RPMs to be installed on RHEL 7.2,7.3,and 7.4 in production. Martin On Fri, Jul 6, 2018 at 10:31 AM Odintsov Vladislav wrote: > Hi all, > > > I'm trying to understand the needs to have openvswitch-kmod-fedora.spec > and openvswitch-kmod-rhel6.spec simultaneously. > > Is it still possible to build master branch for el6 dists (CentOS/RH/OEL > 6)? > > > I'm going to make this PR up to day: > https://github.com/openvswitch/ovs/pull/169 and want to understand does > community need both spec-files. > > Thanks for your thoughts. > > > ___ > > Regards, Vlad. > ___ > 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] dpif-netlink-rtnl: Retry a smaller MTU for netdev when MAX_MTU is too large.
Hi Ben, Yes, this is GRE-specific, somehow gre_sys doesn't support current MTU which is 65000. `make check-kmod` no. 11 and 21 fail due to this issue. On the kernel side, the error shows up as: [ 6135.11] gre_sys: Invalid MTU 65000 requested, hw max 1500 It reflects to userspace as error below: dpif_netlink_rtnl|WARN|setting MTU of tunnel gre_sys failed (Invalid argument) Thanks, Yifeng On Fri, Jul 6, 2018 at 10:58 AM, Ben Pfaff wrote: > On Thu, Jul 05, 2018 at 11:55:46AM -0700, Yifeng Sun wrote: > > When MAX_MTU is larger than hw supported max MTU, > > dpif_netlink_rtnl_create will fail. This leads to > > testing failure '11: datapath - ping over gre tunnel' > > in 'make check-kmod'. > > > > This patch fixes this issue by retrying a smaller MTU > > when MAX_MTU is too large. > > > > Signed-off-by: Yifeng Sun > > Is this GRE-specific? (Why would only GRE care about large MTUs?) > ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] dpif-netlink-rtnl: Retry a smaller MTU for netdev when MAX_MTU is too large.
On Thu, Jul 05, 2018 at 11:55:46AM -0700, Yifeng Sun wrote: > When MAX_MTU is larger than hw supported max MTU, > dpif_netlink_rtnl_create will fail. This leads to > testing failure '11: datapath - ping over gre tunnel' > in 'make check-kmod'. > > This patch fixes this issue by retrying a smaller MTU > when MAX_MTU is too large. > > Signed-off-by: Yifeng Sun Is this GRE-specific? (Why would only GRE care about large MTUs?) ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] Symantec Users List
Hi, I would like to know if you are interested in acquiring Symantec Users List. We alos have other types of technology users like: McAfee Users, Trend Micro Users, Kaspersky Lab Users, Sophos Users, LifeLock Users, FireEye Users, Comodo Users, CISCO Users, Cylance Users, Veritas Software Users, BitDefender Users, Webroot Users, ESET Users, F-Secure Users, VMware Users, Fortinet Users and many more.. Contact Field: Contact Name, Title, Verified Email, Phone, Fax Number, Company Name, Company URL, Company physical address, SIC Code, Industry Vertical, Technology Type, Company Size (Revenue and Employee) and LinkedIn URLs. My goal today is to help you find suitable solutions to your challenges as I am confident that my data is relevant and GDPR compliant. Let me know your target geography and target title if you are interested so that I will get back with exact counts and price list sample file Best Regards, Alex John Data Consultant To opt out, please reply with Leave Out in the Subject Line ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [ovs-discuss] openvswitch-kmod: on uninstall, get depmod: ERROR: fstatat
We're working on a fix for this right now. Thanks, - Greg On 6/18/2018 8:14 PM, Paul Greenberg wrote: Build Fedora RPM from 2.9.90. On "yum remove", get the following errors: Erasing: openvswitch-kmod-2.9.90-1.el7.x86_64 3/6 depmod: ERROR: fstatat(4, vport-gre.ko): No such file or directory depmod: ERROR: fstatat(4, vport-stt.ko): No such file or directory depmod: ERROR: fstatat(4, vport-geneve.ko): No such file or directory depmod: ERROR: fstatat(4, vport-lisp.ko): No such file or directory depmod: ERROR: fstatat(4, vport-vxlan.ko): No such file or directory depmod: ERROR: fstatat(4, openvswitch.ko): No such file or directory depmod: ERROR: fstatat(4, vport-gre.ko): No such file or directory depmod: ERROR: fstatat(4, vport-stt.ko): No such file or directory depmod: ERROR: fstatat(4, vport-geneve.ko): No such file or directory depmod: ERROR: fstatat(4, vport-lisp.ko): No such file or directory depmod: ERROR: fstatat(4, vport-vxlan.ko): No such file or directory rmdir: failed to remove '.': Invalid argument rmdir: failed to remove '.': Invalid argument rmdir: failed to remove '.': Invalid argument rmdir: failed to remove '.': Invalid argument rmdir: failed to remove '.': Invalid argument rmdir: failed to remove '.': Invalid argument rmdir: failed to remove '.': Invalid argument rmdir: failed to remove '.': Invalid argument rmdir: failed to remove '.': Invalid argument rmdir: failed to remove '.': Invalid argument rmdir: failed to remove '.': Invalid argument ___ discuss mailing list disc...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-discuss ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] ovs-kmod spec files
Hi all, I'm trying to understand the needs to have openvswitch-kmod-fedora.spec and openvswitch-kmod-rhel6.spec simultaneously. Is it still possible to build master branch for el6 dists (CentOS/RH/OEL 6)? I'm going to make this PR up to day: https://github.com/openvswitch/ovs/pull/169 and want to understand does community need both spec-files. Thanks for your thoughts. ___ Regards, Vlad. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] oss-fuzz: Move oss-fuzz test harnesses and fuzzer configs to ovs source repo
On Fri, Jul 06, 2018 at 11:06:50AM +0200, Bhargava Shastry wrote: > > We do need a Signed-off-by on any patch. Can you provide one? > > Sure, this should not be a problem. I'm not sure how the patch was > generated from my pull request. I did a "git pull" on your repo, then "git send-email" generated the patch. > Since you prefer patches, I can switch to git-format-patch or > something like that to take this forward. I will do this once I have a > new patch ready that addresses your comments. Thanks! > > Normally I expect new code to be mentioned in some makefile. It's not > > obvious to me how this gets built. Does it get built by oss-fuzz itself > > somehow? Or are you building it by hand? Or something else? > > The new code (test harnesses) get built from a build script inside the > oss-fuzz repo: > https://github.com/google/oss-fuzz/blob/master/projects/openvswitch/build.sh Is that the preferred way to do it? It seems a little ad hoc. Another way would be to add a target to the OVS build tree, so that the script could just become something like ./boot.sh && ./configure && make -j$(nproc) tests/oss-fuzz/fuzzer or whatever. But, maybe this ad hoc build method is conventional for oss-fuzz? I'm not familiar with their usual processes. > > This causes some build breakage at "make" time. I'll walk you through > > it. First, we normally expect #include at the beginning of > > each file. Since I don't understand how this gets built, maybe that's > > not appropriate (but in that case we need to update our blacklist for > > this rule): > > > > tests/oss-fuzz/flow_extract_target.c > > tests/oss-fuzz/json_parser_target.c > > tests/oss-fuzz/ofp_print_target.c > > See above for list of violations of the rule that > > every C source file must #include . > > Is this comment still valid in the light of my comment on build process? Well, either they have to have #include or we need to blacklist these files, one or the other. The former is probably harmless and possibly helpful. > > The new files need to get mentioned in an automake.mk, at least in > > EXTRA_DIST, to ensure that "make dist" will put them into the tarball: > > > > The following files are in git but not the distribution: > > tests/oss-fuzz/config/flow_extract_fuzzer.options > > tests/oss-fuzz/config/json_parser_fuzzer.options > > tests/oss-fuzz/config/ofp_print_fuzzer.options > > tests/oss-fuzz/config/ovs.dict > > tests/oss-fuzz/flow_extract_target.c > > tests/oss-fuzz/json_parser_target.c > > tests/oss-fuzz/ofp_print_target.c > > There are several automake files to choose from. How do I do this? I'd add a new tests/oss-fuzz/automake.mk and then include that in tests/automake.mk. Thanks! Ben ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] dpif-netlink: Add meter support.
On Fri, Jun 29, 2018 at 11:09:18AM -0700, Justin Pettit wrote: > From: Andy Zhou > > To work with kernel datapath that supports meter. > > Signed-off-by: Andy Zhou > Co-authored-by: Justin Pettit > Signed-off-by: Justin Pettit Thanks for reviving this. The new code here seems to use our older declaration style where all declarations happen at the top of functions, rather than the newer one where they tend to be closer to initialization or first use. This is a quibble and feel free to ignore it if you think I am nit-picking. It's probably worth carefully looking at the log messages. Some of them seem unnecessary, e.g. the first one in dpif_netlink_meter_transact(), because the caller or the callee already logs. Others seem like the wrong log level, e.g. the second one in dpif_netlink_meter_transact(). And I think that most or all should be rate-limited. In dpif_netlink_meter_get_features(), I think that the memset() call is not needed because dpif_meter_get_features() already does it. In dpif_netlink_meter_set(), these checks seem like ones that should happen centrally at a higher level: if (!(config->flags & (OFPMF13_KBPS | OFPMF13_PKTPS))) { return EBADF; /* Rate unit type not set. */ } if ((config->flags & OFPMF13_KBPS) && (config->flags & OFPMF13_PKTPS)) { return EBADF; /* Both rate units may not be set. */ } Maybe this one too? if (config->n_bands == 0) { return EINVAL; /* No bands. */ } Shouldn't dpif_netlink_meter_set() reject bands that are not "drop"? The inner NL_NESTED_FOR_EACH in dpif_netlink_get_stats() could use nl_attr_find(). In the case where dpif_netlink_get_stats() is used to delete a meter, and statistics are wanted, and some bands are present but the statistics aren't present in at least one of those bands, the function reports an error, even though the meter was deleted successfully. That seems like a weird corner case. In dpif_netlink_get_stats(), 'stats' has lots of members but this function seems to only initialize some of them. This also seems true of dpif_meter_get(). In dpif-netlink.c, it looks to me like initialization fails if meters aren't available. I think that breaks backward compatibility with old kernel modules. I doubt we want that. Thanks, Ben. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH] OVN: add unit test for ICMPv6 TTL exceeded
Add unit test for the ICMPv6 TTL exceeded packet sent by OVN logical router when it receives an IPv6 packet whose TTL has expired (ip.ttl == {0, 1}) Signed-off-by: Lorenzo Bianconi --- tests/ovn.at | 26 +++--- 1 file changed, 23 insertions(+), 3 deletions(-) diff --git a/tests/ovn.at b/tests/ovn.at index 48805ddb1..0185317e2 100644 --- a/tests/ovn.at +++ b/tests/ovn.at @@ -10330,6 +10330,25 @@ test_ip_packet() { as hv$hv ovs-appctl netdev-dummy/receive vif$inport $packet } +# test_ip6_packet INPORT HV ETH_SRC ETH_DST IPV6_SRC IPV6_DST IPV6_ROUTER EXP_ICMP_CHKSUM +# +# Causes a packet to be received on INPORT of the hypervisor HV. The packet is an IPv6 +# packet with ETH_SRC, ETH_DST, IPV6_SRC and IPV6_DST as specified. +# IPV6_ROUTER and EXP_ICMP_CHKSUM are the source IP and checksum of the icmpv6 ttl exceeded +# packet sent by OVN logical router +test_ip6_packet() { +local inport=$1 hv=$2 eth_src=$3 eth_dst=$4 ipv6_src=$5 ipv6_dst=$6 ipv6_router=$7 exp_icmp_chksum=$8 +shift 8 + +local ip6_hdr=60151101${ipv6_src}${ipv6_dst} +local packet=${eth_dst}${eth_src}86dd${ip6_hdr}dbb8303900155bac6b646f65206676676e6d66720a + +local reply=${eth_src}${eth_dst}86dd60303afe${ipv6_router}${ipv6_src}0300${exp_icmp_chksum}${ip6_hdr} +echo $reply >> vif$inport.expected + +as hv$hv ovs-appctl netdev-dummy/receive vif$inport $packet +} + ip_to_hex() { printf "%02x%02x%02x%02x" "$@" } @@ -10344,7 +10363,7 @@ for i in 1 2; do ovn_attach n$i br-phys 192.168.$i.1 ovn-nbctl lsp-add sw$i sw$i-p${i}0 -- \ -lsp-set-addresses sw$i-p${i}0 "00:00:00:00:00:0$i 192.168.$i.1" +lsp-set-addresses sw$i-p${i}0 "00:00:00:00:00:0$i 192.168.$i.1 2001:db8:$i::11" ovs-vsctl -- add-port br-int vif$i -- \ set interface vif$i \ @@ -10356,10 +10375,10 @@ done ovn-nbctl lr-add lr0 for i in 1 2; do -ovn-nbctl lrp-add lr0 lrp$i 00:00:00:00:ff:0$i 192.168.$i.254/24 +ovn-nbctl lrp-add lr0 lrp$i 00:00:00:00:ff:0$i 192.168.$i.254/24 2001:db8:$i::1/64 ovn-nbctl -- lsp-add sw$i lrp$i-attachment \ -- set Logical_Switch_Port lrp$i-attachment type=router \ -options:router-port=lrp$i addresses='"00:00:00:00:ff:'0$i'"' +options:router-port=lrp$i addresses='"00:00:00:00:ff:0'$i' 192.168.'$i'.254 2001:db8:'$i'::1"' done OVN_POPULATE_ARP @@ -10367,6 +10386,7 @@ OVN_POPULATE_ARP ovn-nbctl --wait=hv sync test_ip_packet 1 1 0001 ff01 $(ip_to_hex 192 168 1 1) $(ip_to_hex 192 168 2 1) $(ip_to_hex 192 168 1 254) 7dae f4ff +test_ip6_packet 1 1 0001 ff01 20010db800010011 20010db800020011 20010db800010001 d461 OVN_CHECK_PACKETS([hv1/vif1-tx.pcap], [vif1.expected]) OVN_CLEANUP([hv1], [hv2]) -- 2.17.1 ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v11 0/7] OVS-DPDK flow offload with rte_flow
On Thu, Jun 28, 2018 at 02:39:32PM -0300, Flavio Leitner wrote: > Ian, I am afraid that the issue might be local and we most probably can > fix with a follow up patch later, so if others are happy with the patchset, > please go ahead. I can see this only in the mid of next week. The issue was indeed local. I ran few tests with the patchset and found no issues. I got at least +1Mpps in my simple flow and 200 flows test cases. Thanks again, fbl ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH v3] dpif-netdev: Avoid reordering of packets in a batch with same megaflow
OVS reads packets in batches from a given port and packets in the batch are subjected to potentially 3 levels of lookups to identify the datapath megaflow entry (or flow) associated with the packet. Each megaflow entry has a dedicated buffer in which packets that match the flow classification criteria are collected. This buffer helps OVS perform batch processing for all packets associated with a given flow. Each packet in the received batch is first subjected to lookup in the Exact Match Cache (EMC). Each EMC entry will point to a flow. If the EMC lookup is successful, the packet is moved from the rx batch to the per-flow buffer. Packets that did not match any EMC entry are rearranged in the rx batch at the beginning and are now subjected to a lookup in the megaflow cache. Packets that match a megaflow cache entry are *appended* to the per-flow buffer. Packets that do not match any megaflow entry are subjected to slow-path processing through the upcall mechanism. This cannot change the order of packets as by definition upcall processing is only done for packets without matching megaflow entry. The EMC entry match fields encompass all potentially significant header fields, typically more than specified in the associated flow's match criteria. Hence, multiple EMC entries can point to the same flow. Given that per-flow batching happens at each lookup stage, packets belonging to the same megaflow can get re-ordered because some packets match EMC entries while others do not. The following example can illustrate the issue better. Consider following batch of packets (labelled P1 to P8) associated with a single TCP connection and associated with a single flow. Let us assume that packets with just the ACK bit set in TCP flags have been received in a prior batch also and a corresponding EMC entry exists. 1. P1 (TCP Flag: ACK) 2. P2 (TCP Flag: ACK) 3. P3 (TCP Flag: ACK) 4. P4 (TCP Flag: ACK, PSH) 5. P5 (TCP Flag: ACK) 6. P6 (TCP Flag: ACK) 7. P7 (TCP Flag: ACK) 8. P8 (TCP Flag: ACK) The megaflow classification criteria does not include TCP flags while the EMC match criteria does. Thus, all packets other than P4 match the existing EMC entry and are moved to the per-flow packet batch. Subsequently, packet P4 is moved to the same per-flow packet batch as a result of the megaflow lookup. Though the packets have all been correctly classified as being associated with the same flow, the packet order has not been preserved because of the per-flow batching performed during the EMC lookup stage. This packet re-ordering has performance implications for TCP applications. This patch preserves the packet ordering by performing the per-flow batching after both the EMC and megaflow lookups are complete. As an optimization, packets are flow-batched in emc processing till any packet in the batch has an EMC miss. A new flow map is maintained to keep the original order of packet along with flow information. Post fastpath processing, packets from flow map are *appended* to per-flow buffer. Signed-off-by: Vishal Deep Ajmera Co-authored-by: Venkatesan Pradeep Signed-off-by: Venkatesan Pradeep --- lib/dpif-netdev.c | 80 ++- 1 file changed, 67 insertions(+), 13 deletions(-) diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c index 9390fff..23cda57 100644 --- a/lib/dpif-netdev.c +++ b/lib/dpif-netdev.c @@ -207,6 +207,13 @@ struct dpcls_rule { /* 'flow' must be the last field, additional space is allocated here. */ }; +/* data structure to keep packet order till fastpath processing */ +struct dp_packet_flow_map { +struct dp_packet *packet; +struct dp_netdev_flow *flow; +uint16_t tcp_flags; +}; + static void dpcls_init(struct dpcls *); static void dpcls_destroy(struct dpcls *); static void dpcls_sort_subtable_vector(struct dpcls *); @@ -5081,10 +5088,10 @@ struct packet_batch_per_flow { static inline void packet_batch_per_flow_update(struct packet_batch_per_flow *batch, struct dp_packet *packet, - const struct miniflow *mf) + uint16_t tcp_flags) { batch->byte_count += dp_packet_size(packet); -batch->tcp_flags |= miniflow_get_tcp_flags(mf); +batch->tcp_flags |= tcp_flags; batch->array.packets[batch->array.count++] = packet; } @@ -5118,7 +5125,7 @@ packet_batch_per_flow_execute(struct packet_batch_per_flow *batch, static inline void dp_netdev_queue_batches(struct dp_packet *pkt, -struct dp_netdev_flow *flow, const struct miniflow *mf, +struct dp_netdev_flow *flow, uint16_t tcp_flags, struct packet_batch_per_flow *batches, size_t *n_batches) { @@ -5129,7 +5136,7 @@ dp_netdev_queue_batches(struct dp_packet *pkt, packet_batch_per_flow_init(batch, flow); } -packet_batch_per_flow_update(batch, pkt, mf); +
Re: [ovs-dev] OVS DPDK: dpdk_merge pull request for branch-2.9
Bleep bloop. Greetings Ian Stokes, I am a robot and I have tried out your patch. Thanks for your contribution. I encountered some error that I wasn't expecting. See the details below. checkpatch: ERROR: Too many signoffs; are you missing Co-authored-by lines? Lines checked: 92, Warnings: 0, Errors: 1 Please check this out. If you feel there has been an error, please email acon...@bytheb.org Thanks, 0-day Robot ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v2] dpif-netdev: Avoid reordering of packets in a batch with same megaflow
> Thanks for the V2 Vishal, > > as part of the pull request this week we're also introducing the HWOL > patchset. > Both this and the HWOL patchset touch a lot of the same code. > > I've cc'd Shahaf and Finn who have worked on the HWOL to date to help gauge > if this has an impact for HWOL. > Thanks Ian. I will address your comments in V3 patch. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH 2/2] OVN: add IPv6 address unreachable support to OVN router ports
Add priority-70 flows to generate ICMPv6 address unreachable messages in reply to IPv6 packets directed to the router's IP address on IP protocols other than UDP, TCP, and ICMP Signed-off-by: Lorenzo Bianconi --- ovn/northd/ovn-northd.8.xml | 5 +++-- ovn/northd/ovn-northd.c | 14 ++ tests/ovn.at| 1 + 3 files changed, 18 insertions(+), 2 deletions(-) diff --git a/ovn/northd/ovn-northd.8.xml b/ovn/northd/ovn-northd.8.xml index 08e0325a0..fa5e5f9d7 100644 --- a/ovn/northd/ovn-northd.8.xml +++ b/ovn/northd/ovn-northd.8.xml @@ -1339,8 +1339,9 @@ nd_na { - Protocol unreachable. Priority-70 flows generate ICMP protocol - unreachable messages in reply to packets directed to the router's IP + Protocol or address unreachable. Priority-70 flows generate ICMP + protocol or address unreachable messages for IPv4 and IPv6 + respectively in reply to packets directed to the router's IP address on IP protocols other than UDP, TCP, and ICMP, except in the special case of gateways, which accept traffic directed to a router IP for load balancing purposes. diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c index 912e0188d..1d1484687 100644 --- a/ovn/northd/ovn-northd.c +++ b/ovn/northd/ovn-northd.c @@ -5352,6 +5352,20 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap *ports, "next; };"; ovn_lflow_add(lflows, op->od, S_ROUTER_IN_IP_INPUT, 80, ds_cstr(), action); + +ds_clear(); +ds_put_format(, + "ip6 && ip6.dst == %s && !ip.later_frag", + op->lrp_networks.ipv6_addrs[i].addr_s); +action = "icmp6 {" + "eth.dst <-> eth.src; " + "ip6.dst <-> ip6.src; " + "ip.ttl = 255; " + "icmp6.type = 1; " + "icmp6.code = 3; " + "next; };"; +ovn_lflow_add(lflows, op->od, S_ROUTER_IN_IP_INPUT, 70, + ds_cstr(), action); } } diff --git a/tests/ovn.at b/tests/ovn.at index eac928fbb..48805ddb1 100644 --- a/tests/ovn.at +++ b/tests/ovn.at @@ -10495,6 +10495,7 @@ test_ip6_packet 1 1 0001 ff01 20010db800010011 2 OVN_CHECK_PACKETS([hv1/vif1-tx.pcap], [vif1.expected]) test_tcp_syn_packet 2 2 0002 ff02 $(ip_to_hex 192 168 2 1) $(ip_to_hex 192 168 2 254) 8b40 3039 7bae 4486 +test_ip6_packet 2 2 0002 ff02 20010db800020011 20010db800020001 84 0004 01020304 0103 627e OVN_CHECK_PACKETS([hv2/vif2-tx.pcap], [vif2.expected]) OVN_CLEANUP([hv1], [hv2]) -- 2.17.1 ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH 0/2] add IPv6 port unreachable support to OVN logical router
Add ICMPv6 port unreachable messages in reply to IPv6 packets (not TCP) directed to the logical router's IP addresses Lorenzo Bianconi (2): OVN: add IPv6 UDP port unreachable support to OVN logical router OVN: add IPv6 address unreachable support to OVN router ports ovn/northd/ovn-northd.8.xml | 5 +++-- ovn/northd/ovn-northd.c | 30 +- tests/ovn.at| 27 --- 3 files changed, 56 insertions(+), 6 deletions(-) -- 2.17.1 ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] OVS DPDK: dpdk_merge pull request for master
Bleep bloop. Greetings Ian Stokes, I am a robot and I have tried out your patch. Thanks for your contribution. I encountered some error that I wasn't expecting. See the details below. checkpatch: ERROR: Too many signoffs; are you missing Co-authored-by lines? Lines checked: 92, Warnings: 0, Errors: 1 Please check this out. If you feel there has been an error, please email acon...@bytheb.org Thanks, 0-day Robot ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] OVS DPDK: dpdk_merge pull request for branch-2.9
Hi Ben, The following changes since commit 0ed5bf7577bec8ddbe551648f038ea1dec1ced96: ofp-actions: Fix undefined behavior shifting 'int' 16 places left. (2018-07-05 15:10:44 -0700) are available in the git repository at: https://github.com/istokes/ovs dpdk_merge_2_9 for you to fetch changes up to ac2fb7d4fae5ad6d37dde4d1d3f0907e6f950329: dpdk: Use DPDK 17.11.3 release. (2018-07-06 10:52:54 +0100) Ian Stokes (1): dpdk: Use DPDK 17.11.3 release. .travis/linux-build.sh | 2 +- Documentation/faq/releases.rst | 4 ++-- Documentation/intro/install/dpdk.rst | 8 Documentation/topics/dpdk/vhost-user.rst | 6 +++--- 4 files changed, 10 insertions(+), 10 deletions(-) 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 23626bcf3bd987f7a5e03b93bec8450b10421c31: OVN: add ICMPv6 time exceeded support to OVN logical router (2018-07-05 15:46:09 -0700) are available in the git repository at: https://github.com/istokes/ovs dpdk_merge for you to fetch changes up to 43307ad0e2543c9c8443f3ab1150ab03f4eb551c: dpdk: Support both shared and per port mempools. (2018-07-06 12:46:26 +0100) Finn Christensen (1): netdev-dpdk: implement flow offload with rte flow Ian Stokes (2): dpdk: Use DPDK 17.11.3 release. dpdk: Support both shared and per port mempools. Yuanhan Liu (6): dpif-netdev: associate flow with a mark id flow: Introduce IP packet sanity checks 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 .travis/linux-build.sh |2 +- Documentation/automake.mk|1 + Documentation/faq/releases.rst |4 +- Documentation/howto/dpdk.rst | 22 Documentation/intro/install/dpdk.rst | 14 ++- Documentation/topics/dpdk/index.rst |1 + Documentation/topics/dpdk/memory.rst | 216 Documentation/topics/dpdk/vhost-user.rst |6 +- NEWS |3 +- lib/dp-packet.h | 13 +++ lib/dpdk-stub.c |6 ++ lib/dpdk.c | 12 +++ lib/dpdk.h |1 + lib/dpif-netdev.c| 499 +-- lib/flow.c | 168 +-- lib/flow.h |1 + lib/netdev-dpdk.c| 1029 +-- lib/netdev.h |6 ++ vswitchd/vswitch.xml | 17 19 files changed, 1873 insertions(+), 148 deletions(-) create mode 100644 Documentation/topics/dpdk/memory.rst Thanks Ian ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v2] dpdk: Support both shared and per port mempools.
> On 27/06/2018 14:58, Ian Stokes wrote: > > This commit re-introduces the concept of shared mempools as the > > default memory model for DPDK devices. Per port mempools are still > > available but must be enabled explicitly by a user. > > > > OVS previously used a shared mempool model for ports with the same MTU > > and socket configuration. This was replaced by a per port mempool > > model to address issues flagged by users such as: > > > > https://mail.openvswitch.org/pipermail/ovs-discuss/2016-September/0425 > > 60.html > > > > However the per port model potentially requires an increase in memory > > resource requirements to support the same number of ports and > > configuration as the shared port model. > > > > This is considered a blocking factor for current deployments of OVS > > when upgrading to future OVS releases as a user may have to > > redimension memory for the same deployment configuration. This may not > > be possible for users. > > > > This commit resolves the issue by re-introducing shared mempools as > > the default memory behaviour in OVS DPDK but also refactors the memory > > configuration code to allow for per port mempools. > > > > This patch adds a new global config option, per-port-memory, that > > controls the enablement of per port mempools for DPDK devices. > > > > ovs-vsctl set Open_vSwitch . other_config:per-port-memory=true > > > > This value defaults to false; to enable per port memory support, this > > field should be set to true when setting other global parameters on > > init (such as "dpdk-socket-mem", for example). Changing the value at > > runtime is not supported, and requires restarting the vswitch daemon. > > > > The mempool sweep functionality is also replaced with the sweep > > functionality from OVS 2.9 found in commits > > > > c77f692 (netdev-dpdk: Free mempool only when no in-use mbufs.) > > a7fb0a4 (netdev-dpdk: Add mempool reuse/free debug.) > > > > A new document to discuss the specifics of the memory models and > > example memory requirement calculations is also added. > > > > Signed-off-by: Ian Stokes > > Hi Ian, > > Thanks for your work on this. > > I've tested with the same set up I had for RFC v1 (re-configuring MTUs of > existing ports, adding new ports, deleting existing ones, etc) and things > still work the same. > > I don't have additional concerns either, as my comments from RFC v1 have > been addressed / clarified and the code looks more straightforward now. > > Acked-by: Tiago Lam > Tested-by: Tiago Lam Thanks Tiago, I'll add your acked/tested tags to the commit. Thanks Ian ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v3 00/14] Support multi-segment mbufs
On 06/07/2018 09:51, Eelco Chaudron wrote: > > > On 5 Jul 2018, at 18:29, Tiago Lam wrote: > [snip] >> >> >> Performance notes (based on v8) >> = >> In order to test for regressions in performance, tests were run on top >> of master 88125d6 and v8 of this patchset, both with the multi-segment >> mbufs option enabled and disabled. >> >> VSperf was used to run the phy2phy_cont and pvp_cont tests with >> varying >> packet sizes of 64B, 1500B and 7000B, on a 10Gbps interface. >> >> Test | Size | Master | Multi-seg disabled | Multi-seg enabled >> - >> p2p | 64 | ~22.7 | ~22.65| ~18.3 >> p2p | 1500 | ~1.6 |~1.6|~1.6 >> p2p | 7000 | ~0.36 | ~0.36| ~0.36 >> pvp | 64 | ~6.7 |~6.7|~6.3 >> pvp | 1500 | ~1.6 |~1.6|~1.6 >> pvp | 7000 | ~0.36 | ~0.36| ~0.36 >> >> Packet size is in bytes, while all packet rates are reported in mpps >> (aggregated). >> >> No noticeable regression has been observed (certainly everything is >> within the ± 5% margin of existing performance), aside from the 64B >> packet size case when multi-segment mbuf is enabled. This is >> expected, however, because of how Tx vectoriszed functions are >> incompatible with multi-segment mbufs on some PMDs. The PMD under >> use during these tests was the i40e (on a Intel X710 NIC), which >> indeed doesn't support vectorized Tx functions with multi-segment >> mbufs. >> > Thanks for all the work Tiago! It all looks good to me, so hereby I > would like to ack the series. > > Cheers, > > Eelco > > > Acked-by: Eelco Chaudron > > > And thanks again for putting the time for reviewing and confirming the results, Eelco. A side note on the series: It will need rebasing once Ian's work on "dpdk: Support both shared and per port mempools" goes in. Tiago. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v2] dpdk: Support both shared and per port mempools.
On 27/06/2018 14:58, Ian Stokes wrote: > This commit re-introduces the concept of shared mempools as the default > memory model for DPDK devices. Per port mempools are still available but > must be enabled explicitly by a user. > > OVS previously used a shared mempool model for ports with the same MTU > and socket configuration. This was replaced by a per port mempool model > to address issues flagged by users such as: > > https://mail.openvswitch.org/pipermail/ovs-discuss/2016-September/042560.html > > However the per port model potentially requires an increase in memory > resource requirements to support the same number of ports and configuration > as the shared port model. > > This is considered a blocking factor for current deployments of OVS > when upgrading to future OVS releases as a user may have to redimension > memory for the same deployment configuration. This may not be possible for > users. > > This commit resolves the issue by re-introducing shared mempools as > the default memory behaviour in OVS DPDK but also refactors the memory > configuration code to allow for per port mempools. > > This patch adds a new global config option, per-port-memory, that > controls the enablement of per port mempools for DPDK devices. > > ovs-vsctl set Open_vSwitch . other_config:per-port-memory=true > > This value defaults to false; to enable per port memory support, > this field should be set to true when setting other global parameters > on init (such as "dpdk-socket-mem", for example). Changing the value at > runtime is not supported, and requires restarting the vswitch > daemon. > > The mempool sweep functionality is also replaced with the > sweep functionality from OVS 2.9 found in commits > > c77f692 (netdev-dpdk: Free mempool only when no in-use mbufs.) > a7fb0a4 (netdev-dpdk: Add mempool reuse/free debug.) > > A new document to discuss the specifics of the memory models and example > memory requirement calculations is also added. > > Signed-off-by: Ian Stokes Hi Ian, Thanks for your work on this. I've tested with the same set up I had for RFC v1 (re-configuring MTUs of existing ports, adding new ports, deleting existing ones, etc) and things still work the same. I don't have additional concerns either, as my comments from RFC v1 have been addressed / clarified and the code looks more straightforward now. Acked-by: Tiago Lam Tested-by: Tiago Lam ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [ovs-dev, 2 of 4] ovs-dev: Gather packets-per-second rate of flows
Bleep bloop. Greetings Sriharsha Basavapatna via dev, I am a robot and I have tried out your patch. Thanks for your contribution. I encountered some error that I wasn't expecting. See the details below. build: /bin/sh ./libtool --tag=CC --mode=compile gcc -std=gnu99 -DHAVE_CONFIG_H -I. -I ./include -I ./include -I ./lib -I ./lib-Wstrict-prototypes -Wall -Wextra -Wno-sign-compare -Wpointer-arith -Wformat -Wformat-security -Wswitch-enum -Wunused-parameter -Wbad-function-cast -Wcast-align -Wstrict-prototypes -Wold-style-definition -Wmissing-prototypes -Wmissing-field-initializers -fno-strict-aliasing -Wshadow-Werror -MT lib/signals.lo -MD -MP -MF $depbase.Tpo -c -o lib/signals.lo lib/signals.c &&\ mv -f $depbase.Tpo $depbase.Plo libtool: compile: gcc -std=gnu99 -DHAVE_CONFIG_H -I. -I ./include -I ./include -I ./lib -I ./lib -Wstrict-prototypes -Wall -Wextra -Wno-sign-compare -Wpointer-arith -Wformat -Wformat-security -Wswitch-enum -Wunused-parameter -Wbad-function-cast -Wcast-align -Wstrict-prototypes -Wold-style-definition -Wmissing-prototypes -Wmissing-field-initializers -fno-strict-aliasing -Wshadow -Werror -MT lib/signals.lo -MD -MP -MF lib/.deps/signals.Tpo -c lib/signals.c -o lib/signals.o depbase=`echo lib/socket-util-unix.lo | sed 's|[^/]*$|.deps/&|;s|\.lo$||'`;\ /bin/sh ./libtool --tag=CC --mode=compile gcc -std=gnu99 -DHAVE_CONFIG_H -I. -I ./include -I ./include -I ./lib -I ./lib-Wstrict-prototypes -Wall -Wextra -Wno-sign-compare -Wpointer-arith -Wformat -Wformat-security -Wswitch-enum -Wunused-parameter -Wbad-function-cast -Wcast-align -Wstrict-prototypes -Wold-style-definition -Wmissing-prototypes -Wmissing-field-initializers -fno-strict-aliasing -Wshadow-Werror -MT lib/socket-util-unix.lo -MD -MP -MF $depbase.Tpo -c -o lib/socket-util-unix.lo lib/socket-util-unix.c &&\ mv -f $depbase.Tpo $depbase.Plo libtool: compile: gcc -std=gnu99 -DHAVE_CONFIG_H -I. -I ./include -I ./include -I ./lib -I ./lib -Wstrict-prototypes -Wall -Wextra -Wno-sign-compare -Wpointer-arith -Wformat -Wformat-security -Wswitch-enum -Wunused-parameter -Wbad-function-cast -Wcast-align -Wstrict-prototypes -Wold-style-definition -Wmissing-prototypes -Wmissing-field-initializers -fno-strict-aliasing -Wshadow -Werror -MT lib/socket-util-unix.lo -MD -MP -MF lib/.deps/socket-util-unix.Tpo -c lib/socket-util-unix.c -o lib/socket-util-unix.o depbase=`echo lib/stream-unix.lo | sed 's|[^/]*$|.deps/&|;s|\.lo$||'`;\ /bin/sh ./libtool --tag=CC --mode=compile gcc -std=gnu99 -DHAVE_CONFIG_H -I. -I ./include -I ./include -I ./lib -I ./lib-Wstrict-prototypes -Wall -Wextra -Wno-sign-compare -Wpointer-arith -Wformat -Wformat-security -Wswitch-enum -Wunused-parameter -Wbad-function-cast -Wcast-align -Wstrict-prototypes -Wold-style-definition -Wmissing-prototypes -Wmissing-field-initializers -fno-strict-aliasing -Wshadow-Werror -MT lib/stream-unix.lo -MD -MP -MF $depbase.Tpo -c -o lib/stream-unix.lo lib/stream-unix.c &&\ mv -f $depbase.Tpo $depbase.Plo libtool: compile: gcc -std=gnu99 -DHAVE_CONFIG_H -I. -I ./include -I ./include -I ./lib -I ./lib -Wstrict-prototypes -Wall -Wextra -Wno-sign-compare -Wpointer-arith -Wformat -Wformat-security -Wswitch-enum -Wunused-parameter -Wbad-function-cast -Wcast-align -Wstrict-prototypes -Wold-style-definition -Wmissing-prototypes -Wmissing-field-initializers -fno-strict-aliasing -Wshadow -Werror -MT lib/stream-unix.lo -MD -MP -MF lib/.deps/stream-unix.Tpo -c lib/stream-unix.c -o lib/stream-unix.o depbase=`echo lib/dpif-netlink.lo | sed 's|[^/]*$|.deps/&|;s|\.lo$||'`;\ /bin/sh ./libtool --tag=CC --mode=compile gcc -std=gnu99 -DHAVE_CONFIG_H -I. -I ./include -I ./include -I ./lib -I ./lib-Wstrict-prototypes -Wall -Wextra -Wno-sign-compare -Wpointer-arith -Wformat -Wformat-security -Wswitch-enum -Wunused-parameter -Wbad-function-cast -Wcast-align -Wstrict-prototypes -Wold-style-definition -Wmissing-prototypes -Wmissing-field-initializers -fno-strict-aliasing -Wshadow-Werror -MT lib/dpif-netlink.lo -MD -MP -MF $depbase.Tpo -c -o lib/dpif-netlink.lo lib/dpif-netlink.c &&\ mv -f $depbase.Tpo $depbase.Plo libtool: compile: gcc -std=gnu99 -DHAVE_CONFIG_H -I. -I ./include -I ./include -I ./lib -I ./lib -Wstrict-prototypes -Wall -Wextra -Wno-sign-compare -Wpointer-arith -Wformat -Wformat-security -Wswitch-enum -Wunused-parameter -Wbad-function-cast -Wcast-align -Wstrict-prototypes -Wold-style-definition -Wmissing-prototypes -Wmissing-field-initializers -fno-strict-aliasing -Wshadow -Werror -MT lib/dpif-netlink.lo -MD -MP -MF lib/.deps/dpif-netlink.Tpo -c lib/dpif-netlink.c -o lib/dpif-netlink.o lib/dpif-netlink.c:1007:1: error: return type defaults to 'int' [-Werror] dpif_netlink_port_add(struct dpif *dpif_, struct netdev *netdev, ^ lib/dpif-netlink.c:1007:1: error: no previous prototype for 'dpif_netlink_port_add' [-Werror=missing-prototypes] cc1: all warnings being treated as errors make[2]: ***
Re: [ovs-dev] [ovs-dev, 1 of 4] ovs-dev: Detect Out-Of-Resource condition on a netdev
Bleep bloop. Greetings Sriharsha Basavapatna via dev, I am a robot and I have tried out your patch. Thanks for your contribution. I encountered some error that I wasn't expecting. See the details below. build: /bin/sh ./libtool --tag=CC --mode=compile gcc -std=gnu99 -DHAVE_CONFIG_H -I. -I ./include -I ./include -I ./lib -I ./lib-Wstrict-prototypes -Wall -Wextra -Wno-sign-compare -Wpointer-arith -Wformat -Wformat-security -Wswitch-enum -Wunused-parameter -Wbad-function-cast -Wcast-align -Wstrict-prototypes -Wold-style-definition -Wmissing-prototypes -Wmissing-field-initializers -fno-strict-aliasing -Wshadow-Werror -MT lib/signals.lo -MD -MP -MF $depbase.Tpo -c -o lib/signals.lo lib/signals.c &&\ mv -f $depbase.Tpo $depbase.Plo libtool: compile: gcc -std=gnu99 -DHAVE_CONFIG_H -I. -I ./include -I ./include -I ./lib -I ./lib -Wstrict-prototypes -Wall -Wextra -Wno-sign-compare -Wpointer-arith -Wformat -Wformat-security -Wswitch-enum -Wunused-parameter -Wbad-function-cast -Wcast-align -Wstrict-prototypes -Wold-style-definition -Wmissing-prototypes -Wmissing-field-initializers -fno-strict-aliasing -Wshadow -Werror -MT lib/signals.lo -MD -MP -MF lib/.deps/signals.Tpo -c lib/signals.c -o lib/signals.o depbase=`echo lib/socket-util-unix.lo | sed 's|[^/]*$|.deps/&|;s|\.lo$||'`;\ /bin/sh ./libtool --tag=CC --mode=compile gcc -std=gnu99 -DHAVE_CONFIG_H -I. -I ./include -I ./include -I ./lib -I ./lib-Wstrict-prototypes -Wall -Wextra -Wno-sign-compare -Wpointer-arith -Wformat -Wformat-security -Wswitch-enum -Wunused-parameter -Wbad-function-cast -Wcast-align -Wstrict-prototypes -Wold-style-definition -Wmissing-prototypes -Wmissing-field-initializers -fno-strict-aliasing -Wshadow-Werror -MT lib/socket-util-unix.lo -MD -MP -MF $depbase.Tpo -c -o lib/socket-util-unix.lo lib/socket-util-unix.c &&\ mv -f $depbase.Tpo $depbase.Plo libtool: compile: gcc -std=gnu99 -DHAVE_CONFIG_H -I. -I ./include -I ./include -I ./lib -I ./lib -Wstrict-prototypes -Wall -Wextra -Wno-sign-compare -Wpointer-arith -Wformat -Wformat-security -Wswitch-enum -Wunused-parameter -Wbad-function-cast -Wcast-align -Wstrict-prototypes -Wold-style-definition -Wmissing-prototypes -Wmissing-field-initializers -fno-strict-aliasing -Wshadow -Werror -MT lib/socket-util-unix.lo -MD -MP -MF lib/.deps/socket-util-unix.Tpo -c lib/socket-util-unix.c -o lib/socket-util-unix.o depbase=`echo lib/stream-unix.lo | sed 's|[^/]*$|.deps/&|;s|\.lo$||'`;\ /bin/sh ./libtool --tag=CC --mode=compile gcc -std=gnu99 -DHAVE_CONFIG_H -I. -I ./include -I ./include -I ./lib -I ./lib-Wstrict-prototypes -Wall -Wextra -Wno-sign-compare -Wpointer-arith -Wformat -Wformat-security -Wswitch-enum -Wunused-parameter -Wbad-function-cast -Wcast-align -Wstrict-prototypes -Wold-style-definition -Wmissing-prototypes -Wmissing-field-initializers -fno-strict-aliasing -Wshadow-Werror -MT lib/stream-unix.lo -MD -MP -MF $depbase.Tpo -c -o lib/stream-unix.lo lib/stream-unix.c &&\ mv -f $depbase.Tpo $depbase.Plo libtool: compile: gcc -std=gnu99 -DHAVE_CONFIG_H -I. -I ./include -I ./include -I ./lib -I ./lib -Wstrict-prototypes -Wall -Wextra -Wno-sign-compare -Wpointer-arith -Wformat -Wformat-security -Wswitch-enum -Wunused-parameter -Wbad-function-cast -Wcast-align -Wstrict-prototypes -Wold-style-definition -Wmissing-prototypes -Wmissing-field-initializers -fno-strict-aliasing -Wshadow -Werror -MT lib/stream-unix.lo -MD -MP -MF lib/.deps/stream-unix.Tpo -c lib/stream-unix.c -o lib/stream-unix.o depbase=`echo lib/dpif-netlink.lo | sed 's|[^/]*$|.deps/&|;s|\.lo$||'`;\ /bin/sh ./libtool --tag=CC --mode=compile gcc -std=gnu99 -DHAVE_CONFIG_H -I. -I ./include -I ./include -I ./lib -I ./lib-Wstrict-prototypes -Wall -Wextra -Wno-sign-compare -Wpointer-arith -Wformat -Wformat-security -Wswitch-enum -Wunused-parameter -Wbad-function-cast -Wcast-align -Wstrict-prototypes -Wold-style-definition -Wmissing-prototypes -Wmissing-field-initializers -fno-strict-aliasing -Wshadow-Werror -MT lib/dpif-netlink.lo -MD -MP -MF $depbase.Tpo -c -o lib/dpif-netlink.lo lib/dpif-netlink.c &&\ mv -f $depbase.Tpo $depbase.Plo libtool: compile: gcc -std=gnu99 -DHAVE_CONFIG_H -I. -I ./include -I ./include -I ./lib -I ./lib -Wstrict-prototypes -Wall -Wextra -Wno-sign-compare -Wpointer-arith -Wformat -Wformat-security -Wswitch-enum -Wunused-parameter -Wbad-function-cast -Wcast-align -Wstrict-prototypes -Wold-style-definition -Wmissing-prototypes -Wmissing-field-initializers -fno-strict-aliasing -Wshadow -Werror -MT lib/dpif-netlink.lo -MD -MP -MF lib/.deps/dpif-netlink.Tpo -c lib/dpif-netlink.c -o lib/dpif-netlink.o lib/dpif-netlink.c:1007:1: error: return type defaults to 'int' [-Werror] dpif_netlink_port_add(struct dpif *dpif_, struct netdev *netdev, ^ lib/dpif-netlink.c:1007:1: error: no previous prototype for 'dpif_netlink_port_add' [-Werror=missing-prototypes] cc1: all warnings being treated as errors make[2]: ***
[ovs-dev] [PATCH 4/4] ovs-dev: Add an openvswitch option to enable dynamic rebalancing of flows
This is the fourth patch in the patch-set to support dynamic rebalancing of offloaded flows. A new OVS configuration parameter "offload-rebalance", is added to ovsdb. The default value of this is "disable". To enable this feature, set the value of this parameter to "pps-rate", which provides packets-per-second rate based policy to dynamically offload and un-offload flows. Note: This option can be enabled only when 'hw-offload' policy is enabled. It also requires 'tc-policy' to be set to 'skip_sw'; otherwise, flow offload errors (specifically ENOSPC error this feature depends on) reported by an offloaded device are supressed by TC-Flower kernel module. Signed-off-by: Sriharsha Basavapatna Co-authored-by: Venkat Duvvuru Signed-off-by: Venkat Duvvuru Reviewed-by: Sathya Perla --- lib/dpif-netlink.c| 3 ++- lib/netdev.c | 43 +++ lib/netdev.h | 2 ++ ofproto/ofproto-dpif-upcall.c | 7 -- vswitchd/vswitch.xml | 22 ++ 5 files changed, 74 insertions(+), 3 deletions(-) diff --git a/lib/dpif-netlink.c b/lib/dpif-netlink.c index 3c4b9b69f..dfe21971d 100644 --- a/lib/dpif-netlink.c +++ b/lib/dpif-netlink.c @@ -2177,7 +2177,8 @@ parse_flow_put(struct dpif_netlink *dpif, struct dpif_flow_put *put) VLOG_DBG("added flow"); } else if (err != EEXIST) { -if (outdev && dev && (err == ENOSPC)) { +if (netdev_is_offload_rebalance_policy_enabled() && outdev && +dev && (err == ENOSPC)) { tunnel_netdev = flow_get_tunnel_netdev(); if (tunnel_netdev) { oor_netdev = tunnel_netdev; diff --git a/lib/netdev.c b/lib/netdev.c index b17f0563f..9992b846c 100644 --- a/lib/netdev.c +++ b/lib/netdev.c @@ -2474,6 +2474,43 @@ netdev_free_custom_stats_counters(struct netdev_custom_stats *custom_stats) #ifdef __linux__ +enum offload_rebalance_policy { +OFFLOAD_REBALANCE_POLICY_DISABLE, +OFFLOAD_REBALANCE_POLICY_PPS_RATE +}; + +static enum +offload_rebalance_policy netdev_offload_rebalance_policy = +OFFLOAD_REBALANCE_POLICY_DISABLE; + +static void +netdev_offload_rebalance_set_policy(const char *policy) +{ +if (!policy) { +return; +} + +if (!strcmp(policy, "pps-rate")) { +netdev_offload_rebalance_policy = OFFLOAD_REBALANCE_POLICY_PPS_RATE; +} else if (!strcmp(policy, "disable")) { +netdev_offload_rebalance_policy = OFFLOAD_REBALANCE_POLICY_DISABLE; +} else { +VLOG_WARN("netdev: Invalid policy '%s'", policy); +return; +} + +VLOG_INFO("netdev: Using policy '%s'", policy); +} + +bool +netdev_is_offload_rebalance_policy_enabled(void) +{ +if (netdev_offload_rebalance_policy == OFFLOAD_REBALANCE_POLICY_PPS_RATE) { +return true; +} +return false; +} + static void netdev_ports_flow_init(void) { @@ -2494,12 +2531,18 @@ netdev_set_flow_api_enabled(const struct smap *ovs_other_config) if (ovsthread_once_start()) { netdev_flow_api_enabled = true; +const char *offl_rebal_policy = NULL; VLOG_INFO("netdev: Flow API Enabled"); tc_set_policy(smap_get_def(ovs_other_config, "tc-policy", TC_POLICY_DEFAULT)); +offl_rebal_policy = smap_get_def(ovs_other_config, + "offload-rebalance", + OFFLOAD_REBALANCE_POLICY_DEFAULT); +netdev_offload_rebalance_set_policy(offl_rebal_policy); + netdev_ports_flow_init(); ovsthread_once_done(); diff --git a/lib/netdev.h b/lib/netdev.h index c941f1e1e..0ac2774c9 100644 --- a/lib/netdev.h +++ b/lib/netdev.h @@ -223,6 +223,8 @@ int netdev_init_flow_api(struct netdev *); uint32_t netdev_get_block_id(struct netdev *); bool netdev_is_flow_api_enabled(void); void netdev_set_flow_api_enabled(const struct smap *ovs_other_config); +bool netdev_is_offload_rebalance_policy_enabled(void); +#define OFFLOAD_REBALANCE_POLICY_DEFAULT "disable" struct dpif_port; int netdev_ports_insert(struct netdev *, const struct dpif_class *, diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c index 28596baea..49922089e 100644 --- a/ofproto/ofproto-dpif-upcall.c +++ b/ofproto/ofproto-dpif-upcall.c @@ -967,7 +967,9 @@ udpif_revalidator(void *arg) dpif_flow_dump_destroy(udpif->dump); seq_change(udpif->dump_seq); -udpif_run_flow_rebalance(udpif); +if (netdev_is_offload_rebalance_policy_enabled()) { +udpif_run_flow_rebalance(udpif); +} duration = MAX(time_msec() - start_time, 1); udpif->dump_duration = duration; @@ -2747,7 +2749,8 @@ revalidate(struct revalidator *revalidator) } ukey->dump_seq = dump_seq; -if (result !=
[ovs-dev] [PATCH 3/4] ovs-dev: Rebalance offloaded flows based on the pps rate
This is the third patch in the patch-set to support dynamic rebalancing of offloaded flows. The dynamic rebalancing functionality is implemented in this patch. The ukeys that are not scheduled for deletion are obtained and passed as input to the rebalancing routine. The rebalancing is done in the context of revalidation leader thread, after all other revalidator threads are done with gathering rebalancing data for flows. For each netdev that is in OOR state, a list of flows - both offloaded and non-offloaded (pending) - is obtained using the ukeys. For each netdev that is in OOR state, the flows are grouped and sorted into offloaded and pending flows. The offloaded flows are sorted in descending order of pps-rate, while pending flows are sorted in ascending order of pps-rate. The rebalancing is done in two phases. In the first phase, we try to offload all pending flows and if that succeeds, the OOR state on the device is cleared. If some (or none) of the pending flows could not be offloaded, then we start replacing an offloaded flow that has a lower pps-rate than a pending flow, until there are no more pending flows with a higher rate than an offloaded flow. The flows that are replaced from the device are added into kernel datapath. Signed-off-by: Sriharsha Basavapatna Co-authored-by: Venkat Duvvuru Signed-off-by: Venkat Duvvuru Reviewed-by: Sathya Perla --- lib/dpif-netdev.c | 3 +- lib/dpif-netlink.c| 15 +- lib/dpif-provider.h | 7 +- lib/dpif.c| 20 +- lib/dpif.h| 20 +- lib/netdev-provider.h | 3 +- ofproto/ofproto-dpif-upcall.c | 435 +- 7 files changed, 482 insertions(+), 21 deletions(-) diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c index 9390fff68..ad5aac62b 100644 --- a/lib/dpif-netdev.c +++ b/lib/dpif-netdev.c @@ -3052,7 +3052,8 @@ dpif_netdev_execute(struct dpif *dpif, struct dpif_execute *execute) } static void -dpif_netdev_operate(struct dpif *dpif, struct dpif_op **ops, size_t n_ops) +dpif_netdev_operate(struct dpif *dpif, struct dpif_op **ops, size_t n_ops, +enum dpif_op_skip_type skip_flag OVS_UNUSED) { size_t i; diff --git a/lib/dpif-netlink.c b/lib/dpif-netlink.c index e20096116..3c4b9b69f 100644 --- a/lib/dpif-netlink.c +++ b/lib/dpif-netlink.c @@ -1004,6 +1004,7 @@ dpif_netlink_rtnl_port_create_and_add(struct dpif_netlink *dpif, return error; } +static int dpif_netlink_port_add(struct dpif *dpif_, struct netdev *netdev, odp_port_t *port_nop) { @@ -2275,7 +2276,8 @@ dpif_netlink_operate_chunks(struct dpif_netlink *dpif, struct dpif_op **ops, } static void -dpif_netlink_operate(struct dpif *dpif_, struct dpif_op **ops, size_t n_ops) +dpif_netlink_operate(struct dpif *dpif_, struct dpif_op **ops, size_t n_ops, + enum dpif_op_skip_type skip_flag) { struct dpif_netlink *dpif = dpif_netlink_cast(dpif_); struct dpif_op *new_ops[OPERATE_MAX_OPS]; @@ -2283,7 +2285,7 @@ dpif_netlink_operate(struct dpif *dpif_, struct dpif_op **ops, size_t n_ops) int i = 0; int err = 0; -if (netdev_is_flow_api_enabled()) { +if (skip_flag != DPIF_OP_SKIP_OFFLOAD && netdev_is_flow_api_enabled()) { while (n_ops > 0) { count = 0; @@ -2292,6 +2294,10 @@ dpif_netlink_operate(struct dpif *dpif_, struct dpif_op **ops, size_t n_ops) err = try_send_to_netdev(dpif, op); if (err && err != EEXIST) { +if (skip_flag == DPIF_OP_SKIP_DP) { +op->error = err; +return; +} new_ops[count++] = op; } else { op->error = err; @@ -2302,8 +2308,11 @@ dpif_netlink_operate(struct dpif *dpif_, struct dpif_op **ops, size_t n_ops) dpif_netlink_operate_chunks(dpif, new_ops, count); } -} else { +} else if (skip_flag != DPIF_OP_SKIP_DP) { dpif_netlink_operate_chunks(dpif, ops, n_ops); +} else { +VLOG_ERR("%s: Invalid skip_flag: %d while flow api is disabled\n", + __func__, skip_flag); } } diff --git a/lib/dpif-provider.h b/lib/dpif-provider.h index cc6571b28..2bbfcf7f6 100644 --- a/lib/dpif-provider.h +++ b/lib/dpif-provider.h @@ -296,12 +296,13 @@ struct dpif_class { int (*flow_dump_next)(struct dpif_flow_dump_thread *thread, struct dpif_flow *flows, int max_flows); - /* Executes each of the 'n_ops' operations in 'ops' on 'dpif', in the order * in which they are specified, placing each operation's results in the * "output" members documented in comments and the 'error' member of each - * dpif_op. */ -void (*operate)(struct dpif *dpif, struct dpif_op **ops, size_t n_ops); + * dpif_op. The skip_flag argument tells the provider if 'ops' should be +
[ovs-dev] [PATCH 1/4] ovs-dev: Detect Out-Of-Resource condition on a netdev
This is the first patch in the patch-set to support dynamic rebalancing of offloaded flows. The patch detects OOR condition on a netdev port when ENOSPC error is returned by TC-Flower while adding a flow rule. A new structure is added to the netdev called "netdev_hw_info", to store OOR related information required to perform dynamic offload-rebalancing. Signed-off-by: Sriharsha Basavapatna Co-authored-by: Venkat Duvvuru Signed-off-by: Venkat Duvvuru Reviewed-by: Sathya Perla --- lib/dpif-netlink.c| 23 ++- lib/flow.c| 27 +++ lib/flow.h| 1 + lib/netdev-provider.h | 7 +++ lib/netdev.c | 2 ++ 5 files changed, 55 insertions(+), 5 deletions(-) diff --git a/lib/dpif-netlink.c b/lib/dpif-netlink.c index aa9bbd946..e20096116 100644 --- a/lib/dpif-netlink.c +++ b/lib/dpif-netlink.c @@ -1004,7 +1004,6 @@ dpif_netlink_rtnl_port_create_and_add(struct dpif_netlink *dpif, return error; } -static int dpif_netlink_port_add(struct dpif *dpif_, struct netdev *netdev, odp_port_t *port_nop) { @@ -2097,11 +2096,16 @@ parse_flow_put(struct dpif_netlink *dpif, struct dpif_flow_put *put) { static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 20); const struct dpif_class *dpif_class = dpif->dpif.dpif_class; +struct netdev_hw_info *hw_info; struct match match; odp_port_t in_port; +odp_port_t out_port; const struct nlattr *nla; size_t left; struct netdev *dev; +struct netdev *outdev = NULL; +struct netdev *tunnel_netdev = NULL; +struct netdev *oor_netdev = NULL; struct offload_info info; ovs_be16 dst_port = 0; int err; @@ -2131,8 +2135,6 @@ parse_flow_put(struct dpif_netlink *dpif, struct dpif_flow_put *put) NL_ATTR_FOR_EACH(nla, left, put->actions, put->actions_len) { if (nl_attr_type(nla) == OVS_ACTION_ATTR_OUTPUT) { const struct netdev_tunnel_config *tnl_cfg; -struct netdev *outdev; -odp_port_t out_port; out_port = nl_attr_get_odp_port(nla); outdev = netdev_ports_get(out_port, dpif_class); @@ -2144,7 +2146,6 @@ parse_flow_put(struct dpif_netlink *dpif, struct dpif_flow_put *put) if (tnl_cfg && tnl_cfg->dst_port != 0) { dst_port = tnl_cfg->dst_port; } -netdev_close(outdev); } } @@ -2175,7 +2176,18 @@ parse_flow_put(struct dpif_netlink *dpif, struct dpif_flow_put *put) VLOG_DBG("added flow"); } else if (err != EEXIST) { -VLOG_ERR_RL(, "failed to offload flow: %s", ovs_strerror(err)); +if (outdev && dev && (err == ENOSPC)) { +tunnel_netdev = flow_get_tunnel_netdev(); +if (tunnel_netdev) { +oor_netdev = tunnel_netdev; +} else { +oor_netdev = dev; +} +hw_info = _netdev->hw_info; +hw_info->oor = true; +} +VLOG_ERR_RL(, "failed to offload flow: %s: %s", ovs_strerror(err), +(oor_netdev ? oor_netdev->name : dev->name)); } out: @@ -2196,6 +2208,7 @@ out: } } +netdev_close(outdev); netdev_close(dev); return err; diff --git a/lib/flow.c b/lib/flow.c index 75ca45672..912afc99d 100644 --- a/lib/flow.c +++ b/lib/flow.c @@ -19,6 +19,7 @@ #include #include #include +#include #include #include #include @@ -41,6 +42,8 @@ #include "unaligned.h" #include "util.h" #include "openvswitch/nsh.h" +#include "ovs-router.h" +#include "lib/netdev-provider.h" COVERAGE_DEFINE(flow_extract); COVERAGE_DEFINE(miniflow_malloc); @@ -3299,3 +3302,27 @@ flow_limit_vlans(int vlan_limit) flow_vlan_limit = MIN(vlan_limit, FLOW_MAX_VLAN_HEADERS); } } + +struct netdev * +flow_get_tunnel_netdev(struct flow_tnl *tunnel) +{ +struct netdev *tunnel_netdev; +char iface[IFNAMSIZ]; +struct in6_addr ip6; +struct in6_addr gw; + +if (tunnel->ip_src) { +in6_addr_set_mapped_ipv4(, tunnel->ip_src); +} else if (ipv6_addr_is_set(>ipv6_src)) { +ip6 = tunnel->ipv6_src; +} else { +return NULL; +} + +if (!ovs_router_lookup(0, , iface, NULL, )) { +return (NULL); +} + +tunnel_netdev = netdev_from_name(iface); +return tunnel_netdev; +} diff --git a/lib/flow.h b/lib/flow.h index 5b6585f11..a67abc9c9 100644 --- a/lib/flow.h +++ b/lib/flow.h @@ -73,6 +73,7 @@ void flow_extract(struct dp_packet *, struct flow *); void flow_zero_wildcards(struct flow *, const struct flow_wildcards *); void flow_unwildcard_tp_ports(const struct flow *, struct flow_wildcards *); void flow_get_metadata(const struct flow *, struct match *flow_metadata); +struct netdev *flow_get_tunnel_netdev(struct flow_tnl *tunnel); const char *ct_state_to_string(uint32_t state); uint32_t ct_state_from_string(const
[ovs-dev] [PATCH 2/4] ovs-dev: Gather packets-per-second rate of flows
This is the second patch in the patch-set to support dynamic rebalancing of offloaded flows. The packets-per-second (pps) rate for each flow is computed in the context of revalidator threads when the flow stats are retrieved. The pps-rate is computed only after a flow is revalidated and is not scheduled for deletion. The parameters used to compute pps and the pps itself are saved in udpif_key since they need to be persisted across iterations of rebalancing. Signed-off-by: Sriharsha Basavapatna Co-authored-by: Venkat Duvvuru Signed-off-by: Venkat Duvvuru Reviewed-by: Sathya Perla --- lib/dpif-provider.h | 1 + ofproto/ofproto-dpif-upcall.c | 157 ++ 2 files changed, 158 insertions(+) diff --git a/lib/dpif-provider.h b/lib/dpif-provider.h index 62b3598ac..cc6571b28 100644 --- a/lib/dpif-provider.h +++ b/lib/dpif-provider.h @@ -39,6 +39,7 @@ struct dpif { char *full_name; uint8_t netflow_engine_type; uint8_t netflow_engine_id; +long long int current_ms; }; void dpif_init(struct dpif *, const struct dpif_class *, const char *name, diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c index 85f579251..6f4101f36 100644 --- a/ofproto/ofproto-dpif-upcall.c +++ b/ofproto/ofproto-dpif-upcall.c @@ -42,6 +42,7 @@ #include "tunnel.h" #include "unixctl.h" #include "openvswitch/vlog.h" +#include "lib/dpif-provider.h" #define MAX_QUEUE_LENGTH 512 #define UPCALL_MAX_BATCH 64 @@ -304,6 +305,16 @@ struct udpif_key { uint32_t key_recirc_id; /* Non-zero if reference is held by the ukey. */ struct recirc_refs recircs; /* Action recirc IDs with references held. */ + +#define OFFL_REBAL_INTVL_MSEC 3000/* dynamic offload rebalance freq */ +struct netdev *in_netdev; /* in_odp_port's netdev */ +struct netdev *out_netdev; /* out_odp_port's netdev */ +struct netdev *tunnel_netdev; /* tunnel netdev */ +bool offloaded;/* True if flow is offloaded */ +float flow_pps_rate; /* Packets-Per-Second rate */ +long long int flow_time; /* last pps update time */ +uint64_t flow_packets; /* #pkts seen in interval */ +uint64_t flow_backlog_packets; /* prev-mode #pkts (offl or kernel) */ }; /* Datapath operation with optional ukey attached. */ @@ -1667,6 +1678,11 @@ ukey_create__(const struct nlattr *key, size_t key_len, ukey->stats.used = used; ukey->xcache = NULL; +ukey->offloaded = false; +ukey->flow_time = 0; +ukey->in_netdev = ukey->out_netdev = ukey->tunnel_netdev = NULL; +ukey->flow_packets = ukey->flow_backlog_packets = 0; + ukey->key_recirc_id = key_recirc_id; recirc_refs_init(>recircs); if (xout) { @@ -2442,6 +2458,143 @@ reval_op_init(struct ukey_op *op, enum reval_result result, } } +/* + * Given a dpif_flow, get its input and output ports (netdevs) by parsing + * the flow keys and actions. Save them in udpif_key. + */ +static void +dpif_flow_to_netdevs(struct dpif *dpif, struct udpif_key *ukey, + struct dpif_flow *f) +{ +const struct dpif_class *dpif_class = dpif->dpif_class; +odp_port_t out_port = ODPP_NONE; +odp_port_t in_port = ODPP_NONE; +const struct nlattr *a; +const struct nlattr *k; +bool forward = false; +unsigned int left; + +ukey->in_netdev = NULL; +ukey->out_netdev = NULL; + +/* Capture the output port */ +NL_ATTR_FOR_EACH (a, left, f->actions, f->actions_len) { +enum ovs_action_attr type = nl_attr_type(a); +if (type == OVS_ACTION_ATTR_OUTPUT) { +/* Only unicast is supported */ +if (forward) { +ukey->out_netdev = NULL; +ukey->in_netdev = NULL; +return; +} + +forward = true; +out_port = nl_attr_get_odp_port(a); +ukey->out_netdev = netdev_ports_get(out_port, dpif_class); +break; +} +} + +/* Now find the input port */ +NL_ATTR_FOR_EACH (k, left, f->key, f->key_len) { +unsigned int type; +struct flow_tnl tnl; +enum odp_key_fitness res; + +type = nl_attr_type(k); + +switch (type) { +case OVS_KEY_ATTR_IN_PORT: +in_port = *(odp_port_t *)nl_attr_get(k); +ukey->in_netdev = netdev_ports_get(in_port, dpif_class); +VLOG_DBG("%s: in_netdev: %s\n", __func__, ukey->in_netdev->name); +break; +case OVS_KEY_ATTR_TUNNEL: +res = odp_tun_key_from_attr(k, ); +if (res == ODP_FIT_ERROR) { +ukey->tunnel_netdev = NULL; +break; +} +ukey->tunnel_netdev = flow_get_tunnel_netdev(); +VLOG_DBG("%s: tunnel_netdev: %s\n", __func__, + ukey->tunnel_netdev->name); +break; +default: +break; +} +
[ovs-dev] [PATCH 0/4] Support dynamic rebalancing of offloaded flows
With the current OVS offload design, when an offload-device fails to add a flow rule and returns an error, OVS adds the rule to the kernel datapath. The flow gets processed by the kernel datapath for the entire life of that flow. This is fine when an error is returned by the device due to lack of support for certain keys or actions. But when an error is returned due to temporary conditions such as lack of resources to add a flow rule, the flow continues to be processed by kernel even when resources become available later. That is, those flows never get offloaded again. This problem becomes more pronounced when a flow that has been initially offloaded may have a smaller packet rate than a later flow that could not be offloaded due to lack of resources. This leads to inefficient use of HW resources and wastage of host CPU cycles. This patch-set addresses this issue by providing a way to detect temporary offload resource constraints (Out-Of-Resource or OOR condition) and to selectively and dynamically offload flows with a higher packets-per-second (pps) rate. This dynamic rebalancing is done periodically on netdevs that are in OOR state until resources become available to offload all pending flows. The patch-set involves the following changes at a high level: 1. Detection of Out-Of-Resources (OOR) condition on an offload-capable netdev. 2. Gathering flow offload selection criteria for all flows on an OOR netdev; i.e, packets-per-second (pps) rate of flows for offloaded and non-offloaded (pending) flows. 3. Dynamically replacing offloaded flows with a lower pps-rate, with non-offloaded flows with a higher pps-rate, on an OOR netdev. 4. A new OpenvSwitch configuration option - "offload-rebalancing" to enable this policy. ** Sriharsha Basavapatna (4): ovs-dev: Detect Out-Of-Resource condition on a netdev ovs-dev: Gather packets-per-second rate of flows ovs-dev: Rebalance offloaded flows based on the pps rate ovs-dev: Add an openvswitch option to enable dynamic rebalancing of flows lib/dpif-netdev.c | 3 +- lib/dpif-netlink.c| 37 ++- lib/dpif-provider.h | 8 +- lib/dpif.c| 20 +- lib/dpif.h| 20 +- lib/flow.c| 27 ++ lib/flow.h| 1 + lib/netdev-provider.h | 8 + lib/netdev.c | 45 +++ lib/netdev.h | 2 + ofproto/ofproto-dpif-upcall.c | 593 +- vswitchd/vswitch.xml | 22 ++ 12 files changed, 763 insertions(+), 23 deletions(-) -- 2.18.0.rc1.1.g6f333ff ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [ovs-dev, patch_v1] db-ctl-base: Use boolean variable values.
0-day Robot writes: > Bleep bloop. Greetings Darrell Ball, I am a robot and I have tried out your > patch. > Thanks for your contribution. > > I encountered some error that I wasn't expecting. See the details below. > > > checkpatch: > ERROR: Too many signoffs; are you missing Co-authored-by lines? > Lines checked: 37, Warnings: 0, Errors: 1 > > > Please check this out. If you feel there has been an error, please > email acon...@bytheb.org Sorry for the noise on this one, Darrell. The patch got applied before the bot processed it - so it noticed Ben's signoff as well. I'll fix up the bot. > Thanks, > 0-day Robot ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] oss-fuzz: Move oss-fuzz test harnesses and fuzzer configs to ovs source repo
Hi Ben, > We do need a Signed-off-by on any patch. Can you provide one? Sure, this should not be a problem. I'm not sure how the patch was generated from my pull request. Since you prefer patches, I can switch to git-format-patch or something like that to take this forward. I will do this once I have a new patch ready that addresses your comments. > It would be helpful to include a little bit of explanation of the > purpose of the patch in the body of the commit message. I guess you did > that in the top-level pull request at > https://github.com/openvswitch/ovs/pull/242, but it doesn't show up in > the patch itself. Ack. > Normally I expect new code to be mentioned in some makefile. It's not > obvious to me how this gets built. Does it get built by oss-fuzz itself > somehow? Or are you building it by hand? Or something else? The new code (test harnesses) get built from a build script inside the oss-fuzz repo: https://github.com/google/oss-fuzz/blob/master/projects/openvswitch/build.sh Please bear in mind that the build script is invoked inside a Docker container defined here: https://github.com/google/oss-fuzz/blob/master/projects/openvswitch/Dockerfile Also, please note that this patch is aimed at moving the test harnesses you are going to find in oss-fuzz to OvS so that oss-fuzz/project/openvswitch contains the bare minimum: project metadata (project.yaml), Dockerfile, and build script. > This causes some build breakage at "make" time. I'll walk you through > it. First, we normally expect #include at the beginning of > each file. Since I don't understand how this gets built, maybe that's > not appropriate (but in that case we need to update our blacklist for > this rule): > > tests/oss-fuzz/flow_extract_target.c > tests/oss-fuzz/json_parser_target.c > tests/oss-fuzz/ofp_print_target.c > See above for list of violations of the rule that > every C source file must #include . Is this comment still valid in the light of my comment on build process? > Please change tabs to spaces: > > tests/oss-fuzz/ofp_print_target.c > See above for files that use tabs for indentation. > Please use spaces instead. Ack. > The new files need to get mentioned in an automake.mk, at least in > EXTRA_DIST, to ensure that "make dist" will put them into the tarball: > > The following files are in git but not the distribution: > tests/oss-fuzz/config/flow_extract_fuzzer.options > tests/oss-fuzz/config/json_parser_fuzzer.options > tests/oss-fuzz/config/ofp_print_fuzzer.options > tests/oss-fuzz/config/ovs.dict > tests/oss-fuzz/flow_extract_target.c > tests/oss-fuzz/json_parser_target.c > tests/oss-fuzz/ofp_print_target.c There are several automake files to choose from. How do I do this? > checkpatch has the following to say about coding style; it would be nice > to fix all of it: > > ERROR: Inappropriate spacing in pointer declaration > WARNING: Line lacks whitespace around operator > #357 FILE: tests/oss-fuzz/flow_extract_:s.c:4: > int LLVMFuzzerTestOneInput(const uint8_t* data, size_t size) > > ERROR: Inappropriate spacing in pointer declaration > WARNING: Line lacks whitespace around operator > #388 FILE: tests/oss-fuzz/json_parser_target.c:17: > int LLVMFuzzerTestOneInput(const uint8_t* data, size_t size) > > ERROR: Inappropriate bracing around statement > #390 FILE: tests/oss-fuzz/json_parser_target.c:19: > if ((size == 0) || (data[size-1] != '\0')) return 0; > > WARNING: Line lacks whitespace around operator > #392 FILE: tests/oss-fuzz/json_parser_target.c:21: > struct json *j1,*j2; > > WARNING: Line has trailing whitespace > #402 FILE: tests/oss-fuzz/json_parser_target.c:31: > > WARNING: Line has trailing whitespace > #414 FILE: tests/oss-fuzz/json_parser_target.c:43: > > ERROR: Inappropriate spacing in pointer declaration > #446 FILE: tests/oss-fuzz/ofp_print_target.c:6: > int LLVMFuzzerTestOneInput(const uint8_t* data, size_t size) > > ERROR: Inappropriate bracing around statement > #450 FILE: tests/oss-fuzz/ofp_print_target.c:10: > if (size < sizeof(struct ofp_header)) return 0; Ack. Regards, Bhargava ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v3 00/14] Support multi-segment mbufs
On 5 Jul 2018, at 18:29, Tiago Lam wrote: Overview This patchset introduces support for multi-segment mbufs to OvS-DPDK. Multi-segment mbufs are typically used when the size of an mbuf is insufficient to contain the entirety of a packet's data. Instead, the data is split across numerous mbufs, each carrying a portion, or 'segment', of the packet data. mbufs are chained via their 'next' attribute (an mbuf pointer). Use Cases = i. Handling oversized (guest-originated) frames, which are marked for hardware accelration/offload (TSO, for example). Packets which originate from a non-DPDK source may be marked for offload; as such, they may be larger than the permitted ingress interface's MTU, and may be stored in an oversized dp-packet. In order to transmit such packets over a DPDK port, their contents must be copied to a DPDK mbuf (via dpdk_do_tx_copy). However, in its current implementation, that function only copies data into a single mbuf; if the space available in the mbuf is exhausted, but not all packet data has been copied, then it is lost. Similarly, when cloning a DPDK mbuf, it must be considered whether that mbuf contains multiple segments. Both issues are resolved within this patchset. ii. Handling jumbo frames. While OvS already supports jumbo frames, it does so by increasing mbuf size, such that the entirety of a jumbo frame may be handled in a single mbuf. This is certainly the preferred, and most performant approach (and remains the default). Enabling multi-segment mbufs Multi-segment and single-segment mbufs are mutually exclusive, and the user must decide on which approach to adopt on init. The introduction of a new OVSDB field, 'dpdk-multi-seg-mbufs', facilitates this. This is a global boolean value, which determines how jumbo frames are represented across all DPDK ports. In the absence of a user-supplied value, 'dpdk-multi-seg-mbufs' defaults to false, i.e. multi-segment mbufs must be explicitly enabled / single-segment mbufs remain the default. Setting the field is identical to setting existing DPDK-specific OVSDB fields: ovs-vsctl set Open_vSwitch . other_config:dpdk-init=true ovs-vsctl set Open_vSwitch . other_config:dpdk-lcore-mask=0x10 ovs-vsctl set Open_vSwitch . other_config:dpdk-socket-mem=4096,0 ==> ovs-vsctl set Open_vSwitch . other_config:dpdk-multi-seg-mbufs=true Performance notes (based on v8) = In order to test for regressions in performance, tests were run on top of master 88125d6 and v8 of this patchset, both with the multi-segment mbufs option enabled and disabled. VSperf was used to run the phy2phy_cont and pvp_cont tests with varying packet sizes of 64B, 1500B and 7000B, on a 10Gbps interface. Test | Size | Master | Multi-seg disabled | Multi-seg enabled - p2p | 64 | ~22.7 | ~22.65| ~18.3 p2p | 1500 | ~1.6 |~1.6|~1.6 p2p | 7000 | ~0.36 | ~0.36| ~0.36 pvp | 64 | ~6.7 |~6.7|~6.3 pvp | 1500 | ~1.6 |~1.6|~1.6 pvp | 7000 | ~0.36 | ~0.36| ~0.36 Packet size is in bytes, while all packet rates are reported in mpps (aggregated). No noticeable regression has been observed (certainly everything is within the ± 5% margin of existing performance), aside from the 64B packet size case when multi-segment mbuf is enabled. This is expected, however, because of how Tx vectoriszed functions are incompatible with multi-segment mbufs on some PMDs. The PMD under use during these tests was the i40e (on a Intel X710 NIC), which indeed doesn't support vectorized Tx functions with multi-segment mbufs. Thanks for all the work Tiago! It all looks good to me, so hereby I would like to ack the series. Cheers, Eelco Acked-by: Eelco Chaudron ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] Monitor Real-Time locations for your Employees
Hi, I recently emailed you to tell how field sales work is optimized for mobile work-force and I haven't heard back from you in a while. Let's schedule a 10 minutes call so that I can explain how we're working and tell you more details of Travelize features. I'm looking forward to hear from you Thanks and Regards Ricky Mobile: 7406878981 From: Ricky [mailto:ri...@acesoftwareproducts.com] Sent: 04 July 2018 13:03 Subject: Monitor Real-Time locations for your Employees Hi, I hope you are doing well. I am delighted to share with you "THE SECRET BEHIND SUCCESSFUL SALES TEAM MANAGEMENT". I found this is beneficial to you & your organization. Check: 5 Advantages of Smart Work-flow: 1. Automate work-force. 2. Increases transparency. 3. Instant communication. 4. Improves business efficiency. 5. Saves time & cost. I think it's something that your organization might see immediate value in. Looking forward to your response Regards Ricky Travelize Address - #No.5, First Floor, Race Course Road, Bengaluru-560009, Karnataka, India Mobile: 7406878981 If you are not interested please reply in subject line as UN-SUBSCRIBE ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev