Re: [ovs-dev] [PATCH v7 0/3] Support dynamic rebalancing of offloaded flows

[Simon Horman]

On Thu, Oct 11, 2018 at 04:01:40PM -0700, Ben Pfaff wrote:
> Thanks for the revision.
> 
> This seems basically OK at a glance but I'd like a second set of eyes.
> Simon, are you willing to review this?  It seems roughly in your area
> too.

Thanks Ben,

this has been on my todo list for a little too long.
I'll try and get to it today.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] RFC for support of PMD Auto load balancing

[Nitin Katiyar]

Hi,
I forgot to mention that this patch does not handle frequent rx scheduling of 
queues due to auto load balancing. That is something we had identified and 
changes need to be done to dampen the frequent scheduling of rx queues across 
PMDs.

Regards,
Nitin

-Original Message-
From: Nitin Katiyar 
Sent: Friday, October 12, 2018 1:30 AM
To: ovs-dev@openvswitch.org
Cc: Nitin Katiyar ; Rohith Basavaraja 

Subject: [PATCH] RFC for support of PMD Auto load balancing

Port rx queues that have not been statically assigned to PMDs are currently 
assigned based on periodically sampled load measurements.
The assignment is performed at specific instances – port addition, port 
deletion, upon reassignment request via CLI etc.

Over time it can cause uneven load among the PMDs due to change in traffic 
pattern and thus resulting in lower overall throughout.

This patch enables the support of auto load balancing of PMDs based on measured 
load of RX queues. Each PMD measures the processing load for each of its 
associated queues every 10 seconds. If the aggregated PMD load exceeds a 
configured threshold for 6 consecutive intervals and if there are receive 
packet drops at the NIC the PMD considers itself to be overloaded.

If any PMD considers itself to be overloaded, a dry-run of the PMD assignment 
algorithm is performed by OVS main thread. The dry-run does NOT change the 
existing queue to PMD assignments.

If the resultant mapping of dry-run indicates an improved distribution of the 
load then the actual reassignment will be performed. The automatic rebalancing 
will be disabled by default and has to be enabled via configuration option. 
Load thresholds, improvement factor etc are also configurable.

Following example commands can be used to set the auto-lb params:
ovs-vsctl set open_vswitch . other_config:pmd-auto-lb="true"
ovs-vsctl set open_vswitch . other_config:pmd-auto-lb-thresh="80"
ovs-vsctl set open_vswitch . other_config:pmd-auto-lb-min-improvement="5"
ovs-vsctl set open_vswitch . other_config:pmd-auto-lb-drop-check="true"

Co-authored-by: Rohith Basavaraja 

Signed-off-by: Nitin Katiyar 
Signed-off-by: Rohith Basavaraja 
---
 lib/dpif-netdev.c | 589 +++---
 1 file changed, 561 insertions(+), 28 deletions(-)

diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c index e322f55..28593cc 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -80,6 +80,26 @@
 
 VLOG_DEFINE_THIS_MODULE(dpif_netdev);
 
+/* Auto Load Balancing Defaults */
+#define ACCEPT_IMPROVE_DEFAULT   (25)
+#define PMD_LOAD_THRE_DEFAULT(99)
+#define PMD_AUTO_LB_DISABLE  false
+#define SKIP_DROP_CHECK_DEFAULT  false
+
+//TODO: Should we make it configurable??
+#define PMD_MIN_NUM_DROPS(1)
+#define PMD_MIN_NUM_QFILLS   (1)
+#define PMD_REBALANCE_POLL_TIMER_INTERVAL 6
+
+extern uint32_t log_q_thr;
+
+static bool pmd_auto_lb = PMD_AUTO_LB_DISABLE; static bool 
+auto_lb_skip_drop_check = SKIP_DROP_CHECK_DEFAULT; static float 
+auto_lb_pmd_load_ther = PMD_LOAD_THRE_DEFAULT; static unsigned int 
+auto_lb_accept_improve = ACCEPT_IMPROVE_DEFAULT; static long long int 
+pmd_rebalance_poll_timer = 0;
+
+
 #define FLOW_DUMP_MAX_BATCH 50
 /* Use per thread recirc_depth to prevent recirculation loop. */  #define 
MAX_RECIRC_DEPTH 6 @@ -393,6 +413,8 @@ enum rxq_cycles_counter_type {
interval. */
 RXQ_CYCLES_PROC_HIST,   /* Total cycles of all intervals that are used
during rxq to pmd assignment. */
+RXQ_CYCLES_IDLE_CURR,   /* Cycles spent in idling. */
+RXQ_CYCLES_IDLE_HIST,   /* Total cycles of all idle intervals. */
 RXQ_N_CYCLES
 };
 
@@ -429,6 +451,14 @@ static struct ovsthread_once offload_thread_once
 
 #define XPS_TIMEOUT 50LL/* In microseconds. */
 
+typedef struct {
+unsigned long long prev_drops;
+} q_drops;
+typedef struct {
+unsigned int num_vhost_qfill;
+unsigned int prev_num_vhost_qfill;
+} vhost_qfill;
+
 /* Contained by struct dp_netdev_port's 'rxqs' member.  */  struct 
