Re: [ovs-dev] [PATCH] Remove oss-fuzz tests carried over from ovs
Bleep bloop. Greetings , 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. git-am: fatal: sha1 information is lacking or useless (tests/automake.mk). Repository lacks necessary blobs to fall back on 3-way merge. Cannot fall back to three-way merge. Patch failed at 0001 Remove oss-fuzz tests carried over from ovs The copy of the patch that failed is found in: /var/lib/jenkins/jobs/upstream_build_from_pw/workspace/.git/rebase-apply/patch When you have resolved this problem, run "git am --resolved". If you prefer to skip this patch, run "git am --skip" instead. To restore the original branch and stop patching, run "git am --abort". Please check this out. If you feel there has been an error, please email acon...@redhat.com Thanks, 0-day Robot ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH] Remove oss-fuzz tests carried over from ovs
From: Bhargava Shastry It appears that ossfuzz specific test harnesses and configuration files were carried over to the ovn repo from the ovs repo without justification. This patch removes them until there is a need to continuously fuzz ovn code as the ovs code is currently fuzzed. Signed-off-by: Bhargava Shastry --- tests/automake.mk | 2 - tests/oss-fuzz/automake.mk| 66 --- tests/oss-fuzz/config/expr.dict | 120 - .../oss-fuzz/config/expr_parse_target.options | 3 - .../config/flow_extract_target.options| 3 - .../config/json_parser_target.options | 2 - tests/oss-fuzz/config/miniflow_target.options | 3 - tests/oss-fuzz/config/odp.dict| 170 --- tests/oss-fuzz/config/odp_target.options | 3 - .../config/ofctl_parse_target.options | 3 - tests/oss-fuzz/config/ofp-flow.dict | 45 -- .../oss-fuzz/config/ofp_print_target.options | 3 - tests/oss-fuzz/config/ovs.dict| 293 --- tests/oss-fuzz/expr_parse_target.c| 464 -- tests/oss-fuzz/flow_extract_target.c | 100 tests/oss-fuzz/fuzzer.h | 9 - tests/oss-fuzz/json_parser_target.c | 42 -- tests/oss-fuzz/miniflow_target.c | 365 -- tests/oss-fuzz/odp_target.c | 149 -- tests/oss-fuzz/ofctl_parse_target.c | 113 - tests/oss-fuzz/ofp_print_target.c | 47 -- 21 files changed, 2005 deletions(-) delete mode 100644 tests/oss-fuzz/automake.mk delete mode 100644 tests/oss-fuzz/config/expr.dict delete mode 100644 tests/oss-fuzz/config/expr_parse_target.options delete mode 100644 tests/oss-fuzz/config/flow_extract_target.options delete mode 100644 tests/oss-fuzz/config/json_parser_target.options delete mode 100644 tests/oss-fuzz/config/miniflow_target.options delete mode 100644 tests/oss-fuzz/config/odp.dict delete mode 100644 tests/oss-fuzz/config/odp_target.options delete mode 100644 tests/oss-fuzz/config/ofctl_parse_target.options delete mode 100644 tests/oss-fuzz/config/ofp-flow.dict delete mode 100644 tests/oss-fuzz/config/ofp_print_target.options delete mode 100644 tests/oss-fuzz/config/ovs.dict delete mode 100644 tests/oss-fuzz/expr_parse_target.c delete mode 100644 tests/oss-fuzz/flow_extract_target.c delete mode 100644 tests/oss-fuzz/fuzzer.h delete mode 100644 tests/oss-fuzz/json_parser_target.c delete mode 100644 tests/oss-fuzz/miniflow_target.c delete mode 100644 tests/oss-fuzz/odp_target.c delete mode 100644 tests/oss-fuzz/ofctl_parse_target.c delete mode 100644 tests/oss-fuzz/ofp_print_target.c diff --git a/tests/automake.mk b/tests/automake.mk index 013e59280..e86a5273e 100644 --- a/tests/automake.mk +++ b/tests/automake.mk @@ -246,5 +246,3 @@ clean-pki: rm -f tests/pki/stamp rm -rf tests/pki endif - -include tests/oss-fuzz/automake.mk diff --git a/tests/oss-fuzz/automake.mk b/tests/oss-fuzz/automake.mk deleted file mode 100644 index 5bf7d0d7c..0 --- a/tests/oss-fuzz/automake.mk +++ /dev/null @@ -1,66 +0,0 @@ -OSS_FUZZ_TARGETS = \ - tests/oss-fuzz/flow_extract_target \ - tests/oss-fuzz/json_parser_target \ - tests/oss-fuzz/ofp_print_target \ - tests/oss-fuzz/expr_parse_target \ - tests/oss-fuzz/odp_target \ - tests/oss-fuzz/miniflow_target \ - tests/oss-fuzz/ofctl_parse_target -EXTRA_PROGRAMS += $(OSS_FUZZ_TARGETS) -oss-fuzz-targets: $(OSS_FUZZ_TARGETS) - -tests_oss_fuzz_flow_extract_target_SOURCES = \ - tests/oss-fuzz/flow_extract_target.c \ - tests/oss-fuzz/fuzzer.h -tests_oss_fuzz_flow_extract_target_LDADD = lib/libopenvswitch.la -tests_oss_fuzz_flow_extract_target_LDFLAGS = $(LIB_FUZZING_ENGINE) -lc++ - -tests_oss_fuzz_json_parser_target_SOURCES = \ - tests/oss-fuzz/json_parser_target.c \ - tests/oss-fuzz/fuzzer.h -tests_oss_fuzz_json_parser_target_LDADD = lib/libopenvswitch.la -tests_oss_fuzz_json_parser_target_LDFLAGS = $(LIB_FUZZING_ENGINE) -lc++ - -tests_oss_fuzz_ofp_print_target_SOURCES = \ - tests/oss-fuzz/ofp_print_target.c \ - tests/oss-fuzz/fuzzer.h -tests_oss_fuzz_ofp_print_target_LDADD = lib/libopenvswitch.la -tests_oss_fuzz_ofp_print_target_LDFLAGS = $(LIB_FUZZING_ENGINE) -lc++ - -tests_oss_fuzz_expr_parse_target_SOURCES = \ -tests/oss-fuzz/expr_parse_target.c \ -tests/oss-fuzz/fuzzer.h -tests_oss_fuzz_expr_parse_target_LDADD = lib/libopenvswitch.la \ - ovn/lib/libovn.la -tests_oss_fuzz_expr_parse_target_LDFLAGS = $(LIB_FUZZING_ENGINE) -lc++ - -tests_oss_fuzz_odp_target_SOURCES = \ -tests/oss-fuzz/odp_target.c \ -tests/oss-fuzz/fuzzer.h -tests_oss_fuzz_odp_target_LDADD = lib/libopenvswitch.la -tests_oss_fuzz_odp_target_LDFLAGS = $(LIB_FUZZING_ENGINE) -lc++ - -tests_oss_fuzz_miniflow_target_SOURCES = \ -
Re: [ovs-dev] [RFC ovn] Add CoPP (Control Plane Protection).
On Fri, Oct 18, 2019 at 3:53 AM Han Zhou wrote: > > > > On Thu, Oct 10, 2019 at 1:20 AM Dumitru Ceara wrote: > > > > Add new 'Copp' (Control plane protection) table to OVN Northbound DB: > > - this stores mappings between control plane protocol names and meters > > that should be used to rate limit controller-destined traffic for > > those protocols. > > > > Add new 'copp' columns to the following OVN Northbound DB tables: > > - Logical_Switch > > - Logical_Switch_Port > > - Logical_Router > > - Logical_Router_Port > > > > This allows defining control plane policies with different > > granularities. For example a user can decide to enforce a general > > policy for the logical switch but at the same time configure a > > different policy on some of the ports of the logical switch. > > Control plane protocol policies applied to a logical port take > > precedence over the ones defined at logical switch level. For > > logical routers and logical router ports we take the same approach. > > > > Add a new 'controller_meter' column to OVN Southbound Logical_Flow > > table. This stores an optional string which should correspond to > > the Meter that must be used for rate limiting controller actions > > generated by packets hitting the flow. > > > > Add CLI commands in 'ovn-nbctl' to allow the user to manage Control > > Plane Protection Policies at different levels (logical switch, > > logical router, logical port). > > > > Add a new 'ctrl_meter_id' field to 'struct ovn_flow' to be used for > > applying meters to flows that trigger controller actions. > > > > Add a new 'ofctrl_add_flow_meter' function to create a new 'ovn_flow' > > with an attached controller meter. > > > > Change ofctrl_check_and_add_flow to allow specifying a meter ID for > > packets that are punted to controller. > > > > Change consider_logical_flow to parse controller_meter from the logical > > flow and use it when building openflow entries. > > > > Add a new 'ctrl_meter_id' field to 'struct ovnact_encode_params' to be > > used when encoding controller actions from logical flow actions. > > > > Change the ovn-northd implementation to set the new 'controller_meter' > > field for flows that need to punt packets to ovn-controller. For some > > protocols (ARP/ND/DNS) install two sets of flows: > > - one flow with a lower priority for the whole datapath using the > > per-datapath CoPP policy. > > - one flow per port with a higher priority than the per-datapath one if > > there is a per-port CoPP defined for the given port. > > > > Post-RFC remaining items: > > - add autotests for CoPP > > > > Reported-at: > > https://mail.openvswitch.org/pipermail/ovs-dev/2019-September/362732.html > > CC: Han Zhou > > CC: Numan Siddique > > Signed-off-by: Dumitru Ceara > > Thanks Dumitru for the RFC. I didn't review in too much detail but just some > quick feedback. Hi Han, Thanks for having a look. > Overall, it is great that this approach addresses rate limit in a generic way. > It is easy to understand the meters applied to router/switch because each > lflow belongs to a datapath. > However, could you describe more about how the meters for a > logical_switch_port or logical_router_port is defined? Is it applied when the > specific port is the ingress port, or egress port, or both? One of the things the RFC misses is thorough documentation. I will add that in the next version. The meters (regardless if they're defined globally for the switch/router or on a port) are applied only for traffic that hits a flow with action=controller(..). So it's ingress traffic in the logical topology. There might be cases when the packets injected by controller will hit flows that send them back to a controller (sometimes different). One example that comes to mind is a GARP originated for a logical router port that reaches another logical router port which might or might not be on the same controller. In this case too, because the packet hits a flow with action=controller(..), the metering will be performed. > Regarding the attacking problem we have discussed before, when there is > attack from a single src through a single router port, even if we enforce a > dedicated meter on the router port level, e.g. for ARP resolve, then all ARP > resolving through that router port would still be impacted. Did you consider > flow based meters to lower the impact in that situation? Agreed, in case of an attack on a metered port, all incoming ARP traffic on that port will be metered potentially affecting legit packets. It's a first step. What I had in mind for the next step is to allow finer grain rate limiting for protocols where this is applicable (e.g., ARP) by splitting the openflow in two parts: - first hash the packets - then rate limit per hash bucket So a flow like: table=X, match="arp", action="controller()" Would be changed to (assuming 16 hash buckets): table=X, match="arp", action="multipath(symmetric_l3, modulo_n, 16, 0, NXM_NX_REGX[]),
Re: [ovs-dev] [PATCH v9 2/2] netdev-dpdk:Detailed packet drop statistics
On 16.10.2019 19:48, Kevin Traynor wrote: On 16/10/2019 13:16, Ilya Maximets wrote: Hi Kevin, Thanks for review. Some comments inline. On 16.10.2019 12:15, Kevin Traynor wrote: Hi Sriram, Thanks for working on making more fine grained stats for debugging. As mentioned yesterday, this patch needs rebase so I just reviewed docs and netdev-dpdk.c which applied. Comments below. On 21/09/2019 03:40, Sriram Vatala wrote: OVS may be unable to transmit packets for multiple reasons and today there is a single counter to track packets dropped due to any of those reasons. The most common reason is that a VM is unable to read packets fast enough causing the vhostuser port transmit queue on the OVS side to become full. This manifests as a problem with VNFs not receiving all packets. Having a separate drop counter to track packets dropped because the transmit queue is full will clearly indicate that the problem is on the VM side and not in OVS. Similarly maintaining separate It reads like a bit of a contradiction. On one hand the "The *most common* reason is that a VM is unable to read packets fast enough". While having a stat "*will clearly indicate* that the problem is on the VM side". counters for all possible drops helps in indicating sensible cause for packet drops. This patch adds custom software stats counters to track packets dropped at port level and these counters are displayed along with other stats in "ovs-vsctl get interface statistics" command. The detailed stats will be available for both dpdk and vhostuser ports. I think the commit msg could be a bit clearer, suggest something like below - feel free to modify (or reject): OVS may be unable to transmit packets for multiple reasons on the DPDK datapath and today there is a single counter to track packets dropped due to any of those reasons. This patch adds custom software stats for the different reasons packets may be dropped during tx on the DPDK datapath in OVS. - MTU drops : drops that occur due to a too large packet size - Qos drops: drops that occur due to egress QOS - Tx failures: drops as returned by the DPDK PMD send function Note that the reason for tx failures is not specificied in OVS. In practice for vhost ports it is most common that tx failures are because there are not enough available descriptors, which is usually caused by misconfiguration of the guest queues and/or because the guest is not consuming packets fast enough from the queues. These counters are displayed along with other stats in "ovs-vsctl get interface statistics" command and are available for dpdk and vhostuser/vhostuserclient ports. Signed-off-by: Sriram Vatala --- v9:... v8:... Note that signed-off, --- and version info should be like this ^^^. otherwise the review version comments will be in the git commit log when it is applied. -- Changes since v8: Addressed comments given by Ilya. Signed-off-by: Sriram Vatala --- Documentation/topics/dpdk/vhost-user.rst | 13 ++- lib/netdev-dpdk.c | 81 +++ utilities/bugtool/automake.mk | 3 +- utilities/bugtool/ovs-bugtool-get-port-stats | 25 ++ .../plugins/network-status/openvswitch.xml| 1 + 5 files changed, 105 insertions(+), 18 deletions(-) create mode 100755 utilities/bugtool/ovs-bugtool-get-port-stats diff --git a/Documentation/topics/dpdk/vhost-user.rst b/Documentation/topics/dpdk/vhost-user.rst index 724aa62f6..89388a2bf 100644 --- a/Documentation/topics/dpdk/vhost-user.rst +++ b/Documentation/topics/dpdk/vhost-user.rst @@ -551,7 +551,18 @@ processing packets at the required rate. The amount of Tx retries on a vhost-user or vhost-user-client interface can be shown with:: - $ ovs-vsctl get Interface dpdkvhostclient0 statistics:tx_retries + $ ovs-vsctl get Interface dpdkvhostclient0 statistics:netdev_dpdk_tx_retries I think the "netdev_dpdk_" prefixes should be removed for a few reasons, - These are custom stats that will only be displayed for the dpdk ports so they don't need any extra prefix to say they are for dpdk ports - The 'tx_retries' stat is part of OVS 2.12, I don't like to change its name to a different one in OVS 2.13 unless there is a very good reason - The existing stats don't have this type of prefixes (granted most of them are general stats): The main reason for the prefix for me is to distinguish those stats from the stats that comest from the driver/HW. Our new stats has very generic names that could be named a lot like or even equal to HW stats. This might be not an issue for vhostuser, but for HW NICs this may be really confusing for users. I'm not insisting on 'netdev_dpdk_' prefix, but, IMHO, we need a way to clearly distinguish those stats. We may use different prefixes like 'sw_' or just 'ovs_'. ok, I can see that we might want to distinguish custom stats to indicate that they are specific for that device but that also has the side effect of making
Re: [ovs-dev] [PATCH 0/5] Backport upstream Linux kernel patches
On Thu, Oct 17, 2019 at 01:32:27PM -0700, Gregory Rose wrote: > On 10/9/2019 2:22 PM, Greg Rose wrote: > > Pull in upstream kernel patches not related to conntrack and > > add compatibility layer code where needed. > > > > The upstream conntrack related patches will come later from Yi-Hung. > > > > Passes Travis: > > https://travis-ci.org/gvrose8192/ovs-experimental/builds/595755449 > > > > Passes check-kmod with no regressions on Ubuntu 16.04 and 18.04 > > > > Arnd Bergmann (1): > >datapath: hide clang frame-overflow warnings > > > > Greg Rose (1): > >datapath: compat: drop bridge nf reset from nf_reset > > > > Haishuang Yan (1): > >compat: remove the incorrect mtu limit for erspan > > > > Li RongQing (1): > >datapath: change type of UPCALL_PID attribute to NLA_UNSPEC > > > > Pablo Neira Ayuso (1): > >datapath: rename flow_stats to sw_flow_stats > > > > acinclude.m4 | 1 + > > datapath/datapath.c | 17 + > > datapath/flow.c | 8 > > datapath/flow.h | 4 ++-- > > datapath/flow_table.c| 6 +++--- > > datapath/linux/compat/include/linux/skbuff.h | 4 > > datapath/linux/compat/ip_gre.c | 3 +++ > > datapath/vport-internal_dev.c| 2 +- > > 8 files changed, 27 insertions(+), 18 deletions(-) > > > > Hello maintainers, > > Yi-Hung has added his acks. Please consider applying these patches. > > Thanks, > > - Greg Thanks Greg, applied to master. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] tc: Limit the max action number to 16
On Wed, Oct 16, 2019 at 11:53:52AM +, Roi Dayan wrote:
>
>
> On 2019-10-16 2:40 PM, Simon Horman wrote:
> > On Wed, Oct 16, 2019 at 11:37:14AM +0300, Roi Dayan wrote:
> >> From: Chris Mi
> >>
> >> Currently, ovs supports to offload max TCA_ACT_MAX_PRIO(32) actions.
> >> But net sched api has a limit of 4K message size which is not enough
> >> for 32 actions when echo flag is set.
> >>
> >> After a lot of testing, we find that 16 actions is a reasonable number.
> >> So in this commit, we introduced a new define to limit the max actions.
> >>
> >> Fixes: 0c70132cd288("tc: Make the actions order consistent")
> >> Signed-off-by: Chris Mi
> >> Reviewed-by: Roi Dayan
> >
> > Hi Chris,
> >
> > I'm unclear on what problem is this patch solving.
>
> Hi Simon,
>
> I can help with the answer here.
> When ovs send netlink msg to tc to add a filter we also add
> the echo flag to get a reply data. this reply can be big as
> it contains everything from the request and if it pass the 4K
> size then the kernel will just not fill/send it but the return
> status of the tc command is still 0 for success.
>
> In ovs we use that reply to get back the handle id on success but
> for this case will fail.
> To make sure this ack reply always exists for successful tc calls
> we limit the number of actions here to avoid reaching the 4K size limit.
Thanks,
It sounds like it would be good to develop a mechanism where
the information required can be retrieved in a less verbose manner,
thus allowing a higher limit.
But I do agree that this resolves a problem and I have applied it to
master. Let me know if you think it is also appropriate for older branches.
Kind regards,
Simon
>
> Thanks,
> Roi
>
> >
> >> ---
> >> lib/netdev-offload-tc.c | 4 ++--
> >> lib/tc.c| 6 +++---
> >> lib/tc.h| 4 +++-
> >> 3 files changed, 8 insertions(+), 6 deletions(-)
> >>
> >> diff --git a/lib/netdev-offload-tc.c b/lib/netdev-offload-tc.c
> >> index 6814390eab2f..f6d1abb2e695 100644
> >> --- a/lib/netdev-offload-tc.c
> >> +++ b/lib/netdev-offload-tc.c
> >> @@ -1360,8 +1360,8 @@ netdev_tc_flow_put(struct netdev *netdev, struct
> >> match *match,
> >> }
> >>
> >> NL_ATTR_FOR_EACH(nla, left, actions, actions_len) {
> >> -if (flower.action_count >= TCA_ACT_MAX_PRIO) {
> >> -VLOG_DBG_RL(, "Can only support %d actions",
> >> flower.action_count);
> >> +if (flower.action_count >= TCA_ACT_MAX_NUM) {
> >> +VLOG_DBG_RL(, "Can only support %d actions",
> >> TCA_ACT_MAX_NUM);
> >> return EOPNOTSUPP;
> >> }
> >> action = [flower.action_count];
> >> diff --git a/lib/tc.c b/lib/tc.c
> >> index 316a0ee5dc7c..d5487097d8ec 100644
> >> --- a/lib/tc.c
> >> +++ b/lib/tc.c
> >> @@ -1458,7 +1458,7 @@ static int
> >> nl_parse_flower_actions(struct nlattr **attrs, struct tc_flower *flower)
> >> {
> >> const struct nlattr *actions = attrs[TCA_FLOWER_ACT];
> >> -static struct nl_policy actions_orders_policy[TCA_ACT_MAX_PRIO + 1] =
> >> {};
> >> +static struct nl_policy actions_orders_policy[TCA_ACT_MAX_NUM + 1] =
> >> {};
> >> struct nlattr *actions_orders[ARRAY_SIZE(actions_orders_policy)];
> >> const int max_size = ARRAY_SIZE(actions_orders_policy);
> >>
> >> @@ -1477,8 +1477,8 @@ nl_parse_flower_actions(struct nlattr **attrs,
> >> struct tc_flower *flower)
> >> if (actions_orders[i]) {
> >> int err;
> >>
> >> -if (flower->action_count >= TCA_ACT_MAX_PRIO) {
> >> -VLOG_DBG_RL(_rl, "Can only support %d actions",
> >> flower->action_count);
> >> +if (flower->action_count >= TCA_ACT_MAX_NUM) {
> >> +VLOG_DBG_RL(_rl, "Can only support %d actions",
> >> TCA_ACT_MAX_NUM);
> >> return EOPNOTSUPP;
> >> }
> >> err = nl_parse_single_action(actions_orders[i], flower);
> >> diff --git a/lib/tc.h b/lib/tc.h
> >> index f4213579a2fd..f4073c6c47e6 100644
> >> --- a/lib/tc.h
> >> +++ b/lib/tc.h
> >> @@ -208,6 +208,8 @@ enum tc_offloaded_state {
> >> TC_OFFLOADED_STATE_NOT_IN_HW,
> >> };
> >>
> >> +#define TCA_ACT_MAX_NUM 16
> >> +
> >> struct tc_flower {
> >> uint32_t handle;
> >> uint32_t prio;
> >> @@ -216,7 +218,7 @@ struct tc_flower {
> >> struct tc_flower_key mask;
> >>
> >> int action_count;
> >> -struct tc_action actions[TCA_ACT_MAX_PRIO];
> >> +struct tc_action actions[TCA_ACT_MAX_NUM];
> >>
> >> struct ovs_flow_stats stats;
> >> uint64_t lastused;
> >> --
> >> 2.8.4
> >>
> >> ___
> >> dev mailing list
> >> d...@openvswitch.org
> >>
Re: [ovs-dev] [PATCH v13] Improved Packet Drop Statistics in OVS
Are you planning on sending a v14?
Cheers,
Eelco
On 16 Sep 2019, at 16:12, Eelco Chaudron wrote:
Hi Anju,
For some reason, this version does not show up in patchwork! I have
two small comments below but other than that it looks fine to me.
If those two will be the only changes in your next rev, I’ll ack it
straight away :)
Cheers,
Eelco
On 9 Sep 2019, at 13:53, Anju Thomas wrote:
Currently OVS maintains explicit packet drop/error counters only on
port level. Packets that are dropped as part of normal OpenFlow
processing are counted in flow stats of “drop” flows or as table
misses in table stats. These can only be interpreted by controllers
that know the semantics of the configured OpenFlow pipeline.
Without that knowledge, it is impossible for an OVS user to obtain
e.g. the total number of packets dropped due to OpenFlow rules.
Furthermore, there are numerous other reasons for which packets can
be dropped by OVS slow path that are not related to the OpenFlow
pipeline.
The generated datapath flow entries include a drop action to avoid
further expensive upcalls to the slow path, but subsequent packets
dropped by the datapath are not accounted anywhere.
Finally, the datapath itself drops packets in certain error
situations.
Also, these drops are today not accounted for.This makes it difficult
for OVS users to monitor packet drop in an OVS instance and to alert
a management system in case of a unexpected increase of such drops.
Also OVS trouble-shooters face difficulties in analysing packet
drops.
With this patch we implement following changes to address the issues
mentioned above.
1. Identify and account all the silent packet drop scenarios
2. Display these drops in ovs-appctl coverage/show
Co-authored-by: Rohith Basavaraja
Co-authored-by: Keshav Gupta
Signed-off-by: Anju Thomas
Signed-off-by: Rohith Basavaraja
Signed-off-by: Keshav Gupta
---
v13(Eelco and Illya's comments)
1. packet_dropped changed to packets_dropped
2. Uses dp_packet_batch_size instead of packet->size
3. Add version history
v12(Ben's comments)
1. new dp action in kernel block in openvswitch.h
2. change xlate_error enum to be used as u32
3. resetting xlate error in case of congestion drop
and forwarding disabled
datapath/linux/compat/include/linux/openvswitch.h | 1 +
lib/dpif-netdev.c | 47 -
lib/dpif.c| 7 +
lib/dpif.h| 4 +
lib/odp-execute.c | 79
lib/odp-util.c| 9 +
ofproto/ofproto-dpif-ipfix.c | 1 +
ofproto/ofproto-dpif-sflow.c | 1 +
ofproto/ofproto-dpif-xlate.c | 40 -
ofproto/ofproto-dpif-xlate.h | 3 +
ofproto/ofproto-dpif.c| 8 +
ofproto/ofproto-dpif.h| 8 +-
tests/automake.mk | 3 +-
tests/dpif-netdev.at | 8 +
tests/drop-stats.at | 210
++
tests/ofproto-dpif.at | 2 +-
tests/tunnel-push-pop.at | 29 ++-
tests/tunnel.at | 16 +-
18 files changed, 465 insertions(+), 11 deletions(-)
create mode 100644 tests/drop-stats.at
diff --git a/datapath/linux/compat/include/linux/openvswitch.h
b/datapath/linux/compat/include/linux/openvswitch.h
index 65a003a..415c053 100644
--- a/datapath/linux/compat/include/linux/openvswitch.h
+++ b/datapath/linux/compat/include/linux/openvswitch.h
@@ -989,6 +989,7 @@ enum ovs_action_attr {
#ifndef __KERNEL__
OVS_ACTION_ATTR_TUNNEL_PUSH, /* struct ovs_action_push_tnl*/
OVS_ACTION_ATTR_TUNNEL_POP,/* u32 port number. */
+ OVS_ACTION_ATTR_DROP,
#endif
__OVS_ACTION_ATTR_MAX,/* Nothing past this will be accepted
* from userspace. */
diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index a88a78f..8f811be 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -102,6 +102,17 @@ enum { MAX_METERS = 65536 };/* Maximum
number of meters. */
enum { MAX_BANDS = 8 }; /* Maximum number of bands / meter.
*/
enum { N_METER_LOCKS = 64 };/* Maximum number of meters. */
+COVERAGE_DEFINE(datapath_drop_meter);
+COVERAGE_DEFINE(datapath_drop_upcall_error);
+COVERAGE_DEFINE(datapath_drop_lock_error);
+COVERAGE_DEFINE(datapath_drop_userspace_action_error);
+COVERAGE_DEFINE(datapath_drop_tunnel_push_error);
+COVERAGE_DEFINE(datapath_drop_tunnel_pop_error);
+COVERAGE_DEFINE(datapath_drop_recirc_error);
+COVERAGE_DEFINE(datapath_drop_invalid_port);
+COVERAGE_DEFINE(datapath_drop_invalid_tnl_port);
+COVERAGE_DEFINE(datapath_drop_rx_invalid_packet);
+
/* Protects against changes to 'dp_netdevs'. */
static struct ovs_mutex
[ovs-dev] [PATCH] ossfuzz: Simplify miniflow fuzzer harness.
From: Bhargava Shastry
Google's oss-fuzz builder bots were complaining that miniflow_target is
too slow to fuzz in that some tests take longer than a second to
complete. This patch fixes this by replacing the random flow generation
within the harness to a more simpler scenario.
Signed-off-by: Bhargava Shastry
---
tests/oss-fuzz/miniflow_target.c | 139 +++
1 file changed, 10 insertions(+), 129 deletions(-)
diff --git a/tests/oss-fuzz/miniflow_target.c b/tests/oss-fuzz/miniflow_target.c
index 800375d63..d747ff9ae 100644
--- a/tests/oss-fuzz/miniflow_target.c
+++ b/tests/oss-fuzz/miniflow_target.c
@@ -9,17 +9,6 @@
#include "classifier-private.h"
#include "util.h"
-static void
-shuffle_u32s(uint32_t *p, size_t n)
-{
-for (; n > 1; n--, p++) {
-uint32_t *q = [random_range(n)];
-uint32_t tmp = *p;
-*p = *q;
-*q = tmp;
-}
-}
-
/* Returns a copy of 'src'. The caller must eventually free the returned
* miniflow with free(). */
static struct miniflow *
@@ -59,119 +48,8 @@ miniflow_hash__(const struct miniflow *flow, uint32_t basis)
return hash_finish(hash, n_values);
}
-static uint32_t
-random_value(void)
-{
-static const uint32_t values_[] =
-{ 0x, 0x, 0x, 0x8000,
- 0x0001, 0xface, 0x00d00d1e, 0xdeadbeef };
-
-return values_[random_range(ARRAY_SIZE(values_))];
-}
-
-static bool
-choose(unsigned int n, unsigned int *idxp)
-{
-if (*idxp < n) {
-return true;
-} else {
-*idxp -= n;
-return false;
-}
-}
-
#define FLOW_U32S (FLOW_U64S * 2)
-static bool
-init_consecutive_values(int n_consecutive, struct flow *flow,
-unsigned int *idxp)
-{
-uint32_t *flow_u32 = (uint32_t *) flow;
-
-if (choose(FLOW_U32S - n_consecutive + 1, idxp)) {
-int i;
-
-for (i = 0; i < n_consecutive; i++) {
-flow_u32[i + *idxp] = random_value();
-}
-return true;
-} else {
-return false;
-}
-}
-
-static bool
-next_random_flow(struct flow *flow, unsigned int idx)
-{
-uint32_t *flow_u32 = (uint32_t *) flow;
-
-memset(flow, 0, sizeof *flow);
-
-/* Empty flow. */
-if (choose(1, )) {
-return true;
-}
-
-/* All flows with a small number of consecutive nonzero values. */
-for (int i = 1; i <= 4; i++) {
-if (init_consecutive_values(i, flow, )) {
-return true;
-}
-}
-
-/* All flows with a large number of consecutive nonzero values. */
-for (int i = FLOW_U32S - 4; i <= FLOW_U32S; i++) {
-if (init_consecutive_values(i, flow, )) {
-return true;
-}
-}
-
-/* All flows with exactly two nonconsecutive nonzero values. */
-if (choose((FLOW_U32S - 1) * (FLOW_U32S - 2) / 2, )) {
-int ofs1;
-
-for (ofs1 = 0; ofs1 < FLOW_U32S - 2; ofs1++) {
-int ofs2;
-
-for (ofs2 = ofs1 + 2; ofs2 < FLOW_U32S; ofs2++) {
-if (choose(1, )) {
-flow_u32[ofs1] = random_value();
-flow_u32[ofs2] = random_value();
-return true;
-}
-}
-}
-OVS_NOT_REACHED();
-}
-
-/* 16 randomly chosen flows with N >= 3 nonzero values. */
-if (choose(16 * (FLOW_U32S - 4), )) {
-int n = idx / 16 + 3;
-
-for (int i = 0; i < n; i++) {
-flow_u32[i] = random_value();
-}
-shuffle_u32s(flow_u32, FLOW_U32S);
-
-return true;
-}
-
-return false;
-}
-
-static void
-any_random_flow(struct flow *flow)
-{
-static unsigned int max;
-if (!max) {
-while (next_random_flow(flow, max)) {
-max++;
-}
-}
-
-next_random_flow(flow, random_range(max));
-}
-
static void
toggle_masked_flow_bits(struct flow *flow, const struct flow_wildcards *mask)
{
@@ -251,12 +129,15 @@ test_miniflow(struct flow *flow)
/* Check that masked matches work as expected for identical flows and
* miniflows. */
-do {
-next_random_flow(, 1);
-} while (flow_wildcards_is_catchall());
+flow_wildcards_init_for_packet(, flow);
+/* Ensure that mask is not catchall just in case
+ * flow_wildcards_init_for_packet returns a catchall mask
+ */
+uint64_t *mask_u64 = (uint64_t *)
+mask_u64[0] = 1;
+ovs_assert(!flow_wildcards_is_catchall());
minimask = minimask_create();
-ovs_assert(minimask_is_catchall(minimask)
- == flow_wildcards_is_catchall());
+ovs_assert(!minimask_is_catchall(minimask));
ovs_assert(miniflow_equal_in_minimask(miniflow, miniflow2, minimask));
ovs_assert(miniflow_equal_flow_in_minimask(miniflow, , minimask));
ovs_assert(miniflow_hash_in_minimask(miniflow, minimask, 0x12345678) ==
@@ -325,7 +206,7 @@ test_minimask_combine(struct flow *flow)
struct minimask minicombined;
Re: [ovs-dev] [PATCH 0/5] Backport upstream Linux kernel patches
On 10/18/2019 7:23 AM, Simon Horman wrote: On Thu, Oct 17, 2019 at 01:32:27PM -0700, Gregory Rose wrote: Hello maintainers, Yi-Hung has added his acks. Please consider applying these patches. Thanks, - Greg Thanks Greg, applied to master. Thanks Simon! ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH ovn] ovn.at: Fix vtep autotest.
On Thu, Oct 17, 2019 at 03:51:42PM +0200, Dumitru Ceara wrote:
> With the removal of the OVS subtree the vtep autotest stopped working
> because the vtep.ovsschema couldn't be found anymore. The failure was
> however hidden by the "|| return 1" when failing to create the vtep DB.
>
> Fix the path to the vtep schema and make potential failures visible.
>
> CC: Numan Siddique
> Fixes: 84bf7d8435b8 ("Remove ovs subtree")
> Signed-off-by: Dumitru Ceara
I wonder how the "|| return 1" got there in the first place.
Acked-by: Ben Pfaff
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] Remove oss-fuzz tests carried over from ovs
On Fri, Oct 18, 2019 at 11:13:08AM +0200, bsh...@gmail.com wrote: > From: Bhargava Shastry > > It appears that ossfuzz specific test harnesses and configuration files > were carried over to the ovn repo from the ovs repo without > justification. This patch removes them until there is a need to > continuously fuzz ovn code as the ovs code is currently fuzzed. > > Signed-off-by: Bhargava Shastry Thanks a lot! I applied this to ovn master. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [RFC ovn PATCH 3/5] Remove multithreading from pinctrl.
This more-or-less gets pinctrl back to how it was prior to being made multithreaded for OVS 2.11. Since pinctrl is in its own process, there's no immediate need to have multiple threads. Signed-off-by: Mark Michelson --- controller/pinctrl.c | 736 +-- 1 file changed, 190 insertions(+), 546 deletions(-) diff --git a/controller/pinctrl.c b/controller/pinctrl.c index d826da186..edf6ccfb0 100644 --- a/controller/pinctrl.c +++ b/controller/pinctrl.c @@ -60,199 +60,84 @@ VLOG_DEFINE_THIS_MODULE(pinctrl); -/* pinctrl module creates a thread - pinctrl_handler to handle - * the packet-ins from ovs-vswitchd. Some of the OVN actions - * are translated to OF 'controller' actions. See include/ovn/actions.h - * for more details. - * - * pinctrl_handler thread doesn't access the Southbound IDL object. But - * some of the OVN actions which gets translated to 'controller' - * OF action, require data from Southbound DB. Below are the details - * on how these actions are implemented. - * - * pinctrl_run() function is called by ovn-controller main thread. - * A Mutex - 'pinctrl_mutex' is used between the pinctrl_handler() thread - * and pinctrl_run(). - * - * - dns_lookup - In order to do a DNS lookup, this action needs - * to access the 'DNS' table. pinctrl_run() builds a - * local DNS cache - 'dns_cache'. See sync_dns_cache() - * for more details. - * The function 'pinctrl_handle_dns_lookup()' (which is - * called with in the pinctrl_handler thread) looks into - * the local DNS cache to resolve the DNS requests. - * - * - put_arp/put_nd - These actions stores the IPv4/IPv6 and MAC addresses - * in the 'MAC_Binding' table. - * The function 'pinctrl_handle_put_mac_binding()' (which - * is called with in the pinctrl_handler thread), stores - * the IPv4/IPv6 and MAC addresses in the - * hmap - put_mac_bindings. - * - * pinctrl_run(), reads these mac bindings from the hmap - * 'put_mac_bindings' and writes to the 'MAC_Binding' - * table in the Southbound DB. - * - * - arp/nd_ns - These actions generate an ARP/IPv6 Neighbor solicit - * requests. The original packets are buffered and - * injected back when put_arp/put_nd resolves - * corresponding ARP/IPv6 Neighbor solicit requests. - * When pinctrl_run(), writes the mac bindings from the - * 'put_mac_bindings' hmap to the MAC_Binding table in - * SB DB, run_buffered_binding will add the buffered - * packets to buffered_mac_bindings and notify - * pinctrl_handler. - * - * The pinctrl_handler thread calls the function - - * send_mac_binding_buffered_pkts(), which uses - * the hmap - 'buffered_mac_bindings' and reinjects the - * buffered packets. - * - *- igmp - This action punts an IGMP packet to the controller - * which maintains multicast group information. The - * multicast groups (mcast_snoop_map) are synced to - * the 'IGMP_Group' table by ip_mcast_sync(). - * ip_mcast_sync() also reads the 'IP_Multicast' - * (snooping and querier) configuration and builds a - * local configuration mcast_cfg_map. - * ip_mcast_snoop_run() which runs in the - * pinctrl_handler() thread configures the per datapath - * mcast_snoop_map entries according to mcast_cfg_map. - * - * pinctrl module also periodically sends IPv6 Router Solicitation requests - * and gARPs (for the router gateway IPs and configured NAT addresses). - * - * IPv6 RA handling - pinctrl_run() prepares the IPv6 RA information - *(see prepare_ipv6_ras()) in the shash 'ipv6_ras' by - *looking into the Southbound DB table - Port_Binding. - * - *pinctrl_handler thread sends the periodic IPv6 RAs using - *the shash - 'ipv6_ras' - * - * g/rARP handling- pinctrl_run() prepares the g/rARP information - * (see send_garp_rarp_prepare()) in the shash - * 'send_garp_rarp_data' by looking into the - * Southbound DB table Port_Binding. - * pinctrl_handler() thread sends these gARPs using the - * shash 'send_garp_rarp_data'. - * - * IGMP Queries - pinctrl_run() prepares the IGMP queries (at most one - *per local datapath) based on
[ovs-dev] [RFC ovn PATCH 5/5] Flesh out manpage with more details about ovn-pinctrl
Signed-off-by: Mark Michelson --- pinctrl/ovn-pinctrl.8.xml | 77 ++- 1 file changed, 76 insertions(+), 1 deletion(-) diff --git a/pinctrl/ovn-pinctrl.8.xml b/pinctrl/ovn-pinctrl.8.xml index c0322cd4b..df8e717ca 100644 --- a/pinctrl/ovn-pinctrl.8.xml +++ b/pinctrl/ovn-pinctrl.8.xml @@ -8,7 +8,82 @@ Description -stub +ovn-pinctrl is a supplement to ovn-controller> +whose main duty is to handle packets bound for the controller. It +handles a variety of application protocols on behalf of the logical +network. The list of supported protocols can be found below. + + +DHCP + +If DHCP options have been configured for a logical switch, then +ovn-pinctrl will handle incoming DHCP packets and respond +to them with the configured address values. For more information on +possible DHCP settings, see ovn-nb(5). + + +DNS + +If DNS records have been configured in the northbound database, then +ovn-pinctrl will respond to DNS queries that pertain to +the configured domains. For more information on DNS configuration, see +ovn-nb(5). + + +ARP(IPv4) and Neighbor Discovery(IPv6) + +ovn-pinctrl can respond to incoming ARP and neighbor +solicitation requests based on learned MAC bindings. + + +IGMP + +If IGMP has been configured in the northbound database, then +ovn-pinctrl will handle incoming IGMP membership +reports. It will use the data in the IGMP messages to set data +in the southbound database that will alter the destinations of +multicast traffic. For more information on IGMP settings, see +ovn-nb(5). For more information regarding the ways +that IGMP traffic affects the resulting flows, see +ovn-architecture(7). + + +Other pinctrl operations + +In addition to the higher-level protocols listed above, +ovn-pinctrl also has other duties. + + + +It can send ICMP responses to certain incoming packet types. + + + + +It can send TCP resets based on reject ACLs. + + + + +It can send messages to the logs based on configured ACLs. + + + + +It can create Controller_Events in certain circumstances. + + + + +It can send periodic router advertisements if configured. + + + + +It can bind switch ports of type virtual. + + + Options -- 2.14.5 ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [RFC ovn PATCH 4/5] Move ovn-pinctrl to its own directory.
Signed-off-by: Mark Michelson
---
Makefile.am | 1 +
controller/automake.mk| 34 --
pinctrl/automake.mk | 25 +
pinctrl/ovn-pinctrl.8.xml | 35 +++
{controller => pinctrl}/ovn-pinctrl.c | 0
{controller => pinctrl}/pinctrl.c | 12 +---
{controller => pinctrl}/pinctrl.h | 0
tests/automake.mk | 2 +-
8 files changed, 67 insertions(+), 42 deletions(-)
create mode 100644 pinctrl/automake.mk
create mode 100644 pinctrl/ovn-pinctrl.8.xml
rename {controller => pinctrl}/ovn-pinctrl.c (100%)
rename {controller => pinctrl}/pinctrl.c (99%)
rename {controller => pinctrl}/pinctrl.h (100%)
diff --git a/Makefile.am b/Makefile.am
index 33c18c5d8..47dd0c164 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -497,6 +497,7 @@ include xenserver/automake.mk
include python/automake.mk
include tutorial/automake.mk
include selinux/automake.mk
+include pinctrl/automake.mk
include controller/automake.mk
include controller-vtep/automake.mk
include northd/automake.mk
diff --git a/controller/automake.mk b/controller/automake.mk
index d38ee927c..26f0c86d4 100644
--- a/controller/automake.mk
+++ b/controller/automake.mk
@@ -1,37 +1,3 @@
-bin_PROGRAMS += controller/ovn-pinctrl
-controller_ovn_pinctrl_SOURCES = \
-controller/ovn-pinctrl.c \
-controller/pinctrl.c \
- controller/pinctrl.h \
- controller/bfd.c \
- controller/bfd.h \
- controller/binding.c \
- controller/binding.h \
- controller/chassis.c \
- controller/chassis.h \
- controller/encaps.c \
- controller/encaps.h \
- controller/ha-chassis.c \
- controller/ha-chassis.h \
- controller/ip-mcast.c \
- controller/ip-mcast.h \
- controller/lflow.c \
- controller/lflow.h \
- controller/lport.c \
- controller/lport.h \
- controller/ofctrl.c \
- controller/ofctrl.h \
- controller/patch.c \
- controller/patch.h \
- controller/controller-utils.c \
- controller/ovn-controller.h \
- controller/physical.c \
- controller/physical.h
-controller_ovn_pinctrl_LDADD = lib/libovn.la $(OVS_LIBDIR)/libopenvswitch.la
-man_MANS += controller/ovn-pinctrl.8
-EXTRA_DIST += controller/ovn-pinctrl.8.xml
-CLEANFILES += controller/ovn-pinctrl.8
-
bin_PROGRAMS += controller/ovn-controller
controller_ovn_controller_SOURCES = \
controller/bfd.c \
diff --git a/pinctrl/automake.mk b/pinctrl/automake.mk
new file mode 100644
index 0..e240d9b1f
--- /dev/null
+++ b/pinctrl/automake.mk
@@ -0,0 +1,25 @@
+bin_PROGRAMS += pinctrl/ovn-pinctrl
+pinctrl_ovn_pinctrl_SOURCES = \
+pinctrl/ovn-pinctrl.c \
+pinctrl/pinctrl.c \
+ pinctrl/pinctrl.h \
+ controller/bfd.c \
+ controller/bfd.h \
+ controller/binding.c \
+ controller/binding.h \
+ controller/encaps.c \
+ controller/encaps.h \
+ controller/ha-chassis.c \
+ controller/ha-chassis.h \
+ controller/ip-mcast.c \
+ controller/ip-mcast.h \
+ controller/lport.c \
+ controller/lport.h \
+ controller/patch.c \
+ controller/patch.h \
+ controller/controller-utils.c \
+ controller/ovn-controller.h
+pinctrl_ovn_pinctrl_LDADD = lib/libovn.la $(OVS_LIBDIR)/libopenvswitch.la
+man_MANS += pinctrl/ovn-pinctrl.8
+EXTRA_DIST += pinctrl/ovn-pinctrl.8.xml
+CLEANFILES += pinctrl/ovn-pinctrl.8
diff --git a/pinctrl/ovn-pinctrl.8.xml b/pinctrl/ovn-pinctrl.8.xml
new file mode 100644
index 0..c0322cd4b
--- /dev/null
+++ b/pinctrl/ovn-pinctrl.8.xml
@@ -0,0 +1,35 @@
+
+
+Name
+ovn-pinctrl -- Open Virtual Network packet-in controller
+
+Synopsis
+ovn-pinctrl [options]
[ovs-database]
+
+Description
+
+stub
+
+
+Options
+
+Daemon Options
+http://www.w3.org/2003/XInclude"/>
+
+Logging Options
+http://www.w3.org/2003/XInclude"/>
+
+PKI Options
+
+ PKI configuration is required in order to use SSL for the connections to
+ the Northbound and Southbound databases.
+
+http://www.w3.org/2003/XInclude"/>
+http://www.w3.org/2003/XInclude"/>
+http://www.w3.org/2003/XInclude"/>
+
+Other Options
+
+http://www.w3.org/2003/XInclude"/>
+
+
diff --git a/controller/ovn-pinctrl.c b/pinctrl/ovn-pinctrl.c
similarity index 100%
rename from controller/ovn-pinctrl.c
rename to pinctrl/ovn-pinctrl.c
diff --git a/controller/pinctrl.c b/pinctrl/pinctrl.c
similarity index 99%
rename from controller/pinctrl.c
rename to pinctrl/pinctrl.c
index edf6ccfb0..927ae0229 100644
--- a/controller/pinctrl.c
+++ b/pinctrl/pinctrl.c
@@ -22,12 +22,13 @@
#include "csum.h"
#include "dirs.h"
#include "dp-packet.h"
-#include "encaps.h"
#include "flow.h"
-#include "ha-chassis.h"
-#include "lport.h"
#include "nx-match.h"
-#include "ovn-controller.h"
Re: [ovs-dev] [PATCH v2 00/12] Backport upstream conntrack related patches
On Tue, Oct 15, 2019 at 10:27:41AM -0700, Yi-Hung Wei wrote:
> This series backports conntrack related patches from upstream kernel.
> It has been tested on
> * Ubuntu 14.04, 16.04, and 18.04.
> * RHEL 7.4-7.6.
> * 5.0 kernel.
>
> Travis test: https://travis-ci.org/YiHungWei/ovs/builds/596361660
>
> v1->v2:
> - Fix commit message in one patch as noted by Yifeng.
> - Add one more upstream patch
> ("datapath: Fix linking without CONFIG_NF_CONNTRACK_LABELS")
Thanks for doing all this work. Backporting takes time and I appreciate
it.
I applied this series to master.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH ovn] ovn.at: Fix vtep autotest.
On Fri, Oct 18, 2019 at 10:57 PM Ben Pfaff wrote:
> On Thu, Oct 17, 2019 at 03:51:42PM +0200, Dumitru Ceara wrote:
> > With the removal of the OVS subtree the vtep autotest stopped working
> > because the vtep.ovsschema couldn't be found anymore. The failure was
> > however hidden by the "|| return 1" when failing to create the vtep DB.
> >
> > Fix the path to the vtep schema and make potential failures visible.
> >
> > CC: Numan Siddique
> > Fixes: 84bf7d8435b8 ("Remove ovs subtree")
> > Signed-off-by: Dumitru Ceara
>
> I wonder how the "|| return 1" got there in the first place.
>
> Acked-by: Ben Pfaff
>
Thanks. I applied this patch to master.
Numan
> ___
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
--
numan
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH net-next v4 08/10] net: openvswitch: fix possible memleak on destroy flow-table
On Thu, Oct 17, 2019 at 8:16 PM Tonghao Zhang wrote:
>
> On Fri, Oct 18, 2019 at 6:38 AM Pravin Shelar wrote:
> >
> > On Wed, Oct 16, 2019 at 5:50 AM wrote:
> > >
> > > From: Tonghao Zhang
> > >
> > > When we destroy the flow tables which may contain the flow_mask,
> > > so release the flow mask struct.
> > >
> > > Signed-off-by: Tonghao Zhang
> > > Tested-by: Greg Rose
> > > ---
> > > net/openvswitch/flow_table.c | 14 +-
> > > 1 file changed, 13 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/net/openvswitch/flow_table.c b/net/openvswitch/flow_table.c
> > > index 5df5182..d5d768e 100644
> > > --- a/net/openvswitch/flow_table.c
> > > +++ b/net/openvswitch/flow_table.c
> > > @@ -295,6 +295,18 @@ static void table_instance_destroy(struct
> > > table_instance *ti,
> > > }
> > > }
> > >
> > > +static void tbl_mask_array_destroy(struct flow_table *tbl)
> > > +{
> > > + struct mask_array *ma = ovsl_dereference(tbl->mask_array);
> > > + int i;
> > > +
> > > + /* Free the flow-mask and kfree_rcu the NULL is allowed. */
> > > + for (i = 0; i < ma->max; i++)
> > > + kfree_rcu(rcu_dereference_raw(ma->masks[i]), rcu);
> > > +
> > > + kfree_rcu(rcu_dereference_raw(tbl->mask_array), rcu);
> > > +}
> > > +
> > > /* No need for locking this function is called from RCU callback or
> > > * error path.
> > > */
> > > @@ -304,7 +316,7 @@ void ovs_flow_tbl_destroy(struct flow_table *table)
> > > struct table_instance *ufid_ti =
> > > rcu_dereference_raw(table->ufid_ti);
> > >
> > > free_percpu(table->mask_cache);
> > > - kfree_rcu(rcu_dereference_raw(table->mask_array), rcu);
> > > + tbl_mask_array_destroy(table);
> > > table_instance_destroy(ti, ufid_ti, false);
> > > }
> >
> > This should not be required. mask is linked to a flow and gets
> > released when flow is removed.
> > Does the memory leak occur when OVS module is abruptly unloaded and
> > userspace does not cleanup flow table?
> When we destroy the ovs datapath or net namespace is destroyed , the
> mask memory will be happened. The call tree:
> ovs_exit_net/ovs_dp_cmd_del
> -->__dp_destroy
> -->destroy_dp_rcu
> -->ovs_flow_tbl_destroy
> -->table_instance_destroy (which don't release the mask memory because
> don't call the ovs_flow_tbl_remove /flow_mask_remove directly or
> indirectly).
>
Thats what I suggested earlier, we need to call function similar to
ovs_flow_tbl_remove(), we could refactor code to use the code.
This is better since by introducing tbl_mask_array_destroy() is
creating a dangling pointer to mask in sw-flow object. OVS is anyway
iterating entire flow table to release sw-flow in
table_instance_destroy(), it is natural to release mask at that point
after releasing corresponding sw-flow.
> but one thing, when we flush the flow, we don't flush the mask flow.(
> If necessary, one patch should be sent)
>
> > In that case better fix could be calling ovs_flow_tbl_remove()
> > equivalent from table_instance_destroy when it is iterating flow
> > table.
> I think operation of the flow mask and flow table should use
> different API, for example:
> for flow mask, we use the:
> -tbl_mask_array_add_mask
> -tbl_mask_array_del_mask
> -tbl_mask_array_alloc
> -tbl_mask_array_realloc
> -tbl_mask_array_destroy(this patch introduce.)
>
> table instance:
> -table_instance_alloc
> -table_instance_destroy
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] Remove oss-fuzz tests carried over from ovs
This is the right mailing list for now. The convention is to write [PATCH ovn] in subject headings (but I figured it out ;-) On Fri, Oct 18, 2019 at 04:48:40PM +0200, Bhargava Shastry wrote: > Just want to add that the patch is to be applied to ovn source code, I > couldn't find the ovn mailing list so sent it here. Apologies. > > On Fri, Oct 18, 2019 at 11:13 AM wrote: > > > From: Bhargava Shastry > > > > It appears that ossfuzz specific test harnesses and configuration files > > were carried over to the ovn repo from the ovs repo without > > justification. This patch removes them until there is a need to > > continuously fuzz ovn code as the ovs code is currently fuzzed. > > > > Signed-off-by: Bhargava Shastry > > --- > > tests/automake.mk | 2 - > > tests/oss-fuzz/automake.mk| 66 --- > > tests/oss-fuzz/config/expr.dict | 120 - > > .../oss-fuzz/config/expr_parse_target.options | 3 - > > .../config/flow_extract_target.options| 3 - > > .../config/json_parser_target.options | 2 - > > tests/oss-fuzz/config/miniflow_target.options | 3 - > > tests/oss-fuzz/config/odp.dict| 170 --- > > tests/oss-fuzz/config/odp_target.options | 3 - > > .../config/ofctl_parse_target.options | 3 - > > tests/oss-fuzz/config/ofp-flow.dict | 45 -- > > .../oss-fuzz/config/ofp_print_target.options | 3 - > > tests/oss-fuzz/config/ovs.dict| 293 --- > > tests/oss-fuzz/expr_parse_target.c| 464 -- > > tests/oss-fuzz/flow_extract_target.c | 100 > > tests/oss-fuzz/fuzzer.h | 9 - > > tests/oss-fuzz/json_parser_target.c | 42 -- > > tests/oss-fuzz/miniflow_target.c | 365 -- > > tests/oss-fuzz/odp_target.c | 149 -- > > tests/oss-fuzz/ofctl_parse_target.c | 113 - > > tests/oss-fuzz/ofp_print_target.c | 47 -- > > 21 files changed, 2005 deletions(-) > > delete mode 100644 tests/oss-fuzz/automake.mk > > delete mode 100644 tests/oss-fuzz/config/expr.dict > > delete mode 100644 tests/oss-fuzz/config/expr_parse_target.options > > delete mode 100644 tests/oss-fuzz/config/flow_extract_target.options > > delete mode 100644 tests/oss-fuzz/config/json_parser_target.options > > delete mode 100644 tests/oss-fuzz/config/miniflow_target.options > > delete mode 100644 tests/oss-fuzz/config/odp.dict > > delete mode 100644 tests/oss-fuzz/config/odp_target.options > > delete mode 100644 tests/oss-fuzz/config/ofctl_parse_target.options > > delete mode 100644 tests/oss-fuzz/config/ofp-flow.dict > > delete mode 100644 tests/oss-fuzz/config/ofp_print_target.options > > delete mode 100644 tests/oss-fuzz/config/ovs.dict > > delete mode 100644 tests/oss-fuzz/expr_parse_target.c > > delete mode 100644 tests/oss-fuzz/flow_extract_target.c > > delete mode 100644 tests/oss-fuzz/fuzzer.h > > delete mode 100644 tests/oss-fuzz/json_parser_target.c > > delete mode 100644 tests/oss-fuzz/miniflow_target.c > > delete mode 100644 tests/oss-fuzz/odp_target.c > > delete mode 100644 tests/oss-fuzz/ofctl_parse_target.c > > delete mode 100644 tests/oss-fuzz/ofp_print_target.c > > > > diff --git a/tests/automake.mk b/tests/automake.mk > > index 013e59280..e86a5273e 100644 > > --- a/tests/automake.mk > > +++ b/tests/automake.mk > > @@ -246,5 +246,3 @@ clean-pki: > > rm -f tests/pki/stamp > > rm -rf tests/pki > > endif > > - > > -include tests/oss-fuzz/automake.mk > > diff --git a/tests/oss-fuzz/automake.mk b/tests/oss-fuzz/automake.mk > > deleted file mode 100644 > > index 5bf7d0d7c..0 > > --- a/tests/oss-fuzz/automake.mk > > +++ /dev/null > > @@ -1,66 +0,0 @@ > > -OSS_FUZZ_TARGETS = \ > > - tests/oss-fuzz/flow_extract_target \ > > - tests/oss-fuzz/json_parser_target \ > > - tests/oss-fuzz/ofp_print_target \ > > - tests/oss-fuzz/expr_parse_target \ > > - tests/oss-fuzz/odp_target \ > > - tests/oss-fuzz/miniflow_target \ > > - tests/oss-fuzz/ofctl_parse_target > > -EXTRA_PROGRAMS += $(OSS_FUZZ_TARGETS) > > -oss-fuzz-targets: $(OSS_FUZZ_TARGETS) > > - > > -tests_oss_fuzz_flow_extract_target_SOURCES = \ > > - tests/oss-fuzz/flow_extract_target.c \ > > - tests/oss-fuzz/fuzzer.h > > -tests_oss_fuzz_flow_extract_target_LDADD = lib/libopenvswitch.la > > -tests_oss_fuzz_flow_extract_target_LDFLAGS = $(LIB_FUZZING_ENGINE) -lc++ > > - > > -tests_oss_fuzz_json_parser_target_SOURCES = \ > > - tests/oss-fuzz/json_parser_target.c \ > > - tests/oss-fuzz/fuzzer.h > > -tests_oss_fuzz_json_parser_target_LDADD = lib/libopenvswitch.la > > -tests_oss_fuzz_json_parser_target_LDFLAGS = $(LIB_FUZZING_ENGINE) -lc++ > > - > > -tests_oss_fuzz_ofp_print_target_SOURCES = \ > > - tests/oss-fuzz/ofp_print_target.c \ > > - tests/oss-fuzz/fuzzer.h > >
Re: [ovs-dev] [PATCH ovn] Add RDNSS support to OVN
Acked-by: Mark Michelson
On 10/18/19 7:09 AM, Lorenzo Bianconi wrote:
Introduce the possibility to specify a RDNSS option to Router
Advertisement packets. DNS IPv6 address can be specified using
'rdnss' tag in the ipv6_ra_configs column of logical router
port table
Signed-off-by: Lorenzo Bianconi
---
controller/pinctrl.c | 50
northd/ovn-northd.c | 5 +
ovn-nb.xml | 4
tests/ovn.at | 27 +---
4 files changed, 78 insertions(+), 8 deletions(-)
diff --git a/controller/pinctrl.c b/controller/pinctrl.c
index d826da186..5bc4b35e7 100644
--- a/controller/pinctrl.c
+++ b/controller/pinctrl.c
@@ -2192,6 +2192,8 @@ struct ipv6_ra_config {
uint8_t mo_flags; /* Managed/Other flags for RAs */
uint8_t la_flags; /* On-link/autonomous flags for address prefixes */
struct lport_addresses prefixes;
+struct in6_addr rdnss;
+bool has_rdnss;
};
struct ipv6_ra_state {
@@ -2202,6 +2204,16 @@ struct ipv6_ra_state {
bool delete_me;
};
+#define ND_RDNSS_OPT_LEN8
+#define ND_OPT_RDNSS25
+struct nd_rdnss_opt {
+uint8_t type; /* ND_OPT_RDNSS. */
+uint8_t len; /* >= 3. */
+ovs_be16 reserved;/* Always 0. */
+ovs_be32 lifetime;
+const ovs_be128 dns[0];
+};
+
static void
init_ipv6_ras(void)
{
@@ -2289,6 +2301,12 @@ ipv6_ra_update_config(const struct sbrec_port_binding
*pb)
VLOG_WARN("Invalid IP source %s", ip_addr);
goto fail;
}
+const char *rdnss = smap_get(>options, "ipv6_ra_rdnss");
+if (rdnss && !ipv6_parse(rdnss, >rdnss)) {
+VLOG_WARN("Invalid RDNSS source %s", rdnss);
+goto fail;
+}
+config->has_rdnss = !!rdnss;
return config;
@@ -2319,6 +2337,35 @@ put_load(uint64_t value, enum mf_field_id dst, int ofs, int n_bits,
bitwise_one(ofpact_set_field_mask(sf), sf->field->n_bytes, ofs, n_bits);
}
+static void
+packet_put_ra_rdnss_opt(struct dp_packet *b, uint8_t num,
+ovs_be32 lifetime, const struct in6_addr *dns)
+{
+size_t prev_l4_size = dp_packet_l4_size(b);
+struct ip6_hdr *nh = dp_packet_l3(b);
+size_t len = 2 * num + 1;
+size_t size = len * 8;
+
+nh->ip6_plen = htons(prev_l4_size + len * 8);
+
+struct nd_rdnss_opt *nd_rdnss = dp_packet_put_uninit(b, size);
+nd_rdnss->type = ND_OPT_RDNSS;
+nd_rdnss->len = len;
+nd_rdnss->reserved = 0;
+nd_rdnss->lifetime = lifetime;
+
+ovs_be128 *addr = (ovs_be128 *)(nd_rdnss + 1);
+for (int i = 0; i < num; i++) {
+memcpy(addr + i, dns, sizeof(ovs_be32[4]));
+}
+
+struct ovs_ra_msg *ra = dp_packet_l4(b);
+ra->icmph.icmp6_cksum = 0;
+uint32_t icmp_csum = packet_csum_pseudoheader6(dp_packet_l3(b));
+ra->icmph.icmp6_cksum = csum_finish(csum_continue(icmp_csum, ra,
+ prev_l4_size + size));
+}
+
/* Called with in the pinctrl_handler thread context. */
static long long int
ipv6_ra_send(struct rconn *swconn, struct ipv6_ra_state *ra)
@@ -2343,6 +2390,9 @@ ipv6_ra_send(struct rconn *swconn, struct ipv6_ra_state
*ra)
ra->config->la_flags, htonl(IPV6_ND_RA_OPT_PREFIX_VALID_LIFETIME),
htonl(IPV6_ND_RA_OPT_PREFIX_PREFERRED_LIFETIME), addr);
}
+if (ra->config->has_rdnss) {
+packet_put_ra_rdnss_opt(, 1, 0x, >config->rdnss);
+}
uint64_t ofpacts_stub[4096 / 8];
struct ofpbuf ofpacts = OFPBUF_STUB_INITIALIZER(ofpacts_stub);
diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
index ea8ad7c2d..d1de36e08 100644
--- a/northd/ovn-northd.c
+++ b/northd/ovn-northd.c
@@ -6485,6 +6485,11 @@ copy_ra_to_sb(struct ovn_port *op, const char
*address_mode)
smap_add(, "ipv6_ra_prefixes", ds_cstr());
ds_destroy();
+const char *rdnss = smap_get(>nbrp->ipv6_ra_configs, "rdnss");
+if (rdnss) {
+smap_add(, "ipv6_ra_rdnss", rdnss);
+}
+
smap_add(, "ipv6_ra_src_eth", op->lrp_networks.ea_s);
sbrec_port_binding_set_options(op->sb, );
diff --git a/ovn-nb.xml b/ovn-nb.xml
index 1504f8fca..f1f9b969e 100644
--- a/ovn-nb.xml
+++ b/ovn-nb.xml
@@ -1885,6 +1885,10 @@
is one-third of ,
i.e. 200 seconds if that key is unset.
+
+
+IPv6 address of RDNSS server announced in RA packets
+
diff --git a/tests/ovn.at b/tests/ovn.at
index 6bdb9e8ed..80d38ea4c 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -12537,14 +12537,15 @@ construct_expected_ra() {
local mtu=$1
local ra_mo=$2
-local ra_prefix_la=$3
+local rdnss=$3
+local ra_prefix_la=$4
local slla=0101${src_mac}
local mtu_opt=""
if test $mtu != 0; then
mtu_opt=0501${mtu}
fi
-shift 3
+shift 4
local prefix=""
while [[ $# -gt 0 ]] ; do
@@ -12554,7 +12555,12 @@
Re: [ovs-dev] [PATCH 00/11] Backport upstream conntrack related patches
On Tue, Oct 15, 2019 at 10:41:23AM -0700, Yi-Hung Wei wrote: > On Tue, Oct 15, 2019 at 10:18 AM Ben Pfaff wrote: > > > > On Mon, Oct 14, 2019 at 10:37:40AM -0700, Yi-Hung Wei wrote: > > > This series backports conntrack related patches from upstream kernel. > > > It has been tested on > > > * Ubuntu 14.04, 16.04, and 18.04. > > > * RHEL 7.4-7.6. > > > * 5.0 kernel. > > > > Yi-Hung, I think that you've received acks on all of these. If so, > > please add the acks and re-post the series. Thank you! > > Hi Ben, > > I just send out v2 with the added acks. Thanks! ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] ossfuzz: Simplify miniflow fuzzer harness.
On Fri, Oct 18, 2019 at 05:17:34PM +0200, bsh...@gmail.com wrote: > From: Bhargava Shastry > > Google's oss-fuzz builder bots were complaining that miniflow_target is > too slow to fuzz in that some tests take longer than a second to > complete. This patch fixes this by replacing the random flow generation > within the harness to a more simpler scenario. > > Signed-off-by: Bhargava Shastry Thank you! I applied this to master. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [RFC ovn PATCH 1/5] Separate pinctrl to its own process.
This is a minimum effort separation. The pinctrl.c file has not been
altered. A new ovn-pinctrl.c file starts an application that makes calls
to pinctrl as necessary.
To facilitate this change, a couple of extra modifications were made.
* binding_run() now has a do_bind boolean parameter. With this set true,
binding_run() will actually make changes in the southbound database
regarding bindings (e.g. claiming a particular port binding for a
chassis).
* Some functions common to both ovn-controller and ovn-pinctrl have been
moved from ovn-controller into a controller-utils.c file.
Signed-off-by: Mark Michelson
---
controller/automake.mk| 37 ++-
controller/binding.c | 22 +-
controller/binding.h | 3 +-
controller/controller-utils.c | 54
controller/ovn-controller.c | 62 +
controller/ovn-pinctrl.c | 590 ++
tests/ofproto-macros.at | 3 +
tests/ovn.at | 13 +-
tutorial/ovs-sandbox | 5 +
utilities/ovn-ctl | 40 +++
10 files changed, 754 insertions(+), 75 deletions(-)
create mode 100644 controller/controller-utils.c
create mode 100644 controller/ovn-pinctrl.c
diff --git a/controller/automake.mk b/controller/automake.mk
index 45e1bdd36..d38ee927c 100644
--- a/controller/automake.mk
+++ b/controller/automake.mk
@@ -1,3 +1,37 @@
+bin_PROGRAMS += controller/ovn-pinctrl
+controller_ovn_pinctrl_SOURCES = \
+controller/ovn-pinctrl.c \
+controller/pinctrl.c \
+ controller/pinctrl.h \
+ controller/bfd.c \
+ controller/bfd.h \
+ controller/binding.c \
+ controller/binding.h \
+ controller/chassis.c \
+ controller/chassis.h \
+ controller/encaps.c \
+ controller/encaps.h \
+ controller/ha-chassis.c \
+ controller/ha-chassis.h \
+ controller/ip-mcast.c \
+ controller/ip-mcast.h \
+ controller/lflow.c \
+ controller/lflow.h \
+ controller/lport.c \
+ controller/lport.h \
+ controller/ofctrl.c \
+ controller/ofctrl.h \
+ controller/patch.c \
+ controller/patch.h \
+ controller/controller-utils.c \
+ controller/ovn-controller.h \
+ controller/physical.c \
+ controller/physical.h
+controller_ovn_pinctrl_LDADD = lib/libovn.la $(OVS_LIBDIR)/libopenvswitch.la
+man_MANS += controller/ovn-pinctrl.8
+EXTRA_DIST += controller/ovn-pinctrl.8.xml
+CLEANFILES += controller/ovn-pinctrl.8
+
bin_PROGRAMS += controller/ovn-controller
controller_ovn_controller_SOURCES = \
controller/bfd.c \
@@ -18,11 +52,10 @@ controller_ovn_controller_SOURCES = \
controller/lport.h \
controller/ofctrl.c \
controller/ofctrl.h \
- controller/pinctrl.c \
- controller/pinctrl.h \
controller/patch.c \
controller/patch.h \
controller/ovn-controller.c \
+ controller/controller-utils.c \
controller/ovn-controller.h \
controller/physical.c \
controller/physical.h
diff --git a/controller/binding.c b/controller/binding.c
index aad9d39e6..ec97b13f6 100644
--- a/controller/binding.c
+++ b/controller/binding.c
@@ -485,7 +485,8 @@ consider_local_datapath(struct ovsdb_idl_txn *ovnsb_idl_txn,
struct hmap *local_datapaths,
struct shash *lport_to_iface,
struct sset *local_lports,
-struct sset *local_lport_ids)
+struct sset *local_lport_ids,
+bool do_bind)
{
const struct ovsrec_interface *iface_rec
= shash_find_data(lport_to_iface, binding_rec->logical_port);
@@ -554,6 +555,10 @@ consider_local_datapath(struct ovsdb_idl_txn
*ovnsb_idl_txn,
update_local_lport_ids(local_lport_ids, binding_rec);
}
+if (!do_bind) {
+return;
+}
+
ovs_assert(ovnsb_idl_txn);
if (ovnsb_idl_txn) {
const char *vif_chassis = smap_get(_rec->options,
@@ -588,6 +593,9 @@ consider_local_datapath(struct ovsdb_idl_txn *ovnsb_idl_txn,
} else if (binding_rec->chassis == chassis_rec) {
if (!strcmp(binding_rec->type, "virtual")) {
/* pinctrl module takes care of binding the ports
+ *
+ * struct hmap local_datapaths =
HMAP_INITIALIZER(_datapaths);
+ * struct sset
* of type 'virtual'.
* Release such ports if their virtual parents are no
* longer claimed by this chassis. */
@@ -700,7 +708,8 @@ binding_run(struct ovsdb_idl_txn *ovnsb_idl_txn,
const struct ovsrec_bridge_table *bridge_table,
const struct ovsrec_open_vswitch_table *ovs_table,
struct hmap *local_datapaths, struct sset *local_lports,
-struct sset *local_lport_ids)
+struct sset *local_lport_ids,
+bool do_bind)
{
if (!chassis_rec)
[ovs-dev] [RFC ovn PATCH 2/5] Resolve duplicate functions in ovn-controller and ovn-pinctrl.
Most duplicate functions have been extracted to controller-utils.c.
Signed-off-by: Mark Michelson
---
controller/controller-utils.c | 166 ++
controller/ovn-controller.c | 171 +--
controller/ovn-controller.h | 20 +
controller/ovn-pinctrl.c | 205 +++---
4 files changed, 203 insertions(+), 359 deletions(-)
diff --git a/controller/controller-utils.c b/controller/controller-utils.c
index 2ec4df1ac..a74600d0b 100644
--- a/controller/controller-utils.c
+++ b/controller/controller-utils.c
@@ -17,6 +17,12 @@
#include "ovn-controller.h"
#include "lib/vswitch-idl.h"
+#include "lib/stream.h"
+#include "lib/stream-ssl.h"
+#include "lib/sset.h"
+
+#define DEFAULT_BRIDGE_NAME "br-int"
+#define DEFAULT_PROBE_INTERVAL_MSEC 5000
struct local_datapath *
get_local_datapath(const struct hmap *local_datapaths, uint32_t tunnel_key)
@@ -52,3 +58,163 @@ get_bridge(const struct ovsrec_bridge_table *bridge_table,
const char *br_name)
}
return NULL;
}
+
+const char *
+br_int_name(const struct ovsrec_open_vswitch *cfg)
+{
+return smap_get_def(>external_ids, "ovn-bridge", DEFAULT_BRIDGE_NAME);
+}
+
+const struct ovsrec_bridge *
+get_br_int(const struct ovsrec_bridge_table *bridge_table,
+ const struct ovsrec_open_vswitch_table *ovs_table)
+{
+const struct ovsrec_open_vswitch *cfg;
+cfg = ovsrec_open_vswitch_table_first(ovs_table);
+if (!cfg) {
+return NULL;
+}
+
+return get_bridge(bridge_table, br_int_name(cfg));
+}
+
+/* Retrieves the pointer to the OVN Southbound database from 'ovs_idl' and
+ * updates 'sbdb_idl' with that pointer. */
+void
+update_sb_db(struct ovsdb_idl *ovs_idl, struct ovsdb_idl *ovnsb_idl)
+{
+const struct ovsrec_open_vswitch *cfg = ovsrec_open_vswitch_first(ovs_idl);
+
+/* Set remote based on user configuration. */
+const char *remote = NULL;
+if (cfg) {
+remote = smap_get(>external_ids, "ovn-remote");
+}
+ovsdb_idl_set_remote(ovnsb_idl, remote, true);
+
+/* Set probe interval, based on user configuration and the remote. */
+int default_interval = (remote && !stream_or_pstream_needs_probes(remote)
+? 0 : DEFAULT_PROBE_INTERVAL_MSEC);
+int interval = smap_get_int(>external_ids,
+"ovn-remote-probe-interval", default_interval);
+ovsdb_idl_set_probe_interval(ovnsb_idl, interval);
+}
+
+void
+update_ssl_config(const struct ovsrec_ssl_table *ssl_table)
+{
+const struct ovsrec_ssl *ssl = ovsrec_ssl_table_first(ssl_table);
+
+if (ssl) {
+stream_ssl_set_key_and_cert(ssl->private_key, ssl->certificate);
+stream_ssl_set_ca_cert_file(ssl->ca_cert, ssl->bootstrap_ca_cert);
+}
+}
+
+const char *
+get_ovs_chassis_id(const struct ovsrec_open_vswitch_table *ovs_table)
+{
+const struct ovsrec_open_vswitch *cfg
+= ovsrec_open_vswitch_table_first(ovs_table);
+return cfg ? smap_get(>external_ids, "system-id")
+ : NULL;
+}
+
+void
+update_sb_monitors(struct ovsdb_idl *ovnsb_idl,
+ const struct sbrec_chassis *chassis,
+ const struct sset *local_ifaces,
+ struct hmap *local_datapaths)
+{
+/* Monitor Port_Bindings rows for local interfaces and local datapaths.
+ *
+ * Monitor Logical_Flow, MAC_Binding, Multicast_Group, and DNS tables for
+ * local datapaths.
+ *
+ * Monitor Controller_Event rows for local chassis.
+ *
+ * Monitor IP_Multicast for local datapaths.
+ *
+ * Monitor IGMP_Groups for local chassis.
+ *
+ * We always monitor patch ports because they allow us to see the linkages
+ * between related logical datapaths. That way, when we know that we have
+ * a VIF on a particular logical switch, we immediately know to monitor all
+ * the connected logical routers and logical switches. */
+struct ovsdb_idl_condition pb = OVSDB_IDL_CONDITION_INIT();
+struct ovsdb_idl_condition lf = OVSDB_IDL_CONDITION_INIT();
+struct ovsdb_idl_condition mb = OVSDB_IDL_CONDITION_INIT();
+struct ovsdb_idl_condition mg = OVSDB_IDL_CONDITION_INIT();
+struct ovsdb_idl_condition dns = OVSDB_IDL_CONDITION_INIT();
+struct ovsdb_idl_condition ce = OVSDB_IDL_CONDITION_INIT();
+struct ovsdb_idl_condition ip_mcast = OVSDB_IDL_CONDITION_INIT(_mcast);
+struct ovsdb_idl_condition igmp = OVSDB_IDL_CONDITION_INIT();
+sbrec_port_binding_add_clause_type(, OVSDB_F_EQ, "patch");
+/* XXX: We can optimize this, if we find a way to only monitor
+ * ports that have a Gateway_Chassis that point's to our own
+ * chassis */
+sbrec_port_binding_add_clause_type(, OVSDB_F_EQ, "chassisredirect");
+sbrec_port_binding_add_clause_type(, OVSDB_F_EQ, "external");
+if (chassis) {
+/* This should be mostly redundant with the other clauses for
[ovs-dev] [RFC ovn PATCH 0/5] Separate pinctrl to its own process
This proposes a set of patches to move pinctrl operations out of the
ovn-controller process and into its own.
The main reasons for doing this are the following:
1) Separating pinctrl makes it so that receiving a packet-in can't wake
up ovn-controller.
2) Separating pinctrl allows for manipulating the southbound database
directly while handling packets in, thus minimizing the need for storing
local copies of data
3) This lays the groundwork for an easier eventual conversion of
ovn-controller to DDlog, since the DDlog code would need to only handle
flow creation, not packet in handling.
This is an RFC. With this set of changes, item (2) above is not well
addressed here. While the multithreading is removed from pinctrl, the
structural components have not been altered. Were this idea to be
approved, point (2) would be addressed when creating the final patch.
Please share your thoughts.
Mark Michelson (5):
Separate pinctrl to its own process.
Resolve duplicate functions in ovn-controller and ovn-pinctrl.
Remove multithreading from pinctrl.
Move ovn-pinctrl to its own directory.
Flesh out manpage with more details about ovn-pinctrl
Makefile.am | 1 +
controller/automake.mk| 3 +-
controller/binding.c | 22 +-
controller/binding.h | 3 +-
controller/controller-utils.c | 220 +++
controller/ovn-controller.c | 233 +---
controller/ovn-controller.h | 20 +
pinctrl/automake.mk | 25 ++
pinctrl/ovn-pinctrl.8.xml | 110 ++
pinctrl/ovn-pinctrl.c | 413 +
{controller => pinctrl}/pinctrl.c | 748 ++
{controller => pinctrl}/pinctrl.h | 0
tests/automake.mk | 2 +-
tests/ofproto-macros.at | 3 +
tests/ovn.at | 13 +-
tutorial/ovs-sandbox | 5 +
utilities/ovn-ctl | 40 ++
17 files changed, 1064 insertions(+), 797 deletions(-)
create mode 100644 controller/controller-utils.c
create mode 100644 pinctrl/automake.mk
create mode 100644 pinctrl/ovn-pinctrl.8.xml
create mode 100644 pinctrl/ovn-pinctrl.c
rename {controller => pinctrl}/pinctrl.c (84%)
rename {controller => pinctrl}/pinctrl.h (100%)
--
2.14.5
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH net-next v4 09/10] net: openvswitch: don't unlock mutex when changing the user_features fails
On Wed, Oct 16, 2019 at 5:56 AM wrote:
>
> From: Tonghao Zhang
>
> Unlocking of a not locked mutex is not allowed.
> Other kernel thread may be in critical section while
> we unlock it because of setting user_feature fail.
>
> Fixes: 95a7233c4 ("net: openvswitch: Set OvS recirc_id from tc chain index")
> Cc: Paul Blakey
> Signed-off-by: Tonghao Zhang
> Tested-by: Greg Rose
> ---
LGTM
Acked-by: William Tu
> net/openvswitch/datapath.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/net/openvswitch/datapath.c b/net/openvswitch/datapath.c
> index 9fea7e1..aeb76e4 100644
> --- a/net/openvswitch/datapath.c
> +++ b/net/openvswitch/datapath.c
> @@ -1657,6 +1657,7 @@ static int ovs_dp_cmd_new(struct sk_buff *skb, struct
> genl_info *info)
> ovs_dp_reset_user_features(skb, info);
> }
>
> + ovs_unlock();
> goto err_destroy_meters;
> }
>
> @@ -1673,7 +1674,6 @@ static int ovs_dp_cmd_new(struct sk_buff *skb, struct
> genl_info *info)
> return 0;
>
> err_destroy_meters:
> - ovs_unlock();
> ovs_meters_exit(dp);
> err_destroy_ports_array:
> kfree(dp->ports);
> --
> 1.8.3.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 net-next v4 01/10] net: openvswitch: add flow-mask cache for performance
On Wed, Oct 16, 2019 at 5:51 AM wrote: > > From: Tonghao Zhang > > The idea of this optimization comes from a patch which > is committed in 2014, openvswitch community. The author > is Pravin B Shelar. In order to get high performance, I > implement it again. Later patches will use it. > > Pravin B Shelar, says: > | On every packet OVS needs to lookup flow-table with every > | mask until it finds a match. The packet flow-key is first > | masked with mask in the list and then the masked key is > | looked up in flow-table. Therefore number of masks can > | affect packet processing performance. > > Link: > https://github.com/openvswitch/ovs/commit/5604935e4e1cbc16611d2d97f50b717aa31e8ec5 > Signed-off-by: Tonghao Zhang > Tested-by: Greg Rose > --- LGTM Acked-by: William Tu ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH net-next v4 07/10] net: openvswitch: add likely in flow_lookup
On Wed, Oct 16, 2019 at 5:55 AM wrote:
>
> From: Tonghao Zhang
>
> The most case *index < ma->max, and flow-mask is not NULL.
> We add un/likely for performance.
>
> Signed-off-by: Tonghao Zhang
> Tested-by: Greg Rose
> ---
LGTM
Acked-by: William Tu
> net/openvswitch/flow_table.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/net/openvswitch/flow_table.c b/net/openvswitch/flow_table.c
> index 3e3d345..5df5182 100644
> --- a/net/openvswitch/flow_table.c
> +++ b/net/openvswitch/flow_table.c
> @@ -518,7 +518,7 @@ static struct sw_flow *flow_lookup(struct flow_table *tbl,
> struct sw_flow_mask *mask;
> int i;
>
> - if (*index < ma->max) {
> + if (likely(*index < ma->max)) {
> mask = rcu_dereference_ovsl(ma->masks[*index]);
> if (mask) {
> flow = masked_flow_lookup(ti, key, mask, n_mask_hit);
> @@ -533,7 +533,7 @@ static struct sw_flow *flow_lookup(struct flow_table *tbl,
> continue;
>
> mask = rcu_dereference_ovsl(ma->masks[i]);
> - if (!mask)
> + if (unlikely(!mask))
> break;
>
> flow = masked_flow_lookup(ti, key, mask, n_mask_hit);
> --
> 1.8.3.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 net-next v4 10/10] net: openvswitch: simplify the ovs_dp_cmd_new
On Wed, Oct 16, 2019 at 5:56 AM wrote:
>
> From: Tonghao Zhang
>
> use the specified functions to init resource.
>
> Signed-off-by: Tonghao Zhang
> Tested-by: Greg Rose
> ---
Looks like this is simply moving code around.
I don't have any opinion.
> net/openvswitch/datapath.c | 60
> +-
> 1 file changed, 38 insertions(+), 22 deletions(-)
>
> diff --git a/net/openvswitch/datapath.c b/net/openvswitch/datapath.c
> index aeb76e4..4d48e48 100644
> --- a/net/openvswitch/datapath.c
> +++ b/net/openvswitch/datapath.c
> @@ -1576,6 +1576,31 @@ static int ovs_dp_change(struct datapath *dp, struct
> nlattr *a[])
> return 0;
> }
>
> +static int ovs_dp_stats_init(struct datapath *dp)
> +{
> + dp->stats_percpu = netdev_alloc_pcpu_stats(struct dp_stats_percpu);
> + if (!dp->stats_percpu)
> + return -ENOMEM;
> +
> + return 0;
> +}
> +
> +static int ovs_dp_vport_init(struct datapath *dp)
> +{
> + int i;
> +
> + dp->ports = kmalloc_array(DP_VPORT_HASH_BUCKETS,
> + sizeof(struct hlist_head),
> + GFP_KERNEL);
> + if (!dp->ports)
> + return -ENOMEM;
> +
> + for (i = 0; i < DP_VPORT_HASH_BUCKETS; i++)
> + INIT_HLIST_HEAD(>ports[i]);
> +
> + return 0;
> +}
> +
> static int ovs_dp_cmd_new(struct sk_buff *skb, struct genl_info *info)
> {
> struct nlattr **a = info->attrs;
> @@ -1584,7 +1609,7 @@ static int ovs_dp_cmd_new(struct sk_buff *skb, struct
> genl_info *info)
> struct datapath *dp;
> struct vport *vport;
> struct ovs_net *ovs_net;
> - int err, i;
> + int err;
>
> err = -EINVAL;
> if (!a[OVS_DP_ATTR_NAME] || !a[OVS_DP_ATTR_UPCALL_PID])
> @@ -1597,35 +1622,26 @@ static int ovs_dp_cmd_new(struct sk_buff *skb, struct
> genl_info *info)
> err = -ENOMEM;
> dp = kzalloc(sizeof(*dp), GFP_KERNEL);
> if (dp == NULL)
> - goto err_free_reply;
> + goto err_destroy_reply;
>
> ovs_dp_set_net(dp, sock_net(skb->sk));
>
> /* Allocate table. */
> err = ovs_flow_tbl_init(>table);
> if (err)
> - goto err_free_dp;
> + goto err_destroy_dp;
>
> - dp->stats_percpu = netdev_alloc_pcpu_stats(struct dp_stats_percpu);
> - if (!dp->stats_percpu) {
> - err = -ENOMEM;
> + err = ovs_dp_stats_init(dp);
> + if (err)
> goto err_destroy_table;
> - }
>
> - dp->ports = kmalloc_array(DP_VPORT_HASH_BUCKETS,
> - sizeof(struct hlist_head),
> - GFP_KERNEL);
> - if (!dp->ports) {
> - err = -ENOMEM;
> - goto err_destroy_percpu;
> - }
> -
> - for (i = 0; i < DP_VPORT_HASH_BUCKETS; i++)
> - INIT_HLIST_HEAD(>ports[i]);
> + err = ovs_dp_vport_init(dp);
> + if (err)
> + goto err_destroy_stats;
>
> err = ovs_meters_init(dp);
> if (err)
> - goto err_destroy_ports_array;
> + goto err_destroy_ports;
>
> /* Set up our datapath device. */
> parms.name = nla_data(a[OVS_DP_ATTR_NAME]);
> @@ -1675,15 +1691,15 @@ static int ovs_dp_cmd_new(struct sk_buff *skb, struct
> genl_info *info)
>
> err_destroy_meters:
> ovs_meters_exit(dp);
> -err_destroy_ports_array:
> +err_destroy_ports:
> kfree(dp->ports);
> -err_destroy_percpu:
> +err_destroy_stats:
> free_percpu(dp->stats_percpu);
> err_destroy_table:
> ovs_flow_tbl_destroy(>table);
> -err_free_dp:
> +err_destroy_dp:
> kfree(dp);
> -err_free_reply:
> +err_destroy_reply:
> kfree_skb(reply);
> err:
> return err;
> --
> 1.8.3.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] [PATCHv5] netdev-afxdp: Add need_wakeup supprt.
On Fri, Oct 18, 2019 at 05:14:46PM +0200, Ilya Maximets wrote:
> On 26.09.2019 21:29, William Tu wrote:
> >The patch adds support for using need_wakeup flag in AF_XDP rings.
> >A new option, use_need_wakeup, is added. When this option is used,
> >it means that OVS has to explicitly wake up the kernel RX, using poll()
> >syscall and wake up TX, using sendto() syscall. This feature improves
> >the performance by avoiding unnecessary sendto syscalls for TX.
> >For RX, instead of kernel always busy-spinning on fille queue, OVS wakes
> >up the kernel RX processing when fill queue is replenished.
> >
> >The need_wakeup feature is merged into Linux kernel bpf-next tee with commit
> >77cd0d7b3f25 ("xsk: add support for need_wakeup flag in AF_XDP rings") and
> >OVS enables it by default. Running the feature before this version causes
> >xsk bind fails, please use options:use_need_wakeup=false to disable it.
> >If users enable it but runs in an older version of libbpf, then the
> >need_wakeup feature has no effect, and a warning message is logged.
> >
> >For virtual interface, it's better set use_need_wakeup=false, since
> >the virtual device's AF_XDP xmit is synchronous: the sendto syscall
> >enters kernel and process the TX packet on tx queue directly.
> >
> >On Intel Xeon E5-2620 v3 2.4GHz system, performance of physical port
> >to physical port improves from 6.1Mpps to 7.3Mpps.
> >
> >Suggested-by: Ilya Maximets
> >Signed-off-by: William Tu
> >---
> >v5:
> >- address feedback from Ilya
> > - update commit msg about kernel version using bpf-next tree
> > - remove __func__, and use DBG in log_
> > - fix alignment
> > - remove the kernel version requirement for need_wakeup, we can
> > wait until tag is available
> >
> >v4:
> >- move use_need_wakeup check inside xsk_rx_wakeup_if_needed
> >
> >v3:
> >- add warning when user enables it but libbpf not support it
> >- revise documentation
> >
> >v2:
> >- address feedbacks from Ilya and Eelco
> >- add options:use_need_wakeup, default to true
> >- remove poll timeout=1sec, make poll() return immediately
> >- naming change: rename to xsk_rx_wakeup_if_needing
> >- fix indents and return value for errno
> >---
> > Documentation/intro/install/afxdp.rst | 15 -
> > acinclude.m4 | 5 ++
> > lib/netdev-afxdp.c| 108
> > ++
> > lib/netdev-linux-private.h| 2 +
> > vswitchd/vswitch.xml | 12
> > 5 files changed, 126 insertions(+), 16 deletions(-)
> >
> >diff --git a/Documentation/intro/install/afxdp.rst
> >b/Documentation/intro/install/afxdp.rst
> >index 820e9d993d8f..545516b2bbec 100644
> >--- a/Documentation/intro/install/afxdp.rst
> >+++ b/Documentation/intro/install/afxdp.rst
> >@@ -176,9 +176,18 @@ in :doc:`general`::
> >ovs-vswitchd ...
> >ovs-vsctl -- add-br br0 -- set Bridge br0 datapath_type=netdev
> >-Make sure your device driver support AF_XDP, and to use 1 PMD (on core 4)
> >-on 1 queue (queue 0) device, configure these options: **pmd-cpu-mask,
> >-pmd-rxq-affinity, and n_rxq**. The **xdpmode** can be "drv" or "skb"::
> >+Make sure your device driver support AF_XDP, netdev-afxdp supports
> >+the following additional options (see man ovs-vswitchd.conf.db for
> >+more details):
> >+
> >+ * **xdpmode**: use "drv" for driver mode, or "skb" for skb mode.
> >+
> >+ * **use_need_wakeup**: disable by setting to "false", otherwise default
> >+ is "true"
> >+
> >+For example, to use 1 PMD (on core 4) on 1 queue (queue 0) device,
> >+configure these options: **pmd-cpu-mask, pmd-rxq-affinity, and n_rxq**.
> >+The **xdpmode** can be "drv" or "skb"::
> >ethtool -L enp2s0 combined 1
> >ovs-vsctl set Open_vSwitch . other_config:pmd-cpu-mask=0x10
> >diff --git a/acinclude.m4 b/acinclude.m4
> >index f0e38898b17a..f1ecb86adf7c 100644
> >--- a/acinclude.m4
> >+++ b/acinclude.m4
> >@@ -276,6 +276,11 @@ AC_DEFUN([OVS_CHECK_LINUX_AF_XDP], [
> >[Define to 1 if AF_XDP support is available and enabled.])
> > LIBBPF_LDADD=" -lbpf -lelf"
> > AC_SUBST([LIBBPF_LDADD])
> >+
> >+AC_CHECK_DECL([xsk_ring_prod__needs_wakeup], [
> >+ AC_DEFINE([HAVE_XDP_NEED_WAKEUP], [1],
> >+[XDP need wakeup support detected in xsk.h.])
> >+], [], [[#include ]])
> >fi
> >AM_CONDITIONAL([HAVE_AF_XDP], test "$AF_XDP_ENABLE" = true)
> > ])
> >diff --git a/lib/netdev-afxdp.c b/lib/netdev-afxdp.c
> >index 6e01803272aa..fee9413bfd0a 100644
> >--- a/lib/netdev-afxdp.c
> >+++ b/lib/netdev-afxdp.c
> >@@ -26,6 +26,7 @@
> > #include
> > #include
> > #include
> >+#include
> > #include
> > #include
> > #include
> >@@ -82,7 +83,7 @@ BUILD_ASSERT_DECL(PROD_NUM_DESCS == CONS_NUM_DESCS);
> > #define UMEM2DESC(elem, base) ((uint64_t)((char *)elem - (char *)base))
> > static struct xsk_socket_info *xsk_configure(int ifindex, int xdp_queue_id,
> >- int mode);
> >+
Re: [ovs-dev] [PATCH net-next v4 02/10] net: openvswitch: convert mask list in mask array
On Wed, Oct 16, 2019 at 5:52 AM wrote: > > From: Tonghao Zhang > > Port the codes to linux upstream and with little changes. > > Pravin B Shelar, says: > | mask caches index of mask in mask_list. On packet recv OVS > | need to traverse mask-list to get cached mask. Therefore array > | is better for retrieving cached mask. This also allows better > | cache replacement algorithm by directly checking mask's existence. > > Link: > https://github.com/openvswitch/ovs/commit/d49fc3ff53c65e4eca9cabd52ac63396746a7ef5 > Signed-off-by: Tonghao Zhang > Tested-by: Greg Rose > --- LGTM Acked-by: William Tu ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH net-next v4 05/10] net: openvswitch: optimize flow-mask looking up
On Wed, Oct 16, 2019 at 5:54 AM wrote:
>
> From: Tonghao Zhang
>
> The full looking up on flow table traverses all mask array.
> If mask-array is too large, the number of invalid flow-mask
> increase, performance will be drop.
>
> One bad case, for example: M means flow-mask is valid and NULL
> of flow-mask means deleted.
>
> +---+
> | M | NULL | ... | NULL | M|
> +---+
>
> In that case, without this patch, openvswitch will traverses all
> mask array, because there will be one flow-mask in the tail. This
> patch changes the way of flow-mask inserting and deleting, and the
> mask array will be keep as below: there is not a NULL hole. In the
> fast path, we can "break" "for" (not "continue") in flow_lookup
> when we get a NULL flow-mask.
>
> "break"
> v
> +---+
> | M | M | NULL |... | NULL | NULL|
> +---+
>
> This patch don't optimize slow or control path, still using ma->max
> to traverse. Slow path:
> * tbl_mask_array_realloc
> * ovs_flow_tbl_lookup_exact
> * flow_mask_find
Hi Tonghao,
Does this improve performance? After all, the original code simply
check whether the mask is NULL, then goto next mask.
However, with your patch, isn't this invalidated the mask cache entry which
point to the "M" you swap to the front? See my commands inline.
>
> Signed-off-by: Tonghao Zhang
> Tested-by: Greg Rose
> ---
> net/openvswitch/flow_table.c | 101
> ++-
> 1 file changed, 52 insertions(+), 49 deletions(-)
>
> diff --git a/net/openvswitch/flow_table.c b/net/openvswitch/flow_table.c
> index 8d4f50d..a10d421 100644
> --- a/net/openvswitch/flow_table.c
> +++ b/net/openvswitch/flow_table.c
> @@ -538,7 +538,7 @@ static struct sw_flow *flow_lookup(struct flow_table *tbl,
>
> mask = rcu_dereference_ovsl(ma->masks[i]);
> if (!mask)
> - continue;
> + break;
>
> flow = masked_flow_lookup(ti, key, mask, n_mask_hit);
> if (flow) { /* Found */
> @@ -695,7 +695,7 @@ struct sw_flow *ovs_flow_tbl_lookup_ufid(struct
> flow_table *tbl,
> int ovs_flow_tbl_num_masks(const struct flow_table *table)
> {
> struct mask_array *ma = rcu_dereference_ovsl(table->mask_array);
> - return ma->count;
> + return READ_ONCE(ma->count);
> }
>
> static struct table_instance *table_instance_expand(struct table_instance
> *ti,
> @@ -704,21 +704,33 @@ static struct table_instance
> *table_instance_expand(struct table_instance *ti,
> return table_instance_rehash(ti, ti->n_buckets * 2, ufid);
> }
>
> -static void tbl_mask_array_delete_mask(struct mask_array *ma,
> - struct sw_flow_mask *mask)
> +static void tbl_mask_array_del_mask(struct flow_table *tbl,
> + struct sw_flow_mask *mask)
> {
> - int i;
> + struct mask_array *ma = ovsl_dereference(tbl->mask_array);
> + int i, ma_count = READ_ONCE(ma->count);
>
> /* Remove the deleted mask pointers from the array */
> - for (i = 0; i < ma->max; i++) {
> - if (mask == ovsl_dereference(ma->masks[i])) {
> - RCU_INIT_POINTER(ma->masks[i], NULL);
> - ma->count--;
> - kfree_rcu(mask, rcu);
> - return;
> - }
> + for (i = 0; i < ma_count; i++) {
> + if (mask == ovsl_dereference(ma->masks[i]))
> + goto found;
> }
> +
> BUG();
> + return;
> +
> +found:
> + WRITE_ONCE(ma->count, ma_count -1);
> +
> + rcu_assign_pointer(ma->masks[i], ma->masks[ma_count -1]);
> + RCU_INIT_POINTER(ma->masks[ma_count -1], NULL);
So when you swap the ma->masks[ma_count -1], the mask cache entry
who's 'mask_index == ma_count' become all invalid?
Regards,
William
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH net-next v4 06/10] net: openvswitch: simplify the flow_hash
On Wed, Oct 16, 2019 at 5:54 AM wrote:
>
> From: Tonghao Zhang
>
> Simplify the code and remove the unnecessary BUILD_BUG_ON.
>
> Signed-off-by: Tonghao Zhang
> Tested-by: Greg Rose
> ---
LGTM
Acked-by: William Tu
> net/openvswitch/flow_table.c | 8 ++--
> 1 file changed, 2 insertions(+), 6 deletions(-)
>
> diff --git a/net/openvswitch/flow_table.c b/net/openvswitch/flow_table.c
> index a10d421..3e3d345 100644
> --- a/net/openvswitch/flow_table.c
> +++ b/net/openvswitch/flow_table.c
> @@ -432,13 +432,9 @@ int ovs_flow_tbl_flush(struct flow_table *flow_table)
> static u32 flow_hash(const struct sw_flow_key *key,
> const struct sw_flow_key_range *range)
> {
> - int key_start = range->start;
> - int key_end = range->end;
> - const u32 *hash_key = (const u32 *)((const u8 *)key + key_start);
> - int hash_u32s = (key_end - key_start) >> 2;
> -
> + const u32 *hash_key = (const u32 *)((const u8 *)key + range->start);
> /* Make sure number of hash bytes are multiple of u32. */
> - BUILD_BUG_ON(sizeof(long) % sizeof(u32));
> + int hash_u32s = range_n_bytes(range) >> 2;
>
> return jhash2(hash_key, hash_u32s, 0);
> }
> --
> 1.8.3.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 net-next v4 03/10] net: openvswitch: shrink the mask array if necessary
On Wed, Oct 16, 2019 at 5:53 AM wrote:
>
> From: Tonghao Zhang
>
> When creating and inserting flow-mask, if there is no available
> flow-mask, we realloc the mask array. When removing flow-mask,
> if necessary, we shrink mask array.
>
> Signed-off-by: Tonghao Zhang
> Tested-by: Greg Rose
> ---
LGTM
Acked-by: William Tu
On the other hand, maybe we should have an upper limit on the
mask cash size?
Regards,
William`
> net/openvswitch/flow_table.c | 33 +++--
> 1 file changed, 23 insertions(+), 10 deletions(-)
>
> diff --git a/net/openvswitch/flow_table.c b/net/openvswitch/flow_table.c
> index 0d1df53..237cf85 100644
> --- a/net/openvswitch/flow_table.c
> +++ b/net/openvswitch/flow_table.c
> @@ -693,6 +693,23 @@ static struct table_instance
> *table_instance_expand(struct table_instance *ti,
> return table_instance_rehash(ti, ti->n_buckets * 2, ufid);
> }
>
> +static void tbl_mask_array_delete_mask(struct mask_array *ma,
> + struct sw_flow_mask *mask)
> +{
> + int i;
> +
> + /* Remove the deleted mask pointers from the array */
> + for (i = 0; i < ma->max; i++) {
> + if (mask == ovsl_dereference(ma->masks[i])) {
> + RCU_INIT_POINTER(ma->masks[i], NULL);
> + ma->count--;
> + kfree_rcu(mask, rcu);
> + return;
> + }
> + }
> + BUG();
> +}
> +
> /* Remove 'mask' from the mask list, if it is not needed any more. */
> static void flow_mask_remove(struct flow_table *tbl, struct sw_flow_mask
> *mask)
> {
> @@ -706,18 +723,14 @@ static void flow_mask_remove(struct flow_table *tbl,
> struct sw_flow_mask *mask)
>
> if (!mask->ref_count) {
> struct mask_array *ma;
> - int i;
>
> ma = ovsl_dereference(tbl->mask_array);
> - for (i = 0; i < ma->max; i++) {
> - if (mask == ovsl_dereference(ma->masks[i])) {
> - RCU_INIT_POINTER(ma->masks[i], NULL);
> - ma->count--;
> - kfree_rcu(mask, rcu);
> - return;
> - }
> - }
> - BUG();
> + tbl_mask_array_delete_mask(ma, mask);
> +
> + /* Shrink the mask array if necessary. */
> + if (ma->max >= (MASK_ARRAY_SIZE_MIN * 2) &&
> + ma->count <= (ma->max / 3))
> + tbl_mask_array_realloc(tbl, ma->max / 2);
> }
> }
> }
> --
> 1.8.3.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] debian and rhel: Add libunwind dev package.
On Thu, Oct 17, 2019 at 01:38:08PM -0700, Yi-Hung Wei wrote: > On Thu, Oct 17, 2019 at 12:56 PM William Tu wrote: > > > > The patch add libunwind dev package to debian and rhel. > > > > Signed-off-by: William Tu > > --- > > Looks good to me. > > Acked-by: Yi-Hung Wei Applied to master, thanks William ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