dp_netdev_rxq {
 struct dp_netdev_port *port;
@@ -439,6 +469,10 @@ struct dp_netdev_rxq {
   particular core. */
 unsigned intrvl_idx;   /* Write index for 'cycles_intrvl'. */
 struct dp_netdev_pmd_thread *pmd;  /* pmd thread that polls this queue. */
+struct dp_netdev_pmd_thread *dry_run_pmd;
+   /* During auto lb trigger, pmd thread
+  associated with this q during dry
+  run. */
 bool is_vhost; /* Is rxq of a vhost port. */
 
 /* Counters of cycles spent successfully polling and processing pkts. */ 
@@ -446,6 +480,16 @@ struct dp_netdev_rxq {
 /* We store PMD_RXQ_INTERVAL_MAX intervals of data for an rxq and then
sum them to yield the cycles used for 

[ovs-dev] L'essentiel de l'actualité Marketing, Communication & Digital accessible en 1 clic

[Service Lecteurs]

en avant

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] ovn-controller: Support processing DHCPv6 information request message type

[Ben Pfaff]

On Fri, Oct 12, 2018 at 06:07:51AM +0530, Numan Siddique wrote:
> On Fri, Oct 12, 2018, 1:43 AM Ben Pfaff  wrote:
> 
> > On Wed, Oct 10, 2018 at 10:48:59PM +0530, nusid...@redhat.com wrote:
> > > From: Numan Siddique 
> > >
> > > When 'dhcpv6_stateless' is configured on the logical router ports,
> > > the client will send DHCPv6 information request message type (using
> > > dhclient -6 -S) to get additional options like dns-server. This
> > > patch supports this option. Ideally we should have supported this
> > > option when the DHCPv6 support was added.
> > >
> > > Signed-off-by: Numan Siddique 
> >
> > Applied, thanks!
> >
> 
> Thanks for the review.

You're welcome.

> Is it possible to backport to 2.10 ? Even though this seems like a feature
> but it fixes the dhcpc6 stateless configuration.

Done.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] ovn-ctl: Fix the wrong pidfile argument passed to ovsdb-servers

[Ben Pfaff]

On Fri, Oct 12, 2018 at 06:02:27AM +0530, Numan Siddique wrote:
> On Fri, Oct 12, 2018, 2:46 AM Ben Pfaff  wrote:
> 
> > On Tue, Oct 09, 2018 at 12:47:11PM +0530, nusid...@redhat.com wrote:
> > > From: Numan Siddique 
> > >
> > > When OVN db servers are started usinb ovn-ctl, if the pid files
> > > (/var/run/openvswitch/ovnnb_db.pid for example) are already
> > > present, then ovn-ctl passes "--pidfile=123" if the pid file has
> > > '123' stored in it. Later on when OVN pacemaker RA script calls
> > > status_ovnnb/status_ovnsb() functions, these returns "not running".
> > >
> > > The shell function 'pidfile_is_running()' stores the contents of
> > > the pid file as  "pid=`cat "$pidfile"`". If the caller also
> > > uses the same variable "pid" to store the file name, it gets
> > > overriden.
> > >
> > > This patch fixes this issue by renaming the local variable "pid"
> > > in the "start_ovsdb__()" shell function to "db_file_name".
> > >
> > > Signed-off-by: Numan Siddique 
> >
> > Thanks, applied to master.
> >
> > It would probably be a good idea to more consistently use "local".
> > Using it for $pidfile and $pid in pidfile_is_running would have avoided
> > this problem
> 
> 
> Thanks for the review. Agree.
> Is it possible to backport to 2.10 and 2.9 ? The issue is seen both the
> branches.

Sure.  Done.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] ovn-controller: Support processing DHCPv6 information request message type

[Numan Siddique]

On Fri, Oct 12, 2018, 1:43 AM Ben Pfaff  wrote:

> On Wed, Oct 10, 2018 at 10:48:59PM +0530, nusid...@redhat.com wrote:
> > From: Numan Siddique 
> >
> > When 'dhcpv6_stateless' is configured on the logical router ports,
> > the client will send DHCPv6 information request message type (using
> > dhclient -6 -S) to get additional options like dns-server. This
> > patch supports this option. Ideally we should have supported this
> > option when the DHCPv6 support was added.
> >
> > Signed-off-by: Numan Siddique 
>
> Applied, thanks!
>

Thanks for the review.
Is it possible to backport to 2.10 ? Even though this seems like a feature
but it fixes the dhcpc6 stateless configuration.

Thanks
Numan
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] ovn-ctl: Fix the wrong pidfile argument passed to ovsdb-servers

[Numan Siddique]

On Fri, Oct 12, 2018, 2:46 AM Ben Pfaff  wrote:

> On Tue, Oct 09, 2018 at 12:47:11PM +0530, nusid...@redhat.com wrote:
> > From: Numan Siddique 
> >
> > When OVN db servers are started usinb ovn-ctl, if the pid files
> > (/var/run/openvswitch/ovnnb_db.pid for example) are already
> > present, then ovn-ctl passes "--pidfile=123" if the pid file has
> > '123' stored in it. Later on when OVN pacemaker RA script calls
> > status_ovnnb/status_ovnsb() functions, these returns "not running".
> >
> > The shell function 'pidfile_is_running()' stores the contents of
> > the pid file as  "pid=`cat "$pidfile"`". If the caller also
> > uses the same variable "pid" to store the file name, it gets
> > overriden.
> >
> > This patch fixes this issue by renaming the local variable "pid"
> > in the "start_ovsdb__()" shell function to "db_file_name".
> >
> > Signed-off-by: Numan Siddique 
>
> Thanks, applied to master.
>
> It would probably be a good idea to more consistently use "local".
> Using it for $pidfile and $pid in pidfile_is_running would have avoided
> this problem


Thanks for the review. Agree.
Is it possible to backport to 2.10 and 2.9 ? The issue is seen both the
branches.

Thanks
Numan

.
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] ofproto-dpif-xlate: Avoid deadlock on multicast snooping recursion.

[Ben Pfaff]

On Mon, Aug 20, 2018 at 08:25:51PM -0700, Ben Pfaff wrote:
> Until now, OVS did multicast snooping outputs holding the read-lock on
> the mcast_snooping object.  This could recurse via a patch port to try to
> take the write-lock on the same object, which deadlocked.  This patch fixes
> the problem, by releasing the read-lock before doing any outputs.
> 
> It would probably be better to use RCU for mcast_snooping.  That would be
> a bigger patch and less suitable for backporting.
> 
> Reported-by: Sameh Elsharkawy
> Reported-at: https://github.com/openvswitch/ovs-issues/issues/153
> Signed-off-by: Ben Pfaff 

This still needs a review.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v7 0/3] Support dynamic rebalancing of offloaded flows

[Ben Pfaff]

Thanks for the revision.

This seems basically OK at a glance but I'd like a second set of eyes.
Simon, are you willing to review this?  It seems roughly in your area
too.

On Thu, Sep 27, 2018 at 12:18:24PM +0530, Sriharsha Basavapatna via dev wrote:
> 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. A new
>OpenvSwitch configuration option - "offload-rebalance" to enable
>this policy.
> 
> Cost/benefits data points:
> 
> 1. Rough cost of the new rebalancing, in terms of CPU time:
> 
>Ran a test that replaced 256 low pps-rate flows(pings) with 256 high
>pps-rate flows(iperf), in a system with 4 cpus (Intel Xeon E5 @ 2.40GHz;
>2 cores with hw threads enabled, rest disabled). The data showed that cpu
>utilization increased by about ~20%. This increase occurs during the
>specific second in which rebalancing is done. And subsequently (from the
>next second), cpu utilization decreases significantly due to offloading
>of higher pps-rate flows. So effectively there's a bump in cpu utilization
>at the time of rebalancing, that is more than compensated by reduced cpu
>utilization once the right flows get offloaded.
> 
> 2. Rough benefits to the user in terms of offload performance:
> 
>The benefits to the user is reduced cpu utilization in the host, since
>higher pps-rate flows get offloaded, replacing lower pps-rate flows.
>Replacing a single offloaded flood ping flow with an iperf flow (multiple
>connections), shows that the cpu %used that was originally 100% on a
>single cpu (rebalancing disabled) goes down to 35% (rebalancing enabled).
>That is, cpu utilization decreased 65% after rebalancing.
> 
> 3. Circumstances under which the benefits would show up:
> 
>The rebalancing benefits would show up once offload resources are
>exhausted and new flows with higher pps-rate are initiated, that would
>otherwise be handled by kernel datapath costing host cpu cycles.
> 
>This can be observed using 'ovs appctl dpctl/ dump-flows' command. Prior
>to rebalancing, any high pps-rate flows that couldn't be offloaded due to
>resource crunch would show up in the output of 'dump-flows type=ovs' and
>after rebalancing such flows would appear in the output of
>'dump-flows type=offloaded'.
> 
> **
> 
> v6-->v7:
>  - Fixed comments from Simon Horman. Key changes:
>- Removed out_netdev and related code which is not needed.
>- Fixed a potential race between un-offloading and adding a rule to kernel.
>- Moved config option parameter check to respective patches.
>- Removed a couple of assertions.
>- Added a few comments.
>- Added a workaround to avoid a checkpatch warning for a pre-decrement op.
> 
> v5-->v6:
>   - Fixed a build error reported by 0-day robot in Patch-2
> 
> v4-->v5:
>  - Fixed v4 comments from Ben Pfaff. Key changes:
>- Updated patch-0 comments to reflect cost/benefit of rebalancing
>- Release references to netdevs after rebalancing
>- Reduce ref holding window to rebalancing routine
>- Avoid collecting all flows before rebalancing (save time/space)
>- Skip rebalancing when no OOR netdev (new function netdev_any_oor())
>- Avoid direct access to netdev members (add new netdev interfaces)
>- Handle tunnel-netdev as a 

[ovs-dev] PROJECT FINANCING

[Abdulaziz Al Serkal]

Assalam-o-Aleikum  ,

Compliments.

We bring you greetings from Al Mal Capital a premier alternative asset 
management firm in the Middle East. We focus on generating sustainable, 
superior performance for all our stakeholders, growing capital and value 
through world-class expertise and adherence to the following core principles: 
We wish to re-invest through project funding in investment loan to third party 
investors, project owners on a 2.5% interest rate per annum on long term 
investment projects that can generate up to 10% ROI within the period of 
investment.

Al Mal Capital sources is a unique business development opportunities and work 
closely with established companies in its actualization. We at Al Mal Capital 
invite all interested project owners and investors to our project 
financing/investment programme. We pride ourselves on forming true partnerships 
with the owners and stakeholders of businesses we invest in. We work closely 
with our partners to create value and put portfolio companies firmly on an 
accelerated growth path.

If our offer for collaboration is within the acceptable financing scheme 
anticipated by your organization, we will be glad to consider a possible 
collaboration with your organization.

Best regards,
Board Member
Abdulaziz Al Serkal
Al Mal Capital
48 Burj Gate, Downtown Dubai,
Sheikh Zayed Road,Office 901,
P.O. Box 119930,
Dubai, UAE
www.almalcapital.com
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] ossfuzz: Add new target for ODP parsing

[Ben Pfaff]

On Thu, Oct 04, 2018 at 11:55:30AM +0200, Bhargava Shastry wrote:
> Hi,
> 
> Sorry that I was vague about this. The bug manifests as follows:
> 
> There is a function in test-odp.c called "parse_filter" that looks like so:
> 
> ```
> 1. static int
> 2. parse_filter(char *filter_parse)
> 3. {
> 4. struct ds in;
> 5. struct flow flow_filter;
> 6. struct flow_wildcards wc_filter;
> 7. char *error, *filter = NULL;
> 
> 8. vlog_set_levels_from_string_assert("odp_util:console:dbg");
> 9. if (filter_parse && !strncmp(filter_parse, "filter=", 7)) {
> 10.   filter = xstrdup(filter_parse + 7);
> 11.   memset(_filter, 0, sizeof(flow_filter));
> 12.   memset(_filter, 0, sizeof(wc_filter));
> 13.   error = parse_ofp_exact_flow(_filter, _filter, NULL,
> 14.filter, NULL);
> 15.   if (error) {
> 16.   ovs_fatal(0, "Failed to parse filter (%s)", error);
> 17.   }
> 18.   else {
> 19.   ovs_fatal(0, "No filter to parse.");
> 20.   }
> ```
> 
> My understanding of this function is as follows:
>   - it takes a filter string and an ODP flow as string as inputs (ODP
> string is fetched via stdin and hence not seen in the code above)
>   - before dealing with the ODP string it invokes the
> `parse_ofp_exact_flow` function passing the filter string as input
>   - if there is an error in `parse_ofp_exact_flow`, the `parse_filter`
> function aborts with a "Failed to parse filter" error printed to stdout.
> 
> Now, I experimented with several filter strings such as the following
> (copied from odp.at)
> 
> const char *filters[num_filters] = {
> "filter=\'dl_type=0x1235\'",
> "filter=\'dl_vlan=99\'",
> "filter=\'dl_vlan=99,ip\'",
> "filter=\'ip,nw_src=35.8.2.199\'",
> "filter=\'ip,nw_dst=172.16.0.199\'",
> "filter=\'dl_type=0x0800,nw_src=35.8.2.199,nw_dst=172.16.0.199\'",
> "filter=\'icmp,nw_src=35.8.2.199\'",
> "filter=\'arp,arp_spa=1.2.3.5\'",
> "filter=\'tcp,tp_src=90\'",
> "filter=\'tcp6,tp_src=90\'"
> };
> 
> For example, when we feed the filter string "filter=\'dl_vlan=99,ip\'"
> to the function `parse_ofp_exact_flow` we get the following error:
> 
> ```
> unknown field `dl_van`
> ```
> 
> and aborts the test.
> 
> I don't know how `parse_ofp_exact_flow` is implemented so I don't know
> what I am doing wrong. Any suggestions?
> 
>  Test integration 
> 
> My original idea was to integrate parse_filter as follows:
> 
> - Take one random filter string from the const char array above
> - Feed this to a modified parse_filter function declaration like so
> 
> parse_filter(const char *filter, const char *input)
> 
> where
> - filter is a randomly chosen filter string from the list above (see
> filters[])
> - input is the fuzzed input
> 
> A simple way to make selection random is to use the modulo operator like so
> ```
> int
> LLVMFuzzerTestOneInput(const uint8_t *data, size_t size)
> {
> ...
> /* Parse filter. */
> int idx = size % 10;
> parse_filter(filters[idx], input);
> ```
> 
> Does this make sense?

I see what you're saying.

If it's valuable, we can work around these problems.

Do you expect to see much improvement in coverage from fuzzing this?
Does it use OVS functions that don't get called from the other tests?

Thanks,

Ben.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] ovsdb idl: API to get the idl session

[Ben Pfaff]

On Tue, Oct 09, 2018 at 12:56:37PM +0530, Arunkumar Rg wrote:
> Added an API in ovsdb-idl to get the idl->session. This API is useful in
> the
> ovsdb client code as the session information returned by this API would be
> used to get the details about the remote session.
> 
> Signed-off-by: Arunkumar R G 

Thanks for the patch.

"git am" says:

Applying: ovsdb idl: API to get the idl session
error: corrupt patch at line 21
Patch failed at 0001 ovsdb idl: API to get the idl session
The copy of the patch that failed is found in: 
/home/blp/nicira/ovs.git/worktrees/ovs/rebase-apply/patch
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

The problem is that the patch is wordwrapped.  You should resend the
patch without wordwrapping.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH 2/2 v2] Work around Python/C JSON unicode differences

[Ben Pfaff]

On Tue, Oct 09, 2018 at 05:45:22PM +0100, Lucas Alvares Gomes wrote:
> On Tue, Oct 9, 2018 at 5:33 PM Terry Wilson  wrote:
> >
> > The OVS C-based JSON parser operates on bytes, so the parser_feed
> > function returns the number of bytes that are processed. The pure
> > Python JSON parser currently operates on unicode, so it expects
> > that Parser.feed() returns a number of characters. This difference
> > leads to parsing errors when unicode characters are passed to the
> > C JSON parser from Python.
> >
> > Signed-off-by: Terry Wilson 
> > ---
> >  python/ovs/jsonrpc.py | 9 +++--
> >  1 file changed, 7 insertions(+), 2 deletions(-)
> >
> > diff --git a/python/ovs/jsonrpc.py b/python/ovs/jsonrpc.py
> > index 4873cff..1323ba7 100644
> > --- a/python/ovs/jsonrpc.py
> > +++ b/python/ovs/jsonrpc.py
> > @@ -272,7 +272,8 @@ class Connection(object):
> >  # data, so we convert it here as soon as possible.
> >  if data and not error:
> >  try:
> > -data = decoder.decode(data)
> > +if six.PY3 or ovs.json.PARSER == 
> > ovs.json.PARSER_PY:
> > +data = decoder.decode(data)
> >  except UnicodeError:
> >  error = errno.EILSEQ
> >  if error:
> > @@ -298,7 +299,11 @@ class Connection(object):
> >  else:
> >  if self.parser is None:
> >  self.parser = ovs.json.Parser()
> > -self.input = self.input[self.parser.feed(self.input):]
> > +if six.PY3 and ovs.json.PARSER == ovs.json.PARSER_C:
> > +self.input = self.input.encode('utf-8')[
> > +self.parser.feed(self.input):].decode()
> > +else:
> > +self.input = self.input[self.parser.feed(self.input):]
> >  if self.parser.is_done():
> >  msg = self.__process_msg()
> >  if msg:
> > --
> > 1.8.3.1
> >
> > ___
> > dev mailing list
> > d...@openvswitch.org
> > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
> 
> Acked-By: Lucas Alvares Gomes 

Thanks, Terry (and Lucas).  I applied these to master.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v3 2/2] ovndb-servers.ocf: Add ssl support for managing OVN DB resources with pacemaker using LB VIP.

[Ben Pfaff]

You're welcome.  Done.

On Thu, Oct 11, 2018 at 02:33:38PM -0700, aginwala aginwala wrote:
> Thanks Ben:
> 
> Please backport it to 2.10 and 2.9
> 
> On Thu, Oct 11, 2018 at 2:06 PM Ben Pfaff  wrote:
> 
> > On Wed, Oct 10, 2018 at 11:42:08AM +0530, Numan Siddique wrote:
> > > On Wed, Oct 10, 2018 at 3:42 AM aginwala  wrote:
> > >
> > > > When starting OVN DBs in HA using pacemaker with ssl, we need to pass
> > ssl
> > > > certs for starting standby DBs. Hence, we need this change.
> > > >
> > > > Signed-off-by: aginwala 
> > > > Acked-by: Han Zhou 
> > > >
> > >
> > > Acked-by: Numan Siddique 
> >
> > Thanks, Ali and Numan (and Han).  I applied this to master.  Let me know
> > if it needs backports.
> >
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH 2/2] odp-util: Handle returning when parse_odp_key_mask_attr handles ufid

[Yifeng Sun]

Ok, I'll check it out.

On Thu, Oct 11, 2018 at 2:04 PM Ben Pfaff  wrote:

> I think that there is only one caller of parse_odp_key_mask_attr() that
> needs to understand a ufid, which is odp_flow_from_string().  Maybe the
> code to parse a ufid could go there instead.
>
> On Thu, Oct 11, 2018 at 01:53:54PM -0700, Yifeng Sun wrote:
> > Thanks for the review.
> >
> > How about returning a -EINVAL in this case?
> >
> > On Thu, Oct 11, 2018 at 1:39 PM Ben Pfaff  wrote:
> >
> > > On Tue, Oct 09, 2018 at 03:39:18PM -0700, Yifeng Sun wrote:
> > > > When parse_odp_key_mask_attr runs into ufid, it returns length of
> ufid
> > > > without appending data into ofpbufs. This commit adds additional
> > > > checking for this case.
> > > >
> > > > Found this bug when debugging
> > > > https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=10850,
> > > > but not certain it is related.
> > > >
> > > > Signed-off-by: Yifeng Sun 
> > >
> > > Thanks for the fix.
> > >
> > > It might be better to come up with a way to disallow a ufid here, since
> > > it doesn't make sense to put a ufid into a "set" action.
> > >
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v3 2/2] ovndb-servers.ocf: Add ssl support for managing OVN DB resources with pacemaker using LB VIP.

[aginwala aginwala]

Thanks Ben:

Please backport it to 2.10 and 2.9

On Thu, Oct 11, 2018 at 2:06 PM Ben Pfaff  wrote:

> On Wed, Oct 10, 2018 at 11:42:08AM +0530, Numan Siddique wrote:
> > On Wed, Oct 10, 2018 at 3:42 AM aginwala  wrote:
> >
> > > When starting OVN DBs in HA using pacemaker with ssl, we need to pass
> ssl
> > > certs for starting standby DBs. Hence, we need this change.
> > >
> > > Signed-off-by: aginwala 
> > > Acked-by: Han Zhou 
> > >
> >
> > Acked-by: Numan Siddique 
>
> Thanks, Ali and Numan (and Han).  I applied this to master.  Let me know
> if it needs backports.
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v3] ovn-nbctl: Add basic port group commands.

[Ben Pfaff]

On Tue, Oct 09, 2018 at 08:27:05AM -0400, Mark Michelson wrote:
> This adds the following commands:
> 
> pg-add: Add a new port group, optionally adding switch ports at
> creation.
> pg-set-ports: Sets the logical switch ports on a port group
> pg-del: Remove a port group.
> 
> The main motivation for these commands is that it allows for adding
> logical switch ports by name rather than UUID.
> 
> Signed-off-by: Mark Michelson 

Thanks, applied to master.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] ovn-ctl: Fix the wrong pidfile argument passed to ovsdb-servers

[Ben Pfaff]

On Tue, Oct 09, 2018 at 12:47:11PM +0530, nusid...@redhat.com wrote:
> From: Numan Siddique 
> 
> When OVN db servers are started usinb ovn-ctl, if the pid files
> (/var/run/openvswitch/ovnnb_db.pid for example) are already
> present, then ovn-ctl passes "--pidfile=123" if the pid file has
> '123' stored in it. Later on when OVN pacemaker RA script calls
> status_ovnnb/status_ovnsb() functions, these returns "not running".
> 
> The shell function 'pidfile_is_running()' stores the contents of
> the pid file as  "pid=`cat "$pidfile"`". If the caller also
> uses the same variable "pid" to store the file name, it gets
> overriden.
> 
> This patch fixes this issue by renaming the local variable "pid"
> in the "start_ovsdb__()" shell function to "db_file_name".
> 
> Signed-off-by: Numan Siddique 

Thanks, applied to master.

It would probably be a good idea to more consistently use "local".
Using it for $pidfile and $pid in pidfile_is_running would have avoided
this problem.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] ovn-trace: Fix tracing when ip.dst has to go via a gateway router

[Ben Pfaff]

Thanks Numan (and Mark).  I applied this to master.

On Tue, Oct 09, 2018 at 03:39:46PM -0400, Mark Michelson wrote:
> Looks good to me.
> 
> Acked-by: Mark Michelson 
> 
> On 10/09/2018 09:11 AM, nusid...@redhat.com wrote:
> >From: Numan Siddique 
> >
> >ovn-trace does not trace past an l3gateway port type. This patch
> >fixes it.
> >
> >Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=1626080
> >Suggested-by: Dan Williams 
> >Signed-off-by: Numan Siddique 
> >---
> >  ovn/utilities/ovn-trace.c | 9 +
> >  1 file changed, 9 insertions(+)
> >
> >diff --git a/ovn/utilities/ovn-trace.c b/ovn/utilities/ovn-trace.c
> >index 2446b3f76..40a79ceea 100644
> >--- a/ovn/utilities/ovn-trace.c
> >+++ b/ovn/utilities/ovn-trace.c
> >@@ -645,6 +645,15 @@ read_ports(void)
> >  } else if (!strcmp(sbpb->type, "l3gateway")) {
> >  /* Treat all gateways as local for our purposes. */
> >  dp->has_local_l3gateway = true;
> >+const char *peer_name = smap_get(>options, "peer");
> >+if (peer_name) {
> >+struct ovntrace_port *peer
> >+= shash_find_data(, peer_name);
> >+if (peer) {
> >+port->peer = peer;
> >+port->peer->peer = port;
> >+}
> >+}
> >  }
> >  }
> >
> 
> ___
> 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 v7] ovn: Support configuring the BFD params for the tunnel interfaces

[Ben Pfaff]

On Wed, Oct 10, 2018 at 11:38:55AM +0530, nusid...@redhat.com wrote:
> From: Numan Siddique 
> 
> With this commit the users can override the default values of
> the BFD params - min_rx, min_tx, decay_min_rx and mult if desired.
> This can be useful to debug any issues related to BFD (like
> frequent BFD state changes).
> 
> A new column 'options' is added in NB_Global and SB_Global tables
> of OVN_Northbound and OVN_Southbound schemas respectively. CMS
> can define the options 'bfd-min-rx', 'bfd-min-tx',
> 'bfd-decay-min-rx' and 'bfd-mult' in the options column of
> NB_Global table row. ovn-northd copies these options from
> NB_Global to SB_Global. ovn-controller configures these
> options to the tunnel interfaces when enabling BFD.
> 
> When BFD is disabled, this patch now clears the 'bfd' column
> of the interface row, instead of setting 'enable=false'.
> 
> Signed-off-by: Numan Siddique 

Thanks, applied to master.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v3 2/2] ovndb-servers.ocf: Add ssl support for managing OVN DB resources with pacemaker using LB VIP.

[Ben Pfaff]

On Wed, Oct 10, 2018 at 11:42:08AM +0530, Numan Siddique wrote:
> On Wed, Oct 10, 2018 at 3:42 AM aginwala  wrote:
> 
> > When starting OVN DBs in HA using pacemaker with ssl, we need to pass ssl
> > certs for starting standby DBs. Hence, we need this change.
> >
> > Signed-off-by: aginwala 
> > Acked-by: Han Zhou 
> >
> 
> Acked-by: Numan Siddique 

Thanks, Ali and Numan (and Han).  I applied this to master.  Let me know
if it needs backports.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v3 1/2] ovn-ctl: Allow passing ssl certs when starting OVN DBs in ssl mode.

[Ben Pfaff]

On Wed, Oct 10, 2018 at 12:58:24PM -0700, Han Zhou wrote:
> On Tue, Oct 9, 2018 at 3:11 PM aginwala  wrote:
> >
> > For OVN DBs to work with SSL in HA, we need to have capability to pass ssl
> > certs when starting OVN DBs. Say when starting OVN DBs in active passive
> mode,
> > in order for the standby DBs to sync from master node, it cannot sync
> > because the required ssl certs are not passed when standby DBs are
> initialized.
> > Hence, we need to have this option.
> >
> > e.g. start nb db with ssl certs as below:
> > /usr/share/openvswitch/scripts/ovn-ctl
> --ovn-nb-db-ssl-key=/etc/openvswitch/ovnnb-privkey.pem \
> > --ovn-nb-db-ssl-cert=/etc/openvswitch/ovnnb-cert.pem \
> > --ovn-nb-db-ssl-ca-cert=/etc/openvswitch/cacert.pem \
> > --db-nb-create-insecure-remote=no start_nb_ovsdb
> >
> > When certs are passed in the command line, it will read certs from the
> path
> > mentioned instead of default db configs.
> >
> > Certs can be generated based on ovs ssl docs:
> > http://docs.openvswitch.org/en/latest/howto/ssl/
> >
> > Signed-off-by: aginwala 
> > ---
> >  ovn/utilities/ovn-ctl   | 41
> ++---
> >  ovn/utilities/ovn-ctl.8.xml | 14 ++
> >  2 files changed, 52 insertions(+), 3 deletions(-)
> >
> > diff --git a/ovn/utilities/ovn-ctl b/ovn/utilities/ovn-ctl
> > index 3ff0df6..d71071a 100755
> > --- a/ovn/utilities/ovn-ctl
> > +++ b/ovn/utilities/ovn-ctl
> > @@ -116,6 +116,9 @@ start_ovsdb__() {
> >  local addr
> >  local active_conf_file
> >  local use_remote_in_db
> > +local ovn_db_ssl_key
> > +local ovn_db_ssl_cert
> > +local ovn_db_ssl_cacert
> >  eval pid=\$DB_${DB}_PID
> >  eval cluster_local_addr=\$DB_${DB}_CLUSTER_LOCAL_ADDR
> >  eval cluster_local_port=\$DB_${DB}_CLUSTER_LOCAL_PORT
> > @@ -137,6 +140,9 @@ start_ovsdb__() {
> >  eval addr=\$DB_${DB}_ADDR
> >  eval active_conf_file=\$ovn${db}_active_conf_file
> >  eval use_remote_in_db=\$DB_${DB}_USE_REMOTE_IN_DB
> > +eval ovn_db_ssl_key=\$OVN_${DB}_DB_SSL_KEY
> > +eval ovn_db_ssl_cert=\$OVN_${DB}_DB_SSL_CERT
> > +eval ovn_db_ssl_cacert=\$OVN_${DB}_DB_SSL_CA_CERT
> >
> >  # Check and eventually start ovsdb-server for DB
> >  if pidfile_is_running $pid; then
> > @@ -183,9 +189,23 @@ $cluster_remote_port
> >  if test X"$use_remote_in_db" != Xno; then
> >  set "$@" --remote=db:$schema_name,$table_name,connections
> >  fi
> > -set "$@" --private-key=db:$schema_name,SSL,private_key
> > -set "$@" --certificate=db:$schema_name,SSL,certificate
> > -set "$@" --ca-cert=db:$schema_name,SSL,ca_cert
> > +
> > +if test X"$ovn_db_ssl_key" != X; then
> > +set "$@" --private-key=$ovn_db_ssl_key
> > +else
> > +set "$@" --private-key=db:$schema_name,SSL,private_key
> > +fi
> > +if test X"$ovn_db_ssl_cert" != X; then
> > +set "$@" --certificate=$ovn_db_ssl_cert
> > +else
> > +set "$@" --certificate=db:$schema_name,SSL,certificate
> > +fi
> > +if test X"$ovn_db_ssl_cacert" != X; then
> > +set "$@" --ca-cert=$ovn_db_ssl_cacert
> > +else
> > +set "$@" --ca-cert=db:$schema_name,SSL,ca_cert
> > +fi
> > +
> >  set "$@" --ssl-protocols=db:$schema_name,SSL,ssl_protocols
> >  set "$@" --ssl-ciphers=db:$schema_name,SSL,ssl_ciphers
> >
> > @@ -481,6 +501,15 @@ set_defaults () {
> >  OVN_NORTHD_SB_DB="unix:$DB_SB_SOCK"
> >  DB_NB_USE_REMOTE_IN_DB="yes"
> >  DB_SB_USE_REMOTE_IN_DB="yes"
> > +
> > +OVN_NB_DB_SSL_KEY=""
> > +OVN_NB_DB_SSL_CERT=""
> > +OVN_NB_DB_SSL_CA_CERT=""
> > +
> > +OVN_SB_DB_SSL_KEY=""
> > +OVN_SB_DB_SSL_CERT=""
> > +OVN_SB_DB_SSL_CA_CERT=""
> > +
> >  }
> >
> >  set_option () {
> > @@ -536,6 +565,12 @@ Options:
> >--ovn-controller-ssl-cert=CERT OVN Southbound SSL certificate file
> >--ovn-controller-ssl-ca-cert=CERT OVN Southbound SSL CA certificate
> file
> >--ovn-controller-ssl-bootstrap-ca-cert=CERT Bootstrapped OVN
> Southbound SSL CA certificate file
> > +  --ovn-nb-db-ssl-key=KEY OVN Northbound DB SSL private key file
> > +  --ovn-nb-db-ssl-cert=CERT OVN Northbound DB SSL certificate file
> > +  --ovn-nb-db-ssl-ca-cert=CERT OVN Northbound DB SSL CA certificate file
> > +  --ovn-sb-db-ssl-key=KEY OVN Southbound DB SSL private key file
> > +  --ovn-sb-db-ssl-cert=CERT OVN Southbound DB SSL certificate file
> > +  --ovn-sb-db-ssl-ca-cert=CERT OVN Southbound DB SSL CA certificate file
> >--ovn-manage-ovsdb=yes|noWhether or not the OVN databases
> should be
> > automatically started and stopped
> along
> > with ovn-northd. The default is
> "yes". If
> > diff --git a/ovn/utilities/ovn-ctl.8.xml b/ovn/utilities/ovn-ctl.8.xml
> > index 3b0e67a..c5294d7 100644
> > --- a/ovn/utilities/ovn-ctl.8.xml
> > +++ b/ovn/utilities/ovn-ctl.8.xml
> > @@ -198,4 +198,18 @@
> >start_northd
> >  

Re: [ovs-dev] [PATCH 2/2] odp-util: Handle returning when parse_odp_key_mask_attr handles ufid

[Ben Pfaff]

I think that there is only one caller of parse_odp_key_mask_attr() that
needs to understand a ufid, which is odp_flow_from_string().  Maybe the
code to parse a ufid could go there instead.

On Thu, Oct 11, 2018 at 01:53:54PM -0700, Yifeng Sun wrote:
> Thanks for the review.
> 
> How about returning a -EINVAL in this case?
> 
> On Thu, Oct 11, 2018 at 1:39 PM Ben Pfaff  wrote:
> 
> > On Tue, Oct 09, 2018 at 03:39:18PM -0700, Yifeng Sun wrote:
> > > When parse_odp_key_mask_attr runs into ufid, it returns length of ufid
> > > without appending data into ofpbufs. This commit adds additional
> > > checking for this case.
> > >
> > > Found this bug when debugging
> > > https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=10850,
> > > but not certain it is related.
> > >
> > > Signed-off-by: Yifeng Sun 
> >
> > Thanks for the fix.
> >
> > It might be better to come up with a way to disallow a ufid here, since
> > it doesn't make sense to put a ufid into a "set" action.
> >
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH 2/2] odp-util: Handle returning when parse_odp_key_mask_attr handles ufid

[Yifeng Sun]

Thanks for the review.

How about returning a -EINVAL in this case?

On Thu, Oct 11, 2018 at 1:39 PM Ben Pfaff  wrote:

> On Tue, Oct 09, 2018 at 03:39:18PM -0700, Yifeng Sun wrote:
> > When parse_odp_key_mask_attr runs into ufid, it returns length of ufid
> > without appending data into ofpbufs. This commit adds additional
> > checking for this case.
> >
> > Found this bug when debugging
> > https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=10850,
> > but not certain it is related.
> >
> > Signed-off-by: Yifeng Sun 
>
> Thanks for the fix.
>
> It might be better to come up with a way to disallow a ufid here, since
> it doesn't make sense to put a ufid into a "set" action.
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] expr: Disallow < <= >= > comparisons against empty value set.

[Ben Pfaff]

Thanks, applied and backported.

On Thu, Oct 11, 2018 at 01:08:43PM -0700, Yifeng Sun wrote:
> Looks good to me, thanks.
> 
> Reviewed-by: Yifeng Sun 
> 
> On Thu, Oct 11, 2018 at 12:44 PM Ben Pfaff  wrote:
> 
> > OVN expression syntax does not allow a literal empty value set, like {}.
> > Rather, any literal value set has to have at least one value.  However,
> > value sets that originate from address sets or from port groups can be
> > empty.  In such a case, == and != comparisons are allowed but < <= >= >
> > should be errors.  The actual implementation failed to properly disallow
> > the latter and instead tried to access the first element of the value set,
> > a bad read.  This fixes the problem.
> >
> > Reported-at: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=10731
> > Reported-at
> > :
> > https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=10767
> > Signed-off-by
> > :
> > Ben Pfaff 
> > ---
> >  ovn/lib/expr.c | 5 +
> >  tests/ovn.at   | 2 ++
> >  2 files changed, 7 insertions(+)
> >
> > diff --git a/ovn/lib/expr.c b/ovn/lib/expr.c
> > index 148ac869e861..16dd1776b543 100644
> > --- a/ovn/lib/expr.c
> > +++ b/ovn/lib/expr.c
> > @@ -578,6 +578,11 @@ make_cmp(struct expr_context *ctx,
> >  f->symbol->name);
> >  goto exit;
> >  }
> > +if (!cs->n_values) {
> > +lexer_error(ctx->lexer, "Only == and != operators may be used
> > "
> > +"to compare a field against an empty value set.");
> > +goto exit;
> > +}
> >  if (cs->values[0].masked) {
> >  lexer_error(ctx->lexer, "Only == and != operators may be used
> > "
> >  "with masked constants.  Consider using subfields
> > "
> > diff --git a/tests/ovn.at b/tests/ovn.at
> > index 44475175d20a..94676a9aa802 100644
> > --- a/tests/ovn.at
> > +++ b/tests/ovn.at
> > @@ -351,6 +351,8 @@ eth.dst[40] x => Syntax error at `x' expecting end of
> > input.
> >
> >  ip4.src == {1.2.3.4, $set1, $unknownset} => Syntax error at `$unknownset'
> > expecting address set name.
> >  eth.src == {$set3, badmac, 00:00:00:00:00:01} => Syntax error at `badmac'
> > expecting constant.
> > +
> > +ct_label > $set4 => Only == and != operators may be used to compare a
> > field against an empty value set.
> >  ]])
> >  sed 's/ =>.*//' test-cases.txt > input.txt
> >  sed 's/.* => //' test-cases.txt > expout
> > --
> > 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 2/2] odp-util: Handle returning when parse_odp_key_mask_attr handles ufid

[Ben Pfaff]

On Tue, Oct 09, 2018 at 03:39:18PM -0700, Yifeng Sun wrote:
> When parse_odp_key_mask_attr runs into ufid, it returns length of ufid
> without appending data into ofpbufs. This commit adds additional
> checking for this case.
> 
> Found this bug when debugging
> https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=10850,
> but not certain it is related.
> 
> Signed-off-by: Yifeng Sun 

Thanks for the fix.

It might be better to come up with a way to disallow a ufid here, since
it doesn't make sense to put a ufid into a "set" action.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH 1/2] odp-util: Fix a bug that causes stack overflow

[Yifeng Sun]

In scan_set, '\0' is not always put in the buf.
Ok, I think this patch is not the correct way to fix this issue,
I will come up with a new version.

Thanks,
Yifeng

On Thu, Oct 11, 2018 at 1:18 PM Ben Pfaff  wrote:

> On Tue, Oct 09, 2018 at 03:39:17PM -0700, Yifeng Sun wrote:
> > ofpbuf_put_hex doesn't know buf's length and only checks buf's content
> > to process. This is dangerous. This patch fixes it.
> >
> > Reported-at: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=10865
> > Reported-at: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=10863
> > Reported-at: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=10855
> > Signed-off-by: Yifeng Sun 
> > ---
> >  lib/odp-util.c | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/lib/odp-util.c b/lib/odp-util.c
> > index 7705bb30ae21..d482d5bcf968 100644
> > --- a/lib/odp-util.c
> > +++ b/lib/odp-util.c
> > @@ -2107,6 +2107,7 @@ parse_odp_push_nsh_action(const char *s, struct
> ofpbuf *actions)
> >  if (ovs_scan_len(s, , "md2=0x%511[0-9a-fA-F]", buf)) {
> >  ofpbuf_use_stub(, metadata,
> >  NSH_CTX_HDRS_MAX_LEN);
> > +buf[n - 6] = '\0';
>
> I don't understand this patch yet.  ovs_scan_len() should always
> null-terminate buf.  Are there cases where it does not do that?
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH 1/2] odp-util: Fix a bug that causes stack overflow

[Ben Pfaff]

On Tue, Oct 09, 2018 at 03:39:17PM -0700, Yifeng Sun wrote:
> ofpbuf_put_hex doesn't know buf's length and only checks buf's content
> to process. This is dangerous. This patch fixes it.
> 
> Reported-at: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=10865
> Reported-at: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=10863
> Reported-at: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=10855
> Signed-off-by: Yifeng Sun 
> ---
>  lib/odp-util.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/lib/odp-util.c b/lib/odp-util.c
> index 7705bb30ae21..d482d5bcf968 100644
> --- a/lib/odp-util.c
> +++ b/lib/odp-util.c
> @@ -2107,6 +2107,7 @@ parse_odp_push_nsh_action(const char *s, struct ofpbuf 
> *actions)
>  if (ovs_scan_len(s, , "md2=0x%511[0-9a-fA-F]", buf)) {
>  ofpbuf_use_stub(, metadata,
>  NSH_CTX_HDRS_MAX_LEN);
> +buf[n - 6] = '\0';

I don't understand this patch yet.  ovs_scan_len() should always
null-terminate buf.  Are there cases where it does not do that?
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] ovn-controller: Support processing DHCPv6 information request message type

[Ben Pfaff]

On Wed, Oct 10, 2018 at 10:48:59PM +0530, nusid...@redhat.com wrote:
> From: Numan Siddique 
> 
> When 'dhcpv6_stateless' is configured on the logical router ports,
> the client will send DHCPv6 information request message type (using
> dhclient -6 -S) to get additional options like dns-server. This
> patch supports this option. Ideally we should have supported this
> option when the DHCPv6 support was added.
> 
> Signed-off-by: Numan Siddique 

Applied, thanks!
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] expr: Disallow < <= >= > comparisons against empty value set.

[Yifeng Sun]

Looks good to me, thanks.

Reviewed-by: Yifeng Sun 

On Thu, Oct 11, 2018 at 12:44 PM Ben Pfaff  wrote:

> OVN expression syntax does not allow a literal empty value set, like {}.
> Rather, any literal value set has to have at least one value.  However,
> value sets that originate from address sets or from port groups can be
> empty.  In such a case, == and != comparisons are allowed but < <= >= >
> should be errors.  The actual implementation failed to properly disallow
> the latter and instead tried to access the first element of the value set,
> a bad read.  This fixes the problem.
>
> Reported-at: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=10731
> Reported-at
> :
> https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=10767
> Signed-off-by
> :
> Ben Pfaff 
> ---
>  ovn/lib/expr.c | 5 +
>  tests/ovn.at   | 2 ++
>  2 files changed, 7 insertions(+)
>
> diff --git a/ovn/lib/expr.c b/ovn/lib/expr.c
> index 148ac869e861..16dd1776b543 100644
> --- a/ovn/lib/expr.c
> +++ b/ovn/lib/expr.c
> @@ -578,6 +578,11 @@ make_cmp(struct expr_context *ctx,
>  f->symbol->name);
>  goto exit;
>  }
> +if (!cs->n_values) {
> +lexer_error(ctx->lexer, "Only == and != operators may be used
> "
> +"to compare a field against an empty value set.");
> +goto exit;
> +}
>  if (cs->values[0].masked) {
>  lexer_error(ctx->lexer, "Only == and != operators may be used
> "
>  "with masked constants.  Consider using subfields
> "
> diff --git a/tests/ovn.at b/tests/ovn.at
> index 44475175d20a..94676a9aa802 100644
> --- a/tests/ovn.at
> +++ b/tests/ovn.at
> @@ -351,6 +351,8 @@ eth.dst[40] x => Syntax error at `x' expecting end of
> input.
>
>  ip4.src == {1.2.3.4, $set1, $unknownset} => Syntax error at `$unknownset'
> expecting address set name.
>  eth.src == {$set3, badmac, 00:00:00:00:00:01} => Syntax error at `badmac'
> expecting constant.
> +
> +ct_label > $set4 => Only == and != operators may be used to compare a
> field against an empty value set.
>  ]])
>  sed 's/ =>.*//' test-cases.txt > input.txt
>  sed 's/.* => //' test-cases.txt > expout
> --
> 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 v3] expr: Set a limit on the depth of nested parentheses

[Yifeng Sun]

Thanks!

On Thu, Oct 11, 2018 at 1:02 PM Ben Pfaff  wrote:

> On Wed, Oct 10, 2018 at 03:15:52PM -0700, Yifeng Sun wrote:
> > This patch checks the depth of nested parentheses to prevent
> > stack overflow. Since is_chassis_resident doesn't allow
> > nested parentheses, its following parentheses are not taken
> > into acount in the parentheses-depth context.
> >
> > Reported-at: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=10714
> > Signed-off-by: Yifeng Sun 
> > Suggested-by: Ben Pfaff 
> > ---
> > v1->v2: Handle parse_chassis_resident and add new test, thanks Ben!
> > v2->v3: Ignore parentheses from chassis resident.
>
> Thanks a lot for the updated patch.
>
> The grammar of the error message wasn't quite right, so I adjusted it.
>
> I wanted to always keep the depth counter correct, even if there was an
> error, so I moved the updates, like this:
>
> if (ctx->paren_depth >= MAX_PAREN_DEPTH) {
> lexer_error(ctx->lexer, "Parentheses nested too deeply.");
> return NULL;
> }
>
> ctx->paren_depth++;
> struct expr *e = expr_parse__(ctx);
> ctx->paren_depth--;
>
> The unbalanced parentheses in the test were confusing my editor, so I
> balanced them.
>
> And then I applied this to master and backported it as far as
> branch-2.7.  Thanks again!
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v3] expr: Set a limit on the depth of nested parentheses

[Ben Pfaff]

On Wed, Oct 10, 2018 at 03:15:52PM -0700, Yifeng Sun wrote:
> This patch checks the depth of nested parentheses to prevent
> stack overflow. Since is_chassis_resident doesn't allow
> nested parentheses, its following parentheses are not taken
> into acount in the parentheses-depth context.
> 
> Reported-at: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=10714
> Signed-off-by: Yifeng Sun 
> Suggested-by: Ben Pfaff 
> ---
> v1->v2: Handle parse_chassis_resident and add new test, thanks Ben!
> v2->v3: Ignore parentheses from chassis resident.

Thanks a lot for the updated patch.

The grammar of the error message wasn't quite right, so I adjusted it.

I wanted to always keep the depth counter correct, even if there was an
error, so I moved the updates, like this:

if (ctx->paren_depth >= MAX_PAREN_DEPTH) {
lexer_error(ctx->lexer, "Parentheses nested too deeply.");
return NULL;
}

ctx->paren_depth++;
struct expr *e = expr_parse__(ctx);
ctx->paren_depth--;

The unbalanced parentheses in the test were confusing my editor, so I
balanced them.

And then I applied this to master and backported it as far as
branch-2.7.  Thanks again!
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v3] expr: Access expr_constant.mask only when its type is EXPR_C_INTEGER

[Ben Pfaff]

On Wed, Oct 10, 2018 at 04:02:56PM -0700, Yifeng Sun wrote:
> It is unsafe to access expr_constant.masked when its type
> is EXPR_C_STRING as its value is uninitialized. This patch
> fixes this issue.
> 
> Reported-at: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=10731
> Reported-at: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=10767
> Signed-off-by: Yifeng Sun 
> ---
> v1->v2: Fix email subject by adding [ovs-dev]
> v2->v3: Inspect through code to make sure expr_constant is accessed correctly
> by its type, thanks Ben for the review!

Thanks for the fix.

There was something odd about this, which was that if the field is a
string then it would be EXPR_L_NOMINAL, and so the problem should have
been caught in the test for nominal variables.  Also, the triggering
test case was "ct_label > $set4", and ct_label is not a string field.
Looking closer, $set4 is an empty set, like {}.  That means that the
real underlying problem was that the code was not properly disallowing
empty sets.  So, I sent the following patch:
https://patchwork.ozlabs.org/patch/982674/

Thanks,

Ben.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH] expr: Disallow < <= >= > comparisons against empty value set.

[Ben Pfaff]

OVN expression syntax does not allow a literal empty value set, like {}.
Rather, any literal value set has to have at least one value.  However,
value sets that originate from address sets or from port groups can be
empty.  In such a case, == and != comparisons are allowed but < <= >= >
should be errors.  The actual implementation failed to properly disallow
the latter and instead tried to access the first element of the value set,
a bad read.  This fixes the problem.

Reported-at: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=10731
Reported-at: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=10767
Signed-off-by: Ben Pfaff 
---
 ovn/lib/expr.c | 5 +
 tests/ovn.at   | 2 ++
 2 files changed, 7 insertions(+)

diff --git a/ovn/lib/expr.c b/ovn/lib/expr.c
index 148ac869e861..16dd1776b543 100644
--- a/ovn/lib/expr.c
+++ b/ovn/lib/expr.c
@@ -578,6 +578,11 @@ make_cmp(struct expr_context *ctx,
 f->symbol->name);
 goto exit;
 }
+if (!cs->n_values) {
+lexer_error(ctx->lexer, "Only == and != operators may be used "
+"to compare a field against an empty value set.");
+goto exit;
+}
 if (cs->values[0].masked) {
 lexer_error(ctx->lexer, "Only == and != operators may be used "
 "with masked constants.  Consider using subfields "
diff --git a/tests/ovn.at b/tests/ovn.at
index 44475175d20a..94676a9aa802 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -351,6 +351,8 @@ eth.dst[40] x => Syntax error at `x' expecting end of input.
 
 ip4.src == {1.2.3.4, $set1, $unknownset} => Syntax error at `$unknownset' 
expecting address set name.
 eth.src == {$set3, badmac, 00:00:00:00:00:01} => Syntax error at `badmac' 
expecting constant.
+
+ct_label > $set4 => Only == and != operators may be used to compare a field 
against an empty value set.
 ]])
 sed 's/ =>.*//' test-cases.txt > input.txt
 sed 's/.* => //' test-cases.txt > expout
-- 
2.16.1

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] Oferta de préstamo del 3%

[Sun Loans]

Necesitas un préstamo? Si es así, contáctenos ahora para obtener más detalles. 
Correo electrónico: sunloancompanyll...@gmail.com
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] Point releases

[Ben Pfaff]

On Thu, Oct 11, 2018 at 10:13:41AM -0500, Terry Wilson wrote:
> Just curious what the schedule/process for doing point releases was? I
> noticed that there are 100 patches in branch-2.9 after 2.9.2 and 70 in
> branch-2.10 after 2.10.0. Is it just kind of "when it seems right?" I
> ask because there was python-ovs patch that we'd like to get into a
> release (it's been backported) to fix some networking-ovn CI gate
> issues. (Also, I'd love to get the P"Work around Python/C JSON unicode
> differences" fix reviewed and in...hint, hint...) :)
> 
> I can always do a 'dev' release of python-ovs on PyPI, but if there
> was going to be a point release soon anyway, I figured I'd just wait
> for the process as that would be a lot cleaner. Thanks.

It's really just "when it seems useful".  We're currently overdue for
2.10.1.  I think Justin has been planning to do the release for a while
but he's unavoidably out for a few days.  Probably will happen when he's
back.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] Point releases

[Terry Wilson]

Just curious what the schedule/process for doing point releases was? I
noticed that there are 100 patches in branch-2.9 after 2.9.2 and 70 in
branch-2.10 after 2.10.0. Is it just kind of "when it seems right?" I
ask because there was python-ovs patch that we'd like to get into a
release (it's been backported) to fix some networking-ovn CI gate
issues. (Also, I'd love to get the P"Work around Python/C JSON unicode
differences" fix reviewed and in...hint, hint...) :)

I can always do a 'dev' release of python-ovs on PyPI, but if there
was going to be a point release soon anyway, I figured I'd just wait
for the process as that would be a lot cleaner. Thanks.

Terry
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v11 09/14] dp-packet: Add support for data "linearization".

[Eelco Chaudron]

Hi Tiago,

It has been a while since I reviewed this patchset, guess before my 
summer holiday ;)
I picked this patch out of the set as it does not have my acked-by, and 
I assume all the other which have it did not change too much to warrant 
a review. If they did, let me know.


See inline comments...

Cheers,

Eelco

On 10 Oct 2018, at 18:22, Tiago Lam wrote:


Previous commits have added support to the dp_packet API to handle
multi-segmented packets, where data is not stored contiguously in
memory. However, in some cases, it is inevitable and data must be
provided contiguously. Examples of such cases are when performing 
csums

over the entire packet data, or when write()'ing to a file descriptor
(for a tap interface, for example). For such cases, the dp_packet API
has been extended to provide a way to transform a multi-segmented
DPBUF_DPDK packet into a DPBUF_MALLOC system packet (at the expense of 
a

copy of memory). If the packet's data is already stored in memory
contigously then there's no need to convert the packet.

Thus, the main use cases that were assuming that a dp_packet's data is
always held contiguously in memory were changed to make use of the new
"linear functions" in the dp_packet API when there's a need to 
traverse
the entire's packet data. Per the example above, when the packet's 
data
needs to be write() to the tap's file descriptor, or when the 
conntrack
module needs to verify a packet's checksum, the data is now 
linearized.


Additionally, the layer functions, such as dp_packet_l3() and 
variants,
have been modified to check if there's enough data in the packet 
before

returning a pointer to the data (and callers have been modified
accordingly). This requirement is needed to guarantee that a caller
doesn't read beyond the available memory.


I think the last might be a problem, as now the dp_packet_l2_5(), 
dp_packet_l3() and dp_packet_l4() might start returning NULL.
Some the changes you made to the code referencing this functions would 
crash if they return NULL.


Can I assume you verified that all these calls will be guarded by 
dp_packet_linearize() so we do not access a NULL pointer?


In addition, why have you decided not to use the dp_packet_linearize() 
inside these functions (I think I have an idea)?

This might allow skipping the:

 if (!dp_packet_is_linear(pkt)) {
dp_packet_linearize(pkt);
}

Maybe the above can be changed to just dp_packet_linearize(), and have 
the dp_packet_is_linear() part of the first call. They are both inline 
functions already.




Signed-off-by: Tiago Lam 
---
 lib/bfd.c |   3 +-
 lib/cfm.c |   5 +-
 lib/conntrack-icmp.c  |   4 +-
 lib/conntrack-private.h   |   4 +-
 lib/conntrack-tcp.c   |   6 +-
 lib/conntrack.c   | 109 ++--
 lib/crc32c.c  |  35 +++--
 lib/crc32c.h  |   2 +
 lib/dp-packet.c   |  18 +++
 lib/dp-packet.h   | 288 
+-

 lib/dpif-netdev.c |   5 +
 lib/dpif-netlink.c|   5 +
 lib/dpif.c|   9 ++
 lib/flow.c|  44 ---
 lib/lacp.c|   3 +-
 lib/mcast-snooping.c  |   8 +-
 lib/netdev-bsd.c  |   5 +
 lib/netdev-dummy.c|  13 +-
 lib/netdev-linux.c|  13 +-
 lib/netdev-native-tnl.c   |  38 +++---
 lib/odp-execute.c |  28 ++--
 lib/ofp-print.c   |  10 +-
 lib/ovs-lldp.c|   3 +-
 lib/packets.c | 151 --
 lib/packets.h |   7 +
 lib/pcap-file.c   |   2 +-
 ofproto/ofproto-dpif-upcall.c |  20 ++-
 ofproto/ofproto-dpif-xlate.c  |  42 --
 ovn/controller/pinctrl.c  |  29 +++--
 tests/test-conntrack.c|   2 +-
 tests/test-rstp.c |   8 +-
 tests/test-stp.c  |   8 +-
 32 files changed, 637 insertions(+), 290 deletions(-)

diff --git a/lib/bfd.c b/lib/bfd.c
index cc8c685..12e076a 100644
--- a/lib/bfd.c
+++ b/lib/bfd.c
@@ -721,7 +721,8 @@ bfd_process_packet(struct bfd *bfd, const struct 
flow *flow,

 if (!msg) {
 VLOG_INFO_RL(, "%s: Received too-short BFD control message 
(only "

  "%"PRIdPTR" bytes long, at least %d required).",
- bfd->name, (uint8_t *) dp_packet_tail(p) - l7,
+ bfd->name, dp_packet_size(p) -
+ (l7 - (uint8_t *) dp_packet_data(p)),
  BFD_PACKET_LEN);
 goto out;
 }
diff --git a/lib/cfm.c b/lib/cfm.c
index 71d2c02..83baf2a 100644
--- a/lib/cfm.c
+++ b/lib/cfm.c
@@ -584,7 +584,7 @@ cfm_compose_ccm(struct cfm *cfm, struct dp_packet 
*packet,


 atomic_read_relaxed(>extended, );

-ccm = dp_packet_l3(packet);
+ccm = dp_packet_l3(packet, sizeof(*ccm));
 ccm->mdlevel_version = 0;
 ccm->opcode = CCM_OPCODE;
 

Re: [ovs-dev] Possible Regression due to "ossfuzz: Break flow test target into two targets to speed up fuzzing."

[Simon Horman]

Hi,

apologies, I was a bit hasty in sending out my previous email.
I'll dig a bit further.

On Thu, Oct 11, 2018 at 03:14:22PM +0200, Bhargava Shastry wrote:
> Hello,
> 
> I had a quick look at the linked log. Here are my first impressions
> 
> - The two failed tests are
>   - ofproto-dpif packet-out controller
>   - ovn
> 
> Afaiu, neither have anything to do with the reference commit ID. That
> commit only deals with flow_extract() and some miniflow APIs that are
> different from the failing tests.
> 
> Regards,
> Bhargava
> 
> On 10/11/2018 02:51 PM, Simon Horman wrote:
> > Hi,
> > 
> > it seems that travis-ci is failing due to a testsuite regression introduced
> > by 1adcbcee8f4c ("ossfuzz: Break flow test target into two targets to speed
> > up fuzzing.").
> > 
> > https://travis-ci.org/openvswitch/ovs/jobs/439811394
> > 
> 
> -- 
> Bhargava Shastry 
> Security in Telecommunications
> TU Berlin / Telekom Innovation Laboratories
> Ernst-Reuter-Platz 7, Sekr TEL 17 / D - 10587 Berlin, Germany
> phone: +49 30 8353 58235
> Keybase: https://keybase.io/bshastry
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] Possible Regression due to "ossfuzz: Break flow test target into two targets to speed up fuzzing."

[Bhargava Shastry]

Hello,

I had a quick look at the linked log. Here are my first impressions

- The two failed tests are
  - ofproto-dpif packet-out controller
  - ovn

Afaiu, neither have anything to do with the reference commit ID. That
commit only deals with flow_extract() and some miniflow APIs that are
different from the failing tests.

Regards,
Bhargava

On 10/11/2018 02:51 PM, Simon Horman wrote:
> Hi,
> 
> it seems that travis-ci is failing due to a testsuite regression introduced
> by 1adcbcee8f4c ("ossfuzz: Break flow test target into two targets to speed
> up fuzzing.").
> 
> https://travis-ci.org/openvswitch/ovs/jobs/439811394
> 

-- 
Bhargava Shastry 
Security in Telecommunications
TU Berlin / Telekom Innovation Laboratories
Ernst-Reuter-Platz 7, Sekr TEL 17 / D - 10587 Berlin, Germany
phone: +49 30 8353 58235
Keybase: https://keybase.io/bshastry
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] Possible Regression due to "ossfuzz: Break flow test target into two targets to speed up fuzzing."

[Simon Horman]

Hi,

it seems that travis-ci is failing due to a testsuite regression introduced
by 1adcbcee8f4c ("ossfuzz: Break flow test target into two targets to speed
up fuzzing.").

https://travis-ci.org/openvswitch/ovs/jobs/439811394
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH v2] dpif-netdev-perf: Clarify frequency number.

[Ilya Maximets]

'dpif-netdev/pmd-perf-show' command prints the frequency number
calculated from the total number of cycles spent for iterations
for the measured period. This number could be confusing, because
users may think that it should be equal to CPU frequency, especially
on non-x86 systems where TSC frequency likely does not match with
CPU one.

Moreover, counted TSC cycles could differ from the HW TSC cycles
in case of a large number of PMD reloads, because cycles spent
outside of the main polling loop are not taken into account anywhere.
In this case the frequency will not match even TSC frequency.

Let's clarify the meaning in order to avoid this misunderstanding.
'Cycles' replaced with 'Used TSC cycles', which describes how many TSC
cycles consumed by the main polling loop. % of the total TSC cycles
now printed instead of GHz frequency, because GHz is unclear for
understanding, especially without knowing the exact TSC frequency.

Signed-off-by: Ilya Maximets 
---

Version 2:
* Fixed OSX manpage build failure due to missing of .EX and .EE.

 lib/dpif-netdev-perf.c  | 36 ++--
 lib/dpif-netdev-unixctl.man | 28 +++-
 manpages.mk |  2 ++
 vswitchd/ovs-vswitchd.8.in  |  6 +-
 4 files changed, 36 insertions(+), 36 deletions(-)

diff --git a/lib/dpif-netdev-perf.c b/lib/dpif-netdev-perf.c
index 13f1010c9..8ff4091e5 100644
--- a/lib/dpif-netdev-perf.c
+++ b/lib/dpif-netdev-perf.c
@@ -170,7 +170,8 @@ pmd_perf_format_overall_stats(struct ds *str, struct 
pmd_perf_stats *s,
   double duration)
 {
 uint64_t stats[PMD_N_STATS];
-double us_per_cycle = 100.0 / get_tsc_hz();
+uint64_t tsc_hz = get_tsc_hz();
+double us_per_cycle = 100.0 / tsc_hz;
 
 if (duration == 0) {
 return;
@@ -191,25 +192,25 @@ pmd_perf_format_overall_stats(struct ds *str, struct 
pmd_perf_stats *s,
 uint64_t busy_iter = tot_iter >= idle_iter ? tot_iter - idle_iter : 0;
 
 ds_put_format(str,
-"  Cycles:  %12"PRIu64"  (%.2f GHz)\n"
-"  Iterations:  %12"PRIu64"  (%.2f us/it)\n"
-"  - idle:  %12"PRIu64"  (%4.1f %% cycles)\n"
-"  - busy:  %12"PRIu64"  (%4.1f %% cycles)\n",
-tot_cycles, (tot_cycles / duration) / 1E9,
+"  Iterations:%12"PRIu64"  (%.2f us/it)\n"
+"  - Used TSC cycles: %12"PRIu64"  (%5.1f %% of total cycles)\n"
+"  - idle iterations: %12"PRIu64"  (%5.1f %% of used cycles)\n"
+"  - busy iterations: %12"PRIu64"  (%5.1f %% of used cycles)\n",
 tot_iter, tot_cycles * us_per_cycle / tot_iter,
+tot_cycles, 100.0 * (tot_cycles / duration) / tsc_hz,
 idle_iter,
 100.0 * stats[PMD_CYCLES_ITER_IDLE] / tot_cycles,
 busy_iter,
 100.0 * stats[PMD_CYCLES_ITER_BUSY] / tot_cycles);
 if (rx_packets > 0) {
 ds_put_format(str,
-"  Rx packets:  %12"PRIu64"  (%.0f Kpps, %.0f cycles/pkt)\n"
-"  Datapath passes: %12"PRIu64"  (%.2f passes/pkt)\n"
-"  - EMC hits:  %12"PRIu64"  (%4.1f %%)\n"
-"  - Megaflow hits: %12"PRIu64"  (%4.1f %%, %.2f subtbl lookups/"
- "hit)\n"
-"  - Upcalls:   %12"PRIu64"  (%4.1f %%, %.1f us/upcall)\n"
-"  - Lost upcalls:  %12"PRIu64"  (%4.1f %%)\n",
+"  Rx packets:%12"PRIu64"  (%.0f Kpps, %.0f cycles/pkt)\n"
+"  Datapath passes:   %12"PRIu64"  (%.2f passes/pkt)\n"
+"  - EMC hits:%12"PRIu64"  (%5.1f %%)\n"
+"  - Megaflow hits:   %12"PRIu64"  (%5.1f %%, %.2f "
+"subtbl lookups/hit)\n"
+"  - Upcalls: %12"PRIu64"  (%5.1f %%, %.1f us/upcall)\n"
+"  - Lost upcalls:%12"PRIu64"  (%5.1f %%)\n",
 rx_packets, (rx_packets / duration) / 1000,
 1.0 * stats[PMD_CYCLES_ITER_BUSY] / rx_packets,
 passes, rx_packets ? 1.0 * passes / rx_packets : 0,
@@ -225,17 +226,16 @@ pmd_perf_format_overall_stats(struct ds *str, struct 
pmd_perf_stats *s,
 stats[PMD_STAT_LOST],
 100.0 * stats[PMD_STAT_LOST] / passes);
 } else {
-ds_put_format(str, "  Rx packets:  %12d\n", 0);
+ds_put_format(str, "  Rx packets:%12d\n", 0);
 }
 if (tx_packets > 0) {
 ds_put_format(str,
-"  Tx packets:  %12"PRIu64"  (%.0f Kpps)\n"
-"  Tx batches:  %12"PRIu64"  (%.2f pkts/batch)"
-"\n",
+"  Tx packets:%12"PRIu64"  (%.0f Kpps)\n"
+"  Tx batches:%12"PRIu64"  (%.2f pkts/batch)\n",
 tx_packets, (tx_packets / duration) / 1000,
 tx_batches, 1.0 * tx_packets / tx_batches);
 } else {
-ds_put_format(str, "  Tx 

[ovs-dev] [PATCH] RFC for support of PMD Auto load balancing

[Nitin Katiyar]

Port rx queues that have not been statically assigned to PMDs are currently
assigned based on periodically sampled load measurements.
The assignment is performed at specific instances – port addition, port
deletion, upon reassignment request via CLI etc.

Over time it can cause uneven load among the PMDs due to change in traffic
pattern and thus resulting in lower overall throughout.

This patch enables the support of auto load balancing of PMDs based
on measured load of RX queues. Each PMD measures the processing load for
each of its associated queues every 10 seconds. If the aggregated PMD load
exceeds a configured threshold for 6 consecutive intervals and if there are
receive packet drops at the NIC the PMD considers itself to be overloaded.

If any PMD considers itself to be overloaded, a dry-run of the PMD
assignment algorithm is performed by OVS main thread. The dry-run
does NOT change the existing queue to PMD assignments.

If the resultant mapping of dry-run indicates an improved distribution
of the load then the actual reassignment will be performed. The automatic
rebalancing will be disabled by default and has to be enabled via
configuration option. Load thresholds, improvement factor etc are also
configurable.

Following example commands can be used to set the auto-lb params:
ovs-vsctl set open_vswitch . other_config:pmd-auto-lb="true"
ovs-vsctl set open_vswitch . other_config:pmd-auto-lb-thresh="80"
ovs-vsctl set open_vswitch . other_config:pmd-auto-lb-min-improvement="5"
ovs-vsctl set open_vswitch . other_config:pmd-auto-lb-drop-check="true"

Co-authored-by: Rohith Basavaraja 

Signed-off-by: Nitin Katiyar 
Signed-off-by: Rohith Basavaraja 
---
 lib/dpif-netdev.c | 589 +++---
 1 file changed, 561 insertions(+), 28 deletions(-)

diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index e322f55..28593cc 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -80,6 +80,26 @@
 
 VLOG_DEFINE_THIS_MODULE(dpif_netdev);
 
+/* Auto Load Balancing Defaults */
+#define ACCEPT_IMPROVE_DEFAULT   (25)
+#define PMD_LOAD_THRE_DEFAULT(99)
+#define PMD_AUTO_LB_DISABLE  false
+#define SKIP_DROP_CHECK_DEFAULT  false
+
+//TODO: Should we make it configurable??
+#define PMD_MIN_NUM_DROPS(1)
+#define PMD_MIN_NUM_QFILLS   (1)
+#define PMD_REBALANCE_POLL_TIMER_INTERVAL 6
+
+extern uint32_t log_q_thr;
+
+static bool pmd_auto_lb = PMD_AUTO_LB_DISABLE;
+static bool auto_lb_skip_drop_check = SKIP_DROP_CHECK_DEFAULT;
+static float auto_lb_pmd_load_ther = PMD_LOAD_THRE_DEFAULT;
+static unsigned int auto_lb_accept_improve = ACCEPT_IMPROVE_DEFAULT;
+static long long int pmd_rebalance_poll_timer = 0;
+
+
 #define FLOW_DUMP_MAX_BATCH 50
 /* Use per thread recirc_depth to prevent recirculation loop. */
 #define MAX_RECIRC_DEPTH 6
@@ -393,6 +413,8 @@ enum rxq_cycles_counter_type {
interval. */
 RXQ_CYCLES_PROC_HIST,   /* Total cycles of all intervals that are used
during rxq to pmd assignment. */
+RXQ_CYCLES_IDLE_CURR,   /* Cycles spent in idling. */
+RXQ_CYCLES_IDLE_HIST,   /* Total cycles of all idle intervals. */
 RXQ_N_CYCLES
 };
 
@@ -429,6 +451,14 @@ static struct ovsthread_once offload_thread_once
 
 #define XPS_TIMEOUT 50LL/* In microseconds. */
 
+typedef struct {
+unsigned long long prev_drops;
+} q_drops;
+typedef struct {
+unsigned int num_vhost_qfill;
+unsigned int prev_num_vhost_qfill;
+} vhost_qfill;
+
 /* Contained by struct dp_netdev_port's 'rxqs' member.  */
 struct dp_netdev_rxq {
 struct dp_netdev_port *port;
@@ -439,6 +469,10 @@ struct dp_netdev_rxq {
   particular core. */
 unsigned intrvl_idx;   /* Write index for 'cycles_intrvl'. */
 struct dp_netdev_pmd_thread *pmd;  /* pmd thread that polls this queue. */
+struct dp_netdev_pmd_thread *dry_run_pmd;
+   /* During auto lb trigger, pmd thread
+  associated with this q during dry
+  run. */
 bool is_vhost; /* Is rxq of a vhost port. */
 
 /* Counters of cycles spent successfully polling and processing pkts. */
@@ -446,6 +480,16 @@ struct dp_netdev_rxq {
 /* We store PMD_RXQ_INTERVAL_MAX intervals of data for an rxq and then
sum them to yield the cycles used for an rxq. */
 atomic_ullong cycles_intrvl[PMD_RXQ_INTERVAL_MAX];
+
+/* Following param are used to determine the load on the PMD
+ * for automatic load balance
+ */
+atomic_ullong idle_intrvl[PMD_RXQ_INTERVAL_MAX];
+union {
+q_drops rxq_drops;
+vhost_qfill rxq_vhost_qfill;
+} rxq_drops_or_qfill;
+atomic_uint   overloading_pmd;
 };
 
 /* A port in a netdev-based datapath. */
@@ -682,6 +726,12 @@ struct dp_netdev_pmd_thread {
 struct ovs_mutex 

Re: [ovs-dev] [PATCH v2 2/2] dpif-netdev-perf: Print SMC statistics.

[Ilya Maximets]

On 10.10.2018 21:35, Stokes, Ian wrote:
>> Printing of the SMC hits missed in the 'dpif-netdev/pmd-perf-show'
>> appctl command.
>>
> 
> Thanks Ilya,
> 
> This looks good, I'll add this to the pull request this week.
> 
> I've held off applying the Cycles clarification patch for the moment as 
> flagged in the mail below.
> 
> https://mail.openvswitch.org/pipermail/ovs-dev/2018-October/352904.html
> 
> If there's a v2 of cycle clarification patch I'll take the formatting of the 
> smc stats into account when applying as I think they would be changed from 
> 
> "  - SMC hits:  %12"PRIu64"  (%4.1f %%)\n"
> 
> to
> 
> "  - SMC hits:%12"PRIu64"  (%5.1f %%)\n"
> 
> If that makes sense?

Sure. Thanks.

> 
> Ian
> 
>> CC: Yipeng Wang 
>> Fixes: 60d8ccae135f ("dpif-netdev: Add SMC cache after EMC cache")
>> Signed-off-by: Ilya Maximets 
>> Acked-by: Yipeng Wang 
>> ---
>>  lib/dpif-netdev-perf.c  | 3 +++
>>  lib/dpif-netdev-perf.h  | 2 +-
>>  lib/dpif-netdev-unixctl.man | 3 ++-
>>  3 files changed, 6 insertions(+), 2 deletions(-)
>>
>> diff --git a/lib/dpif-netdev-perf.c b/lib/dpif-netdev-perf.c index
>> 13f1010c9..92ac38dab 100644
>> --- a/lib/dpif-netdev-perf.c
>> +++ b/lib/dpif-netdev-perf.c
>> @@ -206,6 +206,7 @@ pmd_perf_format_overall_stats(struct ds *str, struct
>> pmd_perf_stats *s,
>>  "  Rx packets:  %12"PRIu64"  (%.0f Kpps, %.0f
>> cycles/pkt)\n"
>>  "  Datapath passes: %12"PRIu64"  (%.2f passes/pkt)\n"
>>  "  - EMC hits:  %12"PRIu64"  (%4.1f %%)\n"
>> +"  - SMC hits:  %12"PRIu64"  (%4.1f %%)\n"
>>  "  - Megaflow hits: %12"PRIu64"  (%4.1f %%, %.2f subtbl
>> lookups/"
>>
>> "hit)\n"
>>  "  - Upcalls:   %12"PRIu64"  (%4.1f %%, %.1f
>> us/upcall)\n"
>> @@ -215,6 +216,8 @@ pmd_perf_format_overall_stats(struct ds *str, struct
>> pmd_perf_stats *s,
>>  passes, rx_packets ? 1.0 * passes / rx_packets : 0,
>>  stats[PMD_STAT_EXACT_HIT],
>>  100.0 * stats[PMD_STAT_EXACT_HIT] / passes,
>> +stats[PMD_STAT_SMC_HIT],
>> +100.0 * stats[PMD_STAT_SMC_HIT] / passes,
>>  stats[PMD_STAT_MASKED_HIT],
>>  100.0 * stats[PMD_STAT_MASKED_HIT] / passes,
>>  stats[PMD_STAT_MASKED_HIT]
>> diff --git a/lib/dpif-netdev-perf.h b/lib/dpif-netdev-perf.h index
>> 299d52a98..859c05613 100644
>> --- a/lib/dpif-netdev-perf.h
>> +++ b/lib/dpif-netdev-perf.h
>> @@ -56,7 +56,7 @@ extern "C" {
>>
>>  enum pmd_stat_type {
>>  PMD_STAT_EXACT_HIT, /* Packets that had an exact match (emc). */
>> -PMD_STAT_SMC_HIT,/* Packets that had a sig match hit (SMC).
>> */
>> +PMD_STAT_SMC_HIT,   /* Packets that had a sig match hit (SMC). */
>>  PMD_STAT_MASKED_HIT,/* Packets that matched in the flow table. */
>>  PMD_STAT_MISS,  /* Packets that did not match and upcall was
>> ok. */
>>  PMD_STAT_LOST,  /* Packets that did not match and upcall
>> failed. */
>> diff --git a/lib/dpif-netdev-unixctl.man b/lib/dpif-netdev-unixctl.man
>> index c46f6b19c..0705f1cfb 100644
>> --- a/lib/dpif-netdev-unixctl.man
>> +++ b/lib/dpif-netdev-unixctl.man
>> @@ -11,7 +11,7 @@ Shows performance statistics for one or all pmd threads
>> of the datapath  \fIdp\fR. The special thread "main" sums up the
>> statistics of every non pmd  thread.
>>
>> -The sum of "emc hits", "megaflow hits" and "miss" is the number of
>> +The sum of "emc hits", "smc hits", "megaflow hits" and "miss" is the
>> +number of
>>  packet lookups performed by the datapath. Beware that a recirculated
>> packet  experiences one additional lookup per recirculation, so there may
>> be  more lookups than forwarded packets in the datapath.
>> @@ -135,6 +135,7 @@ pmd thread numa_id 0 core_id 1:
>>Rx packets:   2399607  (2381 Kpps, 848 cycles/pkt)
>>Datapath passes:  3599415  (1.50 passes/pkt)
>>- EMC hits:336472  ( 9.3 %)
>> +  - SMC hits: 0  ( 0.0 %)
>>- Megaflow hits:  3262943  (90.7 %, 1.00 subtbl lookups/hit)
>>- Upcalls:  0  ( 0.0 %, 0.0 us/upcall)
>>- Lost upcalls: 0  ( 0.0 %)
>> --
>> 2.17.1
> 
> 
> 
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] dpif-netdev-perf: Clarify frequency number.

[Ilya Maximets]

On 10.10.2018 19:34, Stokes, Ian wrote:
>> 'dpif-netdev/pmd-perf-show' command prints the frequency number calculated
>> from the total number of cycles spent for iterations for the measured
>> period. This number could be confusing, because users may think that it
>> should be equal to CPU frequency, especially on non-x86 systems where TSC
>> frequency likely does not match with CPU one.
>>
>> Moreover, counted TSC cycles could differ from the HW TSC cycles in case
>> of a large number of PMD reloads, because cycles spent outside of the main
>> polling loop are not taken into account anywhere.
>> In this case the frequency will not match even TSC frequency.
>>
>> Let's clarify the meaning in order to avoid this misunderstanding.
>> 'Cycles' replaced with 'Used TSC cycles', which describes how many TSC
>> cycles consumed by the main polling loop. % of the total TSC cycles now
>> printed instead of GHz frequency, because GHz is unclear for
>> understanding, especially without knowing the exact TSC frequency.
>>
> 
> Hi Ilya,
> 
> I had signed off on this patch but I just spotted its causing the travis 
> build for OS X to fail. It seems .EX and .EE macros are undefined when used 
> in the manpages.
> 
> vswitchd/ovs-vswitchd.8:992: warning: macro `EX' not defined
> vswitchd/ovs-vswitchd.8:1010: warning: macro `EE' not defined
> 
> Digging through some info on .EX and .EE its reported that they are supported 
> on many but not all Unix based systems which may be the issue here. Thoughts?
> 
> https://travis-ci.org/istokes/ovs/jobs/439579443
> 
> Ian

Hi Ian.
Thanks for spotting this. The issue could be solved by including
lib/ovs.tmac file which has required definitions.
I'll send v2 with this change.

Later I'll send the patch to include this file to all the remaining
man roots to avoid such problems in the future. This also will reduce
code duplication as many files has equal redefinitions of a common
macroses.

Best regards, Ilya Maximets.

> 
>> Signed-off-by: Ilya Maximets 
>> ---
>>  lib/dpif-netdev-perf.c  | 36 ++--
>>  lib/dpif-netdev-unixctl.man | 28 +++-
>>  2 files changed, 33 insertions(+), 31 deletions(-)
>>
>> diff --git a/lib/dpif-netdev-perf.c b/lib/dpif-netdev-perf.c index
>> 13f1010c9..8ff4091e5 100644
>> --- a/lib/dpif-netdev-perf.c
>> +++ b/lib/dpif-netdev-perf.c
>> @@ -170,7 +170,8 @@ pmd_perf_format_overall_stats(struct ds *str, struct
>> pmd_perf_stats *s,
>>double duration)  {
>>  uint64_t stats[PMD_N_STATS];
>> -double us_per_cycle = 100.0 / get_tsc_hz();
>> +uint64_t tsc_hz = get_tsc_hz();
>> +double us_per_cycle = 100.0 / tsc_hz;
>>
>>  if (duration == 0) {
>>  return;
>> @@ -191,25 +192,25 @@ pmd_perf_format_overall_stats(struct ds *str, struct
>> pmd_perf_stats *s,
>>  uint64_t busy_iter = tot_iter >= idle_iter ? tot_iter - idle_iter :
>> 0;
>>
>>  ds_put_format(str,
>> -"  Cycles:  %12"PRIu64"  (%.2f GHz)\n"
>> -"  Iterations:  %12"PRIu64"  (%.2f us/it)\n"
>> -"  - idle:  %12"PRIu64"  (%4.1f %% cycles)\n"
>> -"  - busy:  %12"PRIu64"  (%4.1f %% cycles)\n",
>> -tot_cycles, (tot_cycles / duration) / 1E9,
>> +"  Iterations:%12"PRIu64"  (%.2f us/it)\n"
>> +"  - Used TSC cycles: %12"PRIu64"  (%5.1f %% of total
>> cycles)\n"
>> +"  - idle iterations: %12"PRIu64"  (%5.1f %% of used
>> cycles)\n"
>> +"  - busy iterations: %12"PRIu64"  (%5.1f %% of used
>> + cycles)\n",
>>  tot_iter, tot_cycles * us_per_cycle / tot_iter,
>> +tot_cycles, 100.0 * (tot_cycles / duration) / tsc_hz,
>>  idle_iter,
>>  100.0 * stats[PMD_CYCLES_ITER_IDLE] / tot_cycles,
>>  busy_iter,
>>  100.0 * stats[PMD_CYCLES_ITER_BUSY] / tot_cycles);
>>  if (rx_packets > 0) {
>>  ds_put_format(str,
>> -"  Rx packets:  %12"PRIu64"  (%.0f Kpps, %.0f
>> cycles/pkt)\n"
>> -"  Datapath passes: %12"PRIu64"  (%.2f passes/pkt)\n"
>> -"  - EMC hits:  %12"PRIu64"  (%4.1f %%)\n"
>> -"  - Megaflow hits: %12"PRIu64"  (%4.1f %%, %.2f subtbl
>> lookups/"
>> -
>> "hit)\n"
>> -"  - Upcalls:   %12"PRIu64"  (%4.1f %%, %.1f
>> us/upcall)\n"
>> -"  - Lost upcalls:  %12"PRIu64"  (%4.1f %%)\n",
>> +"  Rx packets:%12"PRIu64"  (%.0f Kpps, %.0f
>> cycles/pkt)\n"
>> +"  Datapath passes:   %12"PRIu64"  (%.2f passes/pkt)\n"
>> +"  - EMC hits:%12"PRIu64"  (%5.1f %%)\n"
>> +"  - Megaflow hits:   %12"PRIu64"  (%5.1f %%, %.2f "
>> +"subtbl lookups/hit)\n"
>> +"  - Upcalls: %12"PRIu64"  (%5.1f %%, %.1f
>> us/upcall)\n"
>> +"  - Lost upcalls:%12"PRIu64"  (%5.1f %%)\n",
>>   

Re: [ovs-dev] netdev-dpdk: Support the link speed of XL710

[Federico Iezzi]

So, any news on the other link speeds like 25, 50, and 100Gbps?

Thanks

On Mon, 3 Sep 2018 at 19:54, Flavio Leitner  wrote:

> On Fri, Aug 31, 2018 at 05:54:25AM +0200, Federico Iezzi wrote:
> > Any comment here?
> > This seems like a very easy commit :-)
> >
> >
> >
> > On Thu, 23 Aug 2018 at 13:34, Ian Stokes  wrote:
> >
> > > On 8/22/2018 6:14 PM, Federico Iezzi wrote:
> > > > DPDK exposes API all the way from 10Mbps to 100Gbps.
> > > > http://doc.dpdk.org/api/rte__ethdev_8h_source.html
> > > >
> > > > Can other cards be added? 25G is now getting really popular.
> > > >
> > > > Thanks
> > >
> > > It’s a good point, technically there’s nothing stopping users from
> using
> > > 25/50/56/100 Gbp HW.
> > >
> > > 25/50/56 Gb are not defined specifically as a port feature rate in the
> > > openflow specifications at this time so they would have to be defined
> as
> > > NETDEV_F_OTHER to correlate to the feature rate not being in the
> > > ofp_port feature list in openflow.
> > >
> > > The following incremental on the patch below should suffice:
> > >
> > > @@ -2735,9 +2735,21 @@ netdev_dpdk_get_features(const struct netdev
> > > *netdev,
> > >   if (link.link_speed == ETH_SPEED_NUM_10G) {
> > >   *current = NETDEV_F_10GB_FD;
> > >   }
> > > +if (link.link_speed == ETH_SPEED_NUM_25G) {
> > > +*current = NETDEV_F_OTHER;
> > > +}
> > >   if (link.link_speed == ETH_SPEED_NUM_40G) {
> > >   *current = NETDEV_F_40GB_FD;
> > >   }
> > > +if (link.link_speed == ETH_SPEED_NUM_50G) {
> > > +*current = NETDEV_F_OTHER;
> > > +}
> > > +if (link.link_speed == ETH_SPEED_NUM_56G) {
> > > +*current = NETDEV_F_OTHER;
> > > +}
> > > +if (link.link_speed == ETH_SPEED_NUM_100G) {
> > > +*current = NETDEV_F_100GB_FD;
> > > +}
> > >
> > > What are peoples thoughts? I can submit this as a separate patch if
> > > preferred.
>
> That's all the supported speeds OF defines as you mentioned, but there
> are free bits there if it's important to report a more accurate speed.
>
> Alternatively we could expose that information through get_status() as
> a translated string most probably.
>
> What worries me is that 'current' variable is allocated in the stack and
> dpdk doesn't zero it like bsd or linux does, so if speed falls out of those
> values, it will use whatever was in the stack before as reported
> originally.
>
> Perhaps we could do:
>
>  uint32_t supported = 0;
>
>  if (link.link_duplex == ETH_LINK_FULL_DUPLEX) {
>   switch (link.link_speed) {
>   /* OpenFlow defined values: see enum ofp_port_features */
>   [...]
>   case ETH_SPEED_NUM_10G:
>  supported |= NETDEV_F_10GB_FD;
>  break;
>   case ETH_SPEED_NUM_40G:
>  supported |= NETDEV_F_40GB_FD;
>  break;
>   case ETH_SPEED_NUM_100G:
>  supported |= NETDEV_F_100GB_FD
>  break;
>   default:
> supported |= NETDEV_F_OTHER;
>   }
>  else if (link.link_duplex == ETH_LINK_HALF_DUPLEX) {
>   [...]
>  }
>
>  if (link.link_autoneg) {
>  supported |= NETDEV_F_AUTONEG;
>  }
>
>  *current = supported;
>  *advertised = *supported = *peer = 0;
>
>  return 0;
>
>
> fbl
>
> > >
> > > Thanks
> > > Ian
> > >
> > > Ian
> > >
> > > >
> > > > On Wed, 22 Aug 2018 at 16:28, Stokes, Ian  > > > > wrote:
> > > >
> > > >  > In the scenario of XL710, the link speed which stored in the
> > > table of
> > > >  > Interface is not 40G. Because the implementation of query of
> link
> > > > speed
> > > >  > only support to 10G, the parameter 'current' will be a random
> > > > value in the
> > > >  > scenario of higher link speed. In this case, incorrect link
> speed
> > > > of XL710
> > > >  > nic will be stored in the database.
> > > >  >
> > > >
> > > > Good catch, I've tested and it works as expected. I'll add this
> to
> > > > the dpdk_merge pull request for this week and backport it to the
> > > > previous release branches also.
> > > >
> > > > Thanks
> > > > Ian
> > > >
> > > >  > Signed-off-by: Xu Binbin  > > > >
> > > >  > ---
> > > >  >  lib/netdev-dpdk.c | 3 +++
> > > >  >  1 file changed, 3 insertions(+)
> > > >  >
> > > >  > diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c index
> > > > ac02a09..e4b6ced
> > > >  > 100644
> > > >  > --- a/lib/netdev-dpdk.c
> > > >  > +++ b/lib/netdev-dpdk.c
> > > >  > @@ -2735,6 +2735,9 @@ netdev_dpdk_get_features(const struct
> netdev
> > > >  > *netdev,
> > > >  >  if (link.link_speed == ETH_SPEED_NUM_10G) {
> > > >  >  *current = NETDEV_F_10GB_FD;
> > > >   

[ovs-dev] (no subject)

[Henry Muffa]

-- 
My name is Mr.Henry Muffa, I sent you a letter a month ago, but I did
not receive any response from you and am not sure if you receive
ornot, because of that, I repeat, I'm available documents of an amount
of $7,500.000 dollar in an offshore account. Are you willing to be my
partner? If you are willing, send me with your telephone number so
that I will contact you to discuss this issue with you. Waiting.
Sincerely.
Mr.Henry Muffa
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] Queries on LTS Release & OVSDB Replication featureset

[Kishore.Yetikuri]

Dear OVS Team:
  We are interested in using OVSDB replication feature released as patch 
(603155) with a LTS release. 
Towards achieving that, we request following info from you:

Current LTS release - 2.5.x

-  Does current LTS release (2.5.5) contain this patch?

-  If i2.5.x does not contain this patch, is it possible to release 
2.5.6 with this patch?

-  Is this patch (603155) 
compatible with 2.5.x? If we install this patch on top of 2.5.x, can we 
continue to get support as expected of a LTS release?

Next LTS release

-  Pls indicate ETA for next LTS release

-  Is there any plan to convert 2.10.x into a LTS release?


The above info will greatly help us in picking the right release matching all 
our requirements.

Looking forward to hearing from you.

Regards
Kihosre
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH 2/2] netdev-tc-offloads: TC csum option is not matched with tunnel configuration

[Roi Dayan]

From: Eli Britstein 

Tunnels (gre, geneve, vxlan) support 'csum' option (true/false), default is 
false.
Generated encap TC rule will now be configured as the tunnel configuration.

Signed-off-by: Eli Britstein 
Reviewed-by: Roi Dayan 
---
 lib/dpif-netlink.c   |  5 +
 lib/netdev-tc-offloads.c |  5 +
 lib/netdev.h |  1 +
 lib/tc.c | 10 --
 lib/tc.h |  1 +
 5 files changed, 20 insertions(+), 2 deletions(-)

diff --git a/lib/dpif-netlink.c b/lib/dpif-netlink.c
index ac3d2edebff2..d83f3a90370f 100644
--- a/lib/dpif-netlink.c
+++ b/lib/dpif-netlink.c
@@ -1954,6 +1954,7 @@ parse_flow_put(struct dpif_netlink *dpif, struct 
dpif_flow_put *put)
 struct netdev *dev;
 struct offload_info info;
 ovs_be16 dst_port = 0;
+uint8_t csum_on = false;
 int err;
 
 if (put->flags & DPIF_FP_PROBE) {
@@ -1994,12 +1995,16 @@ 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;
 }
+if (tnl_cfg) {
+csum_on = tnl_cfg->csum;
+}
 netdev_close(outdev);
 }
 }
 
 info.dpif_class = dpif_class;
 info.tp_dst_port = dst_port;
+info.tunnel_csum_on = csum_on;
 err = netdev_flow_put(dev, ,
   CONST_CAST(struct nlattr *, put->actions),
   put->actions_len,
diff --git a/lib/netdev-tc-offloads.c b/lib/netdev-tc-offloads.c
index 62ae021e06d5..6f1fbe6678f9 100644
--- a/lib/netdev-tc-offloads.c
+++ b/lib/netdev-tc-offloads.c
@@ -643,6 +643,10 @@ parse_tc_flower_to_match(struct tc_flower *flower,
 }
 nl_msg_put_be16(buf, OVS_TUNNEL_KEY_ATTR_TP_DST,
 action->encap.tp_dst);
+if (!action->encap.no_csum) {
+nl_msg_put_u8(buf, OVS_TUNNEL_KEY_ATTR_CSUM,
+  !action->encap.no_csum);
+}
 
 parse_tc_flower_geneve_opts(action, buf);
 nl_msg_end_nested(buf, tunnel_offset);
@@ -1276,6 +1280,7 @@ netdev_tc_flow_put(struct netdev *netdev, struct match 
*match,
 }
 if (action->type == TC_ACT_ENCAP) {
 action->encap.tp_dst = info->tp_dst_port;
+action->encap.no_csum = !info->tunnel_csum_on;
 }
 } else if (nl_attr_type(nla) == OVS_ACTION_ATTR_SET_MASKED) {
 const struct nlattr *set = nl_attr_get(nla);
diff --git a/lib/netdev.h b/lib/netdev.h
index 556676046395..5ceba88dbf03 100644
--- a/lib/netdev.h
+++ b/lib/netdev.h
@@ -201,6 +201,7 @@ void netdev_send_wait(struct netdev *, int qid);
 struct offload_info {
 const struct dpif_class *dpif_class;
 ovs_be16 tp_dst_port; /* Destination port for tunnel in SET action */
+uint8_t tunnel_csum_on; /* Tunnel header with checksum */
 
 /*
  * The flow mark id assigened to the flow. If any pkts hit the flow,
diff --git a/lib/tc.c b/lib/tc.c
index 20428bb9e952..47127ca2ce90 100644
--- a/lib/tc.c
+++ b/lib/tc.c
@@ -865,6 +865,7 @@ static const struct nl_policy tunnel_key_policy[] = {
 [TCA_TUNNEL_KEY_ENC_TOS] = { .type = NL_A_U8, .optional = true, },
 [TCA_TUNNEL_KEY_ENC_TTL] = { .type = NL_A_U8, .optional = true, },
 [TCA_TUNNEL_KEY_ENC_OPTS] = { .type = NL_A_NESTED, .optional = true, },
+[TCA_TUNNEL_KEY_NO_CSUM] = { .type = NL_A_U8, .optional = true, },
 };
 
 static int
@@ -995,6 +996,7 @@ nl_parse_act_tunnel_key(struct nlattr *options, struct 
tc_flower *flower)
 struct nlattr *tos = tun_attrs[TCA_TUNNEL_KEY_ENC_TOS];
 struct nlattr *ttl = tun_attrs[TCA_TUNNEL_KEY_ENC_TTL];
 struct nlattr *tun_opt = tun_attrs[TCA_TUNNEL_KEY_ENC_OPTS];
+struct nlattr *no_csum = tun_attrs[TCA_TUNNEL_KEY_NO_CSUM];
 
 action = >actions[flower->action_count++];
 action->type = TC_ACT_ENCAP;
@@ -1010,6 +1012,7 @@ nl_parse_act_tunnel_key(struct nlattr *options, struct 
tc_flower *flower)
 action->encap.tp_dst = dst_port ? nl_attr_get_be16(dst_port) : 0;
 action->encap.tos = tos ? nl_attr_get_u8(tos) : 0;
 action->encap.ttl = ttl ? nl_attr_get_u8(ttl) : 0;
+action->encap.no_csum = no_csum ? nl_attr_get_u8(no_csum) : 0;
 
 err = nl_parse_act_tunnel_opts(tun_opt, action);
 if (err) {
@@ -1628,7 +1631,8 @@ nl_msg_put_act_tunnel_key_set(struct ofpbuf *request, 
ovs_be64 id,
   struct in6_addr *ipv6_src,
   struct in6_addr *ipv6_dst,
   ovs_be16 tp_dst, uint8_t tos, uint8_t ttl,
-  struct tun_metadata tun_metadata)
+  struct tun_metadata tun_metadata,
+  uint8_t no_csum)
 {
 size_t offset;
 
@@ -1659,6 +1663,7 @@ nl_msg_put_act_tunnel_key_set(struct 

[ovs-dev] [PATCH 0/2] Add support to offload GRE tunnels

[Roi Dayan]

Hi,

The first patch is to add support to offload to GRE tunnels using tc.
THe second patch is to offload the tunnel csum option correctly.

Thanks,
Roi


Eli Britstein (2):
  netdev-vport: Make gre netdev type to use TC rules
  netdev-tc-offloads: TC csum option is not matched with tunnel
configuration

 lib/dpif-netlink.c   |  5 +
 lib/netdev-tc-offloads.c |  5 +
 lib/netdev-vport.c   |  3 ++-
 lib/netdev.h |  1 +
 lib/tc.c | 10 --
 lib/tc.h |  1 +
 6 files changed, 22 insertions(+), 3 deletions(-)

-- 
2.7.5

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH 1/2] netdev-vport: Make gre netdev type to use TC rules

[Roi Dayan]

From: Eli Britstein 

The offload api functions already assigned to every tunnel class.
For gre tunnel class only need to also assign the get_ifindex function.

Signed-off-by: Eli Britstein 
Reviewed-by: Roi Dayan 
---
 lib/netdev-vport.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/lib/netdev-vport.c b/lib/netdev-vport.c
index a292b4f81c38..808a43f99d30 100644
--- a/lib/netdev-vport.c
+++ b/lib/netdev-vport.c
@@ -1154,7 +1154,8 @@ netdev_vport_tunnel_register(void)
   .type = "gre",
   .build_header = netdev_gre_build_header,
   .push_header = netdev_gre_push_header,
-  .pop_header = netdev_gre_pop_header
+  .pop_header = netdev_gre_pop_header,
+  .get_ifindex = NETDEV_VPORT_GET_IFINDEX,
   },
   {{NULL, NULL, 0, 0}}
 },
-- 
2.7.5

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev