Re: [ovs-dev] [PATCH ovn] Add support for using OVN specific rundirs

[Numan Siddique]

On Mon, Aug 12, 2019 at 2:59 PM Dumitru Ceara  wrote:

> On Fri, Aug 9, 2019 at 8:24 PM  wrote:
> >
> > From: Numan Siddique 
> >
> > Until now, OVN uses the openvswitch rundirs (rundir, logdir, etcdir).
> > The commit [1] changed the package name from openvswitch to ovn, but
> > it didn't take into the account the effects of it. When "make install"
> > is run ovn-ctl utility is copied to /usr/local/share/ovn/scripts folder.
> > ovn-ctl depends on 'ovs-lib' and it is not present in this scripts foler.
> > Because of which we cannot start OVN services using ovn-ctl.
> >
> > This patch addresses all these issues. It changes the rundir to
> > ovn specific ones. (i.e /usr/local/var/run/ovn, /usr/local/var/log/ovn,
> > /usr/local/etc/ovn with default configuration).
> >
> > [1] - 7795e0e28dce("Change the package name from openvswitch to ovn in
> AC_INIT()")
> >
> > Signed-off-by: Numan Siddique 
>
> Hi Numan,
>
> I tried out your patch. Compiling, check, install worked fine:
>
> ./boot.sh
> ./configure --prefix=/home/dceara/local-builds/usr
> --localstatedir=/home/dceara/local-builds/var
> --sysconfdir=/home/dceara/local-builds/etc CFLAGS="-g" --enable-Werror
> --enable-sparse
> make -j8 check-am TESTSUITEFLAGS="-j8"
> make install
>
> While running OVN I noticed the following:
> a. if we don't export the path to ovn-ctl and ovs-ctl and try to run
> the scripts from their path we get:
>
> $ cd /home/dceara/local-builds/usr/share/ovn/scripts/
> $ /home/dceara/local-builds/usr/share/openvswitch/scripts/ovs-ctl
> start --system-id=local && ./ovn-ctl start_nb_ovsdb && ./ovn-ctl
> start_sb_ovsdb && ./ovn-ctl start_northd && ./ovn-ctl start_controller
> Starting ovsdb-server  [  OK  ]
> Configuring Open vSwitch system IDs[  OK  ]
> Starting ovs-vswitchd  [  OK  ]
> Enabling remote OVSDB managers [  OK  ]
> ./ovn-ctl: line 23: ./openvswitch/scripts/ovs-lib: No such file or
> directory
>
> This happens because of the change in ovn-ctl to determine $ovsdir.
> But I guess that's not a big issue as it's not really expected for a
> user to run it like that, right?
> If I use the full path it works fine:
>

Thanks Dumitru for testing this patch.

Since usr/share/openvswitch/scripts/ or usr/share/ovn/scripts/ is not a
standard path
to find any executables, I think it is expected to use the full path.


  OVS) works fine:

>
> $ /home/dceara/local-builds/usr/share/openvswitch/scripts/ovs-ctl
> start --system-id=local && $PWD/ovn-ctl start_nb_ovsdb && $PWD/ovn-ctl
> start_sb_ovsdb && $PWD/ovn-ctl start_northd && $PWD/ovn-ctl
> start_controller
> Starting ovsdb-server  [  OK  ]
> Configuring Open vSwitch system IDs[  OK  ]
> Starting ovs-vswitchd  [  OK  ]
> Enabling remote OVSDB managers [  OK  ]
> Starting ovn-northd[  OK  ]
> Starting ovn-controller[  OK  ]
>
> b. The user has to be careful if setting paths to the ov[sn]-ctl
> scripts. This is because we still install ovn-ctl from the ovs subtree
> in the usr/share/openvswitch/scripts directory. E.g.:
>
> $ export
> PATH=/home/dceara/local-builds/usr/share/openvswitch/scripts/:/home/dceara/local-builds/usr/share/ovn/scripts/:/home/dceara/local-builds/usr/bin/:$PATH
> $ ovs-ctl start --system-id=local && ovn-ctl start_nb_ovsdb && ovn-ctl
> start_sb_ovsdb && ovn-ctl start_northd && ovn-ctl start_controller
> Starting ovsdb-server  [  OK  ]
> Configuring Open vSwitch system IDs[  OK  ]
> Starting ovs-vswitchd  [  OK  ]
> Enabling remote OVSDB managers [  OK  ]
> ovn-nbctl: unix:/home/dceara/local-builds/var/run/ovn/ovnnb_db.sock:
> database connection failed (Connection refused)
>
> <<< Stuck here because we're actually using ovn-ctl from
> usr/share/openvswitch/scripts
>
> I guess this will be fixed once the OVN code is removed from the OVS
> repo. Until then, specifying the paths in the correct order (OVN
> first, then OVS) works fine:
>

Right. Once OVN is removed from OVS, I think this should be fixed.


> $ export
> PATH=/home/dceara/local-builds/usr/share/ovn/scripts/:/home/dceara/local-builds/usr/share/openvswitch/scripts/:/home/dceara/local-builds/usr/bin/:$PATH
> $ ovs-ctl start --system-id=local && ovn-ctl start_nb_ovsdb && ovn-ctl
> start_sb_ovsdb && ovn-ctl start_northd && ovn-ctl start_controller
> Starting ovsdb-server  [  OK  ]
> Configuring Open vSwitch system IDs[  OK  ]
> Starting ovs-vswitchd  [  OK  ]
> Enabling remote OVSDB managers [  OK  ]
> Starting ovn-northd  

Re: [ovs-dev] [PATCH] Do RCU synchronization at fixed interval in PMD main loop.

[Nitin Katiyar]

Hi
A gentle reminder. Please review and provide the feedback.

Regards,
Nitin

> -Original Message-
> From: Nitin Katiyar
> Sent: Wednesday, August 07, 2019 7:52 PM
> To: ovs-dev@openvswitch.org
> Cc: Nitin Katiyar ; Anju Thomas
> 
> Subject: [PATCH] Do RCU synchronization at fixed interval in PMD main loop.
> 
> Each PMD updates the global sequence number for RCU synchronization
> purpose with other OVS threads. This is done at every 1025th iteration in
> PMD main loop.
> 
> If the PMD thread is responsible for polling large number of queues that are
> carrying traffic, it spends a lot of time processing packets and this results 
> in
> significant delay in performing the housekeeping activities.
> 
> If the OVS main thread is waiting to synchronize with the PMD threads and if
> those threads delay performing housekeeping activities for more than 3 sec
> then LACP processing will be impacted and it will lead to LACP flaps. 
> Similarly,
> other controls protocols run by OVS main thread are impacted.
> 
> For e.g. a PMD thread polling 200 ports/queues with average of 1600
> processing cycles per packet with batch size of 32 may take 1024
> (200 * 1600 * 32) cycles per iteration. In system with 2.0 GHz CPU it means
> more than 5 ms per iteration. So, for 1024 iterations to complete it would be
> more than 5 seconds.
> 
> This gets worse when there are PMD threads which are less loaded.
> It reduces possibility of getting mutex lock in ovsrcu_try_quiesce() by 
> heavily
> loaded PMD and next attempt to quiesce would be after 1024 iterations.
> 
> With this patch, PMD RCU synchronization will be performed after fixed
> interval instead after a fixed number of iterations. This will ensure that 
> even if
> the packet processing load is high the RCU synchronization will not be delayed
> long.
> 
> Co-authored-by: Anju Thomas 
> 
> Signed-off-by: Nitin Katiyar 
> Signed-off-by: Anju Thomas 
> ---
>  lib/dpif-netdev-perf.c | 16   lib/dpif-netdev-perf.h | 17
> +
>  lib/dpif-netdev.c  | 27 +++
>  3 files changed, 44 insertions(+), 16 deletions(-)
> 
> diff --git a/lib/dpif-netdev-perf.c b/lib/dpif-netdev-perf.c index
> e7ed49e..c888e5d 100644
> --- a/lib/dpif-netdev-perf.c
> +++ b/lib/dpif-netdev-perf.c
> @@ -43,22 +43,6 @@ uint64_t iter_cycle_threshold;
> 
>  static struct vlog_rate_limit latency_rl = VLOG_RATE_LIMIT_INIT(600, 600);
> 
> -#ifdef DPDK_NETDEV
> -static uint64_t
> -get_tsc_hz(void)
> -{
> -return rte_get_tsc_hz();
> -}
> -#else
> -/* This function is only invoked from PMD threads which depend on DPDK.
> - * A dummy function is sufficient when building without DPDK_NETDEV. */ -
> static uint64_t
> -get_tsc_hz(void)
> -{
> -return 1;
> -}
> -#endif
> -
>  /* Histogram functions. */
> 
>  static void
> diff --git a/lib/dpif-netdev-perf.h b/lib/dpif-netdev-perf.h index
> 244813f..3f2ee1c 100644
> --- a/lib/dpif-netdev-perf.h
> +++ b/lib/dpif-netdev-perf.h
> @@ -187,6 +187,23 @@ struct pmd_perf_stats {
>  char *log_reason;
>  };
> 
> +#ifdef DPDK_NETDEV
> +static inline uint64_t
> +get_tsc_hz(void)
> +{
> +return rte_get_tsc_hz();
> +}
> +#else
> +/* This function is only invoked from PMD threads which depend on DPDK.
> + * A dummy function is sufficient when building without DPDK_NETDEV. */
> +static inline uint64_t
> +get_tsc_hz(void)
> +{
> +return 1;
> +}
> +#endif
> +
> +
>  #ifdef __linux__
>  static inline uint64_t
>  rdtsc_syscall(struct pmd_perf_stats *s) diff --git a/lib/dpif-netdev.c 
> b/lib/dpif-
> netdev.c index d0a1c58..c3d6835 100644
> --- a/lib/dpif-netdev.c
> +++ b/lib/dpif-netdev.c
> @@ -751,6 +751,9 @@ struct dp_netdev_pmd_thread {
> 
>  /* Set to true if the pmd thread needs to be reloaded. */
>  bool need_reload;
> +
> +/* Last time (in tsc) when PMD was last quiesced */
> +uint64_t last_rcu_quiesced;
>  };
> 
>  /* Interface to netdev-based datapath. */ @@ -5445,6 +5448,7 @@
> pmd_thread_main(void *f_)
>  int poll_cnt;
>  int i;
>  int process_packets = 0;
> +uint64_t rcu_quiesce_interval = 0;
> 
>  poll_list = NULL;
> 
> @@ -5486,6 +5490,13 @@ reload:
>  pmd->intrvl_tsc_prev = 0;
>  atomic_store_relaxed(>intrvl_cycles, 0);
>  cycles_counter_update(s);
> +
> +if (get_tsc_hz() > 1) {
> +/* Calculate ~10 ms interval. */
> +rcu_quiesce_interval = get_tsc_hz() / 100;
> +pmd->last_rcu_quiesced = cycles_counter_get(s);
> +}
> +
>  /* Protect pmd stats from external clearing while polling. */
>  ovs_mutex_lock(>perf_stats.stats_mutex);
>  for (;;) {
> @@ -5493,6 +5504,19 @@ reload:
> 
>  pmd_perf_start_iteration(s);
> 
> +/* Do RCU synchronization at fixed interval instead of doing it
> + * at fixed number of iterations. This ensures that synchronization
> + * would not be delayed long even at high load of packet
> + * processing. */
> +
> +if 

Re: [ovs-dev] [PATCH v3 2/9] ovs-vsctl: Add conntrack zone commands.

[Darrell Ball]

Thanks for the patch

Thanks for the fixups; mostly minor comments inline.

On Mon, Aug 12, 2019 at 5:53 PM Yi-Hung Wei  wrote:

> From: William Tu 
>
> The patch adds commands creating/deleting/listing conntrack zone
> timeout policies:
>   $ ovs-vsctl {add,del,list}-zone-tp dp zone=zone_id ...
>
> Signed-off-by: William Tu 
> ---
>  tests/ovs-vsctl.at   |  34 -
>  utilities/ovs-vsctl.8.in |  26 +++
>  utilities/ovs-vsctl.c| 194
> +++
>  3 files changed, 252 insertions(+), 2 deletions(-)
>
> diff --git a/tests/ovs-vsctl.at b/tests/ovs-vsctl.at
> index 46fa3c5b1a33..df15fb6901a0 100644
> --- a/tests/ovs-vsctl.at
> +++ b/tests/ovs-vsctl.at
> @@ -805,6 +805,20 @@ AT_CHECK(
>[RUN_OVS_VSCTL([--if-exists remove netflow x targets '"1.2.3.4:567
> "'])])
>  AT_CHECK(
>[RUN_OVS_VSCTL([--if-exists clear netflow x targets])])
> +
> +AT_CHECK([RUN_OVS_VSCTL([-- --id=@m create Datapath datapath_version=0 --
> set Open_vSwitch . datapaths:"netdev"=@m])], [0], [stdout])
> +AT_CHECK([RUN_OVS_VSCTL([add-zone-tp netdev zone=1 icmp_first=1
> icmp_reply=2])])
> +AT_CHECK([RUN_OVS_VSCTL([--may-exist add-zone-tp netdev zone=1
> icmp_first=1 icmp_reply=2])])
> +AT_CHECK([RUN_OVS_VSCTL([list-zone-tp netdev])], [0], [Zone:1, Timeout
> Policies: icmp_first=1 icmp_reply=2
> +])
> +AT_CHECK([RUN_OVS_VSCTL([add-zone-tp netdev zone=2 icmp_first=2
> icmp_reply=3])])
> +AT_CHECK([RUN_OVS_VSCTL([list-zone-tp netdev])], [0], [Zone:1, Timeout
> Policies: icmp_first=1 icmp_reply=2
> +Zone:2, Timeout Policies: icmp_first=2 icmp_reply=3
> +])
> +AT_CHECK([RUN_OVS_VSCTL([del-zone-tp netdev zone=1])])
> +AT_CHECK([RUN_OVS_VSCTL([--if-exists del-zone-tp netdev zone=1])])
> +AT_CHECK([RUN_OVS_VSCTL([list-zone-tp netdev])], [0], [Zone:2, Timeout
> Policies: icmp_first=2 icmp_reply=3
> +])
>  OVS_VSCTL_CLEANUP
>  AT_CLEANUP
>
> @@ -890,10 +904,10 @@ AT_CHECK([RUN_OVS_VSCTL([set bridge br0
> flood_vlans=-1])],
>  AT_CHECK([RUN_OVS_VSCTL([set bridge br0 flood_vlans=4096])],
>[1], [], [ovs-vsctl: constraint violation: 4096 is not in the valid
> range 0 to 4095 (inclusive)
>  ])
> -AT_CHECK([RUN_OVS_VSCTL([set c br1 'connection-mode=xyz'])],
> +AT_CHECK([RUN_OVS_VSCTL([set controller br1 'connection-mode=xyz'])],
>[1], [], [[ovs-vsctl: constraint violation: xyz is not one of the
> allowed values ([in-band, out-of-band])
>  ]])
> -AT_CHECK([RUN_OVS_VSCTL([set c br1 connection-mode:x=y])],
> +AT_CHECK([RUN_OVS_VSCTL([set controller br1 connection-mode:x=y])],
>[1], [], [ovs-vsctl: cannot specify key to set for non-map column
> connection_mode
>  ])
>  AT_CHECK([RUN_OVS_VSCTL([add bridge br1 datapath_id x y])],
> @@ -929,6 +943,22 @@ AT_CHECK([RUN_OVS_VSCTL([remove bridge br1
> flood-vlans true])],
>  AT_CHECK([RUN_OVS_VSCTL([clear bridge br1 name])],
>[1], [], [ovs-vsctl: cannot modify read-only column name in table Bridge
>  ])
> +
> +AT_CHECK([RUN_OVS_VSCTL([-- --id=@m create Datapath datapath_version=0 --
> set Open_vSwitch . datapaths:"netdev"=@m])], [0], [stdout])
>

If I execute

AT_CHECK([RUN_OVS_VSCTL([-- --id=@m create Datapath datapath_version=0 --
set Open_vSwitch . datapaths:"netdevvv"=@m])], [0], [stdout])

it works, but there is no such datapath type 'netdevvv'

I think it would be better to enforce an enum here as well thru the schema,
as I mentioned in V2, since this handles errors better.
This is actually the same idea as enforcing enum for timeout keys that was
done for V3 to block bad timeout keys like "foo_bar=3"
I commented patch 1 anyways.


> +AT_CHECK([RUN_OVS_VSCTL([add-zone-tp netdevxx zone=1 icmp_first=1
> icmp_reply=2])],
> +  [1], [], [ovs-vsctl: datapath netdevxx does not exist
> +])
> +AT_CHECK([RUN_OVS_VSCTL([add-zone-tp netdev zone=2 icmp_first=2
> icmp_reply=3])])
> +AT_CHECK([RUN_OVS_VSCTL([add-zone-tp netdev zone=2 icmp_first=2
> icmp_reply=3])],
> +  [1], [], [ovs-vsctl: zone id 2 already exists
> +])
> +AT_CHECK([RUN_OVS_VSCTL([list-zone-tp netdev])], [0], [Zone:2, Timeout
> Policies: icmp_first=2 icmp_reply=3
> +])
> +AT_CHECK([RUN_OVS_VSCTL([del-zone-tp netdev zone=11])],
> +  [1], [], [ovs-vsctl: zone id 11 does not exist
> +])
> +AT_CHECK([RUN_OVS_VSCTL([list-zone-tp netdev])], [0], [Zone:2, Timeout
> Policies: icmp_first=2 icmp_reply=3
> +])
>  OVS_VSCTL_CLEANUP
>  AT_CLEANUP
>
> diff --git a/utilities/ovs-vsctl.8.in b/utilities/ovs-vsctl.8.in
> index 7c09df79bd29..5b9883ae1c3d 100644
> --- a/utilities/ovs-vsctl.8.in
> +++ b/utilities/ovs-vsctl.8.in
> @@ -353,6 +353,32 @@ list.
>  Prints the name of the bridge that contains \fIiface\fR on standard
>  output.
>  .
> +.SS "Conntrack Zone Commands"
> +These commands query and modify datapath CT zones and Timeout Policies.
> +.
> +.IP "[\fB\-\-may\-exist\fR] \fBadd\-zone\-tp \fIdatapath
> \fBzone=\fIzone_id \fIpolicies\fR"
> +Creates a conntrack zone timeout policy with \fIzone_id\fR in
> +\fIdatapath\fR.  The \fIpolicies\fR consist of \fIkey\fB=\fIvalue\fR
> +pairs, separated by 

Re: [ovs-dev] [PATCH v3 1/9] ovs-vswitchd: Add Datapath, CT_Zone, and CT_Zone_Policy tables.

[Darrell Ball]

Thanks for the patch

On Mon, Aug 12, 2019 at 5:52 PM Yi-Hung Wei  wrote:

> From: Justin Pettit 
>
> Signed-off-by: Justin Pettit 
> Signed-off-by: Yi-Hung Wei 
> Co-authored-by: Yi-Hung Wei 
> ---
>  vswitchd/vswitch.ovsschema |  51 -
>  vswitchd/vswitch.xml   | 275
> +
>  2 files changed, 277 insertions(+), 49 deletions(-)
>
> diff --git a/vswitchd/vswitch.ovsschema b/vswitchd/vswitch.ovsschema
> index f7c6eb8983cd..c0a2242ad345 100644
> --- a/vswitchd/vswitch.ovsschema
> +++ b/vswitchd/vswitch.ovsschema
> @@ -1,9 +1,14 @@
>  {"name": "Open_vSwitch",
> - "version": "8.0.0",
> - "cksum": "3962141869 23978",
> + "version": "8.1.0",
> + "cksum": "1635647160 26090",
>   "tables": {
> "Open_vSwitch": {
>   "columns": {
> +   "datapaths": {
> + "type": {"key": {"type": "string"},
>

I had a minor comment in V2 about using an enum here for key - 'system' or
'netdev'
Does it work or is there worry that other datapath types will likely develop
and we will need to update the enum ?


> +  "value": {"type": "uuid",
> +"refTable": "Datapath"},
> +  "min": 0, "max": "unlimited"}},

"bridges": {
>   "type": {"key": {"type": "uuid",
>"refTable": "Bridge"},
> @@ -629,6 +634,48 @@
>"min": 0, "max": "unlimited"},
>   "ephemeral": true}},
>   "indexes": [["target"]]},
> +   "Datapath": {
> + "columns": {
> +   "datapath_version": {
> + "type": "string"},
> +   "ct_zones": {
> + "type": {"key": {"type": "integer",
> +  "minInteger": 0,
> +  "maxInteger": 65535},
> +  "value": {"type": "uuid",
> +"refTable": "CT_Zone"},
> +  "min": 0, "max": "unlimited"}},
>

minor comment from V2; I think
+  "min": 0, "max": "unlimited"}},
should be
+  "min": 0, "max": "65536"}},



> +   "external_ids": {
> + "type": {"key": "string", "value": "string",
> +  "min": 0, "max": "unlimited",
> +   "CT_Zone": {
> + "columns": {
> +   "timeout_policy": {
> + "type": {"key": {"type": "uuid",
> +  "refTable": "CT_Timeout_Policy"},
> +  "min": 0, "max": 1}},
> +   "external_ids": {
> + "type": {"key": "string", "value": "string",
> +  "min": 0, "max": "unlimited",
> +   "CT_Timeout_Policy": {
> + "columns": {
> +   "timeouts": {
> + "type": {"key": {"type" : "string",
> +  "enum": ["set", ["tcp_syn_sent", "tcp_syn_recv",
> +   "tcp_established",
> "tcp_fin_wait",
> +   "tcp_close_wait",
> "tcp_last_ack",
> +   "tcp_time_wait", "tcp_close",
> +   "tcp_syn_sent2",
> "tcp_retransmit",
> +   "tcp_unack", "udp_first",
> +   "udp_single", "udp_multiple",
> +   "icmp_first", "icmp_reply"]]},
> +  "value": {"type" : "integer",
> +"minInteger" : 0,
> +"maxInteger" : 4294967295},
> +  "min": 0, "max": "unlimited"}},
>

Should it be ?
+  "min": 0, "max": "16"}},

or will this create upgrade issues ?



> +   "external_ids": {
> + "type": {"key": "string", "value": "string",
> +  "min": 0, "max": "unlimited",
> "SSL": {
>   "columns": {
> "private_key": {
> diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml
> index 027aee2f523b..495f0acad842 100644
> --- a/vswitchd/vswitch.xml
> +++ b/vswitchd/vswitch.xml
> @@ -52,6 +52,13 @@
>  one record in the  table.
>
>  
> +  
> +Map of datapath types to datapaths.  The
> + column of the 
> +table is used as a key for this map.  The value points to a row in
> +the  table.
> +  
> +
>
>  Set of bridges managed by the daemon.
>
> @@ -1192,53 +1199,11 @@
>
>
>
> -
> -  Reports the version number of the Open vSwitch datapath in use.
> -  This allows management software to detect and report
> discrepancies
> -  between Open vSwitch userspace and datapath versions.  (The  -  column="ovs_version" table="Open_vSwitch"/> column in the  -  table="Open_vSwitch"/> reports the Open vSwitch userspace
> version.)
> -  The version reported depends on the datapath in use:
> -
> -
> -
> -  
> -When the kernel module included in the Open vSwitch source
> tree is
> -used, this column reports 

Re: [ovs-dev] [PATCH v3 9/9] ofproto-dpif-xlate: Translate timeout policy in ct action

[Darrell Ball]

Thanks for the patch

Not a full review; I just did a quick run of the test using a more recent
kernel version

dball@ubuntu:~/ovs$ uname -r
5.0.0-23-generic
dball@ubuntu:~/ovs$ lsb_release -a
No LSB modules are available.
Distributor ID: Ubuntu
Description: Ubuntu 18.04.3 LTS
Release: 18.04
Codename: bionic

The test is no longer blocked on subsequent runs, at least with this kernel
version (others: TBD) - cool !

However

## --- ##
## openvswitch 2.12.90 test suite. ##
## --- ##
 75: conntrack - zone-based timeout policy   FAILED (
system-traffic.at:3228)

.
.
.
VSCTL_ADD_ZONE_TIMEOUT_POLICY([zone=5 udp_single=3 icmp_first=3])

dnl Send ICMP and UDP traffic
NS_CHECK_EXEC([at_ns0], [ping -q -c 3 -i 0.3 -w 2 10.1.1.2 | FORMAT_PING],
[0], [dnl   < FAILS HERE
3 packets transmitted, 3 received, 0% packet loss, time 0ms
])
.
.
.

-3 packets transmitted, 3 received, 0% packet loss, time 0ms
+7 packets transmitted, 0 received, 100% packet loss, time 0ms

warnings:

> 2019-08-13T02:19:06.674Z|1|dpif(handler1)|WARN|system@ovs-system:
failed to put[create] (Invalid argument)
ufid:55d8603a-729c-43d7-9612-b54553e46299
recirc_id(0x2),dp_hash(0/0),skb_priority(0/0),in_port(2),skb_mark(0/0),ct_state(0x21/0x23),ct_zone(0x5/0),ct_mark(0/0),ct_label(0/0),ct_tuple4(src=
10.1.1.1/0.0.0.0,dst=10.1.1.2/0.0.0.0,proto=1/0,tp_src=8/0,tp_dst=0/0
),eth(src=8a:ea:c3:02:6f:94/00:00:00:00:00:00,dst=92:48:5b:47:e2:63/00:00:00:00:00:00),eth_type(0x0800),ipv4(src=
10.1.1.1/0.0.0.0,dst=10.1.1.2/0.0.0.0,proto=1,tos=0/0,ttl=64/0,frag=no),icmp(type=8/0,code=0/0),
actions:ct(commit,zone=5,timeout=ovs_tp_0_icmp4),3
> 2019-08-13T02:19:06.674Z|2|dpif(handler1)|WARN|system@ovs-system:
execute ct(commit,zone=5,timeout=ovs_tp_0_icmp4),3 failed (Invalid
argument) on packet
icmp,vlan_tci=0x,dl_src=8a:ea:c3:02:6f:94,dl_dst=92:48:5b:47:e2:63,nw_src=10.1.1.1,nw_dst=10.1.1.2,nw_tos=0,nw_ecn=0,nw_ttl=64,icmp_type=8,icmp_code=0
icmp_csum:4d0a
>  with metadata
skb_priority(0),skb_mark(0),ct_state(0x21),ct_zone(0x5),ct_tuple4(src=10.1.1.1,dst=10.1.1.2,proto=1,tp_src=8,tp_dst=0),in_port(2)
mtu 0
> 2019-08-13T02:19:06.999Z|3|dpif(handler1)|WARN|system@ovs-system:
failed to put[create] (Invalid argument)
ufid:55d8603a-729c-43d7-9612-b54553e46299
recirc_id(0x2),dp_hash(0/0),skb_priority(0/0),in_port(2),skb_mark(0/0),ct_state(0x21/0x23),ct_zone(0x5/0),ct_mark(0/0),ct_label(0/0),ct_tuple4(src=
10.1.1.1/0.0.0.0,dst=10.1.1.2/0.0.0.0,proto=1/0,tp_src=8/0,tp_dst=0/0
),eth(src=8a:ea:c3:02:6f:94/00:00:00:00:00:00,dst=92:48:5b:47:e2:63/00:00:00:00:00:00),eth_type(0x0800),ipv4(src=
10.1.1.1/0.0.0.0,dst=10.1.1.2/0.0.0.0,proto=1,tos=0/0,ttl=64/0,frag=no),icmp(type=8/0,code=0/0),
actions:ct(commit,zone=5,timeout=ovs_tp_0_icmp4),3
> 2019-08-13T02:19:06.999Z|4|dpif(handler1)|WARN|system@ovs-system:
execute ct(commit,zone=5,timeout=ovs_tp_0_icmp4),3 failed (Invalid
argument) on packet
icmp,vlan_tci=0x,dl_src=8a:ea:c3:02:6f:94,dl_dst=92:48:5b:47:e2:63,nw_src=10.1.1.1,nw_dst=10.1.1.2,nw_tos=0,nw_ecn=0,nw_ttl=64,icmp_type=8,icmp_code=0
icmp_csum:2f10
>  with metadata
skb_priority(0),skb_mark(0),ct_state(0x21),ct_zone(0x5),ct_tuple4(src=10.1.1.1,dst=10.1.1.2,proto=1,tp_src=8,tp_dst=0),in_port(2)
mtu 0
> 2019-08-13T02:19:07.319Z|5|dpif(handler1)|WARN|system@ovs-system:
failed to put[create] (Invalid argument)
ufid:55d8603a-729c-43d7-9612-b54553e46299
recirc_id(0x2),dp_hash(0/0),skb_priority(0/0),in_port(2),skb_mark(0/0),ct_state(0x21/0x23),ct_zone(0x5/0),ct_mark(0/0),ct_label(0/0),ct_tuple4(src=
10.1.1.1/0.0.0.0,dst=10.1.1.2/0.0.0.0,proto=1/0,tp_src=8/0,tp_dst=0/0
),eth(src=8a:ea:c3:02:6f:94/00:00:00:00:00:00,dst=92:48:5b:47:e2:63/00:00:00:00:00:00),eth_type(0x0800),ipv4(src=
10.1.1.1/0.0.0.0,dst=10.1.1.2/0.0.0.0,proto=1,tos=0/0,ttl=64/0,frag=no),icmp(type=8/0,code=0/0),
actions:ct(commit,zone=5,timeout=ovs_tp_0_icmp4),3
> 2019-08-13T02:19:07.320Z|6|dpif(handler1)|WARN|system@ovs-system:
execute ct(commit,zone=5,timeout=ovs_tp_0_icmp4),3 failed (Invalid
argument) on packet
icmp,vlan_tci=0x,dl_src=8a:ea:c3:02:6f:94,dl_dst=92:48:5b:47:e2:63,nw_src=10.1.1.1,nw_dst=10.1.1.2,nw_tos=0,nw_ecn=0,nw_ttl=64,icmp_type=8,icmp_code=0
icmp_csum:906c
>  with metadata
skb_priority(0),skb_mark(0),ct_state(0x21),ct_zone(0x5),ct_tuple4(src=10.1.1.1,dst=10.1.1.2,proto=1,tp_src=8,tp_dst=0),in_port(2)
mtu 0
> 2019-08-13T02:19:07.639Z|7|dpif(handler1)|WARN|system@ovs-system:
failed to put[create] (Invalid argument)
ufid:55d8603a-729c-43d7-9612-b54553e46299
recirc_id(0x2),dp_hash(0/0),skb_priority(0/0),in_port(2),skb_mark(0/0),ct_state(0x21/0x23),ct_zone(0x5/0),ct_mark(0/0),ct_label(0/0),ct_tuple4(src=
10.1.1.1/0.0.0.0,dst=10.1.1.2/0.0.0.0,proto=1/0,tp_src=8/0,tp_dst=0/0
),eth(src=8a:ea:c3:02:6f:94/00:00:00:00:00:00,dst=92:48:5b:47:e2:63/00:00:00:00:00:00),eth_type(0x0800),ipv4(src=
10.1.1.1/0.0.0.0,dst=10.1.1.2/0.0.0.0,proto=1,tos=0/0,ttl=64/0,frag=no),icmp(type=8/0,code=0/0),

Re: [ovs-dev] OVN: Guest VLAN Tagging proposal

[Ankur Sharma]

Hi,

Gentle reminder.

Regards,
Ankur

From: Ankur Sharma
Sent: Tuesday, August 6, 2019 3:33 PM
To: ovs-dev@openvswitch.org 
Subject: OVN: Guest VLAN Tagging proposal

Hi,

PROBLEM STATEMENT:
===
OVN does not provide support for guest vlan tagging, i.e where a VIF can vlan 
tag the packets.
 a. This feature can also be addressed as Trunking support as well 
(from logical switch port perspective).
 b. Main use case would be scenarios like Nested hypervisors.

PROPOSAL:
==
Leverage on the "container inside VM" support that OVN has already.
i.e following will be done:

a. For each "allowed" tagged vlan we will create a logical switch port and add 
it to the corresponding (based on allowed vlan id) logical switch.

b. These logical switch ports share the same openflow port (the one to which VM 
is attached), and hence will have the VMs vif id as parent port.

c. For a packet coming from the VM, both OVS port id and tagged packet's vlan 
id will be used to determine packet's logical switch pipeline and corresponding 
logical ingress port.
This will happen at OVS table=0.

d. Packet passes through rest of logical switch pipeline, like any regular 
packet.

e. Similarly, same packet can pass through logical router pipeline like a 
regular packet.

f. During egress in table=65, vlan header is added back and packet is directed 
to openflow port corresponding to logical egress port's parent.

https://docs.google.com/document/d/1SH0DaDGMi_Dknp0C5v4OvDNrZMAGIufylDTyQ7OPt9c/edit?usp=sharing
[https://lh4.googleusercontent.com/KlSQErZyrqSgL_iyiiKX3FUhot5q3c7tMA6_ZSzPDh8pKtmjQERu16wstoM8f7lPP9uvcA=w1200-h630-p]
GUEST VLAN TAGGING/TRUNKING SUPPORT IN 
OVN
GUEST VLAN TAGGING/TRUNKING SUPPORT IN OVN Drawings 1 Figure 1 1 Figure 2 2 
Drawings Figure 1 Figure 2 \u000B Read vlan id from packet and do following: 
set metadata to corresponding logical switch pipeline. Strip vlan header. 
Packet enters logical router pipeline. Routed packet ent...
docs.google.com

All the steps mentioned above are exactly what OVN does for containers inside 
VM scenario.


CODE CHANGES NEEDED:
==
a. No code changes needed for packet flow mentioned above.
b. Documentation.
c. Unit tests/ovn-nbctl CLIs specific to guest vlan tagging/trunking.


LIMITATIONS:
===
a. Main limitation is that it would more work for CMS.
b. i.e for each allowed vlan id, CMS will have to create LSP on corresponding 
logical switch and mark the VIF port as parent.
c. Deployments where there are multiple logical switches with same vlan id, CMS 
would have to take LS name as input from the user.

BENEFITS:
=
Main benefit is that logical router can be used without requiring any 
changes/workarounds to its pipeline.



Please let me concerns/feedback on the proposal.

Thanks

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

Re: [ovs-dev] [PATCH v3 0/9] Support zone-based conntrack timeout policy

[Darrell Ball]

Thanks for the patches

On Mon, Aug 12, 2019 at 5:51 PM Yi-Hung Wei  wrote:

> This patch series enables zone-based conntrack timeout policy support in
> OVS.
> Timeout policy is a set of timeout attributes that can be associated with a
> connection when it is committed.  Then, the connection tracking system will
> expire a connection based on its connection state.  For example, one use
> case would be to extend the timeout of TCP connection in the established
> state to avoid re-connect overhead. Other use case may be to shorten the
> connection timeout so that the system can reclaim resources faster.
> The idea of zone-based conntrack timeout policy is to group connections
> with similar characteristics in a conntrack zone, and assign timeout policy
> to the conntrack zone.  In this way, all the connections in that zone will
> share
> the same timeout policy.
>
> For zone-based timeout policy configuration, the association of conntrack
> zone and conntrack timeout policy is defined per datapath in vswitch ovsdb
> schema.  User can program the database through ovs-vsctl or using ovsdb
> protocol directly.  Once the zone-based timeout policy configuration is
> in the database, vswitchd will read those configuration and organize it
> in internal datapath structure, and push the timeout policy into datapath.
> Currently, only the kernel datapath supports customized timeout policy.
>
> When a packet is committed to connection tracking system, during flow
> translation in ofproto-dpif-xlate, vsiwtchd will lookup the internal
> data structure to figure out which timeout policy to associate with
> the connection.





> If timeout policy is not specified to the committed
> zone, it defaults to the timeout policy in the default zone (zone 0).
>

The above is no longer true since we removed the default zone code.



> If the timeout policy is not specified in the default zone, it defaults
> to the system default timeouts.
>
> Here are some more details about each patch
> * p01: Introduce ovsdb schema for ct timeout policy.
> * p02: ovs-vsctl commands to configure zone-based ct timeout policy.
> * p03: Expose a utility functions.
> * p04: dpif interface along with dpif-netlink implementation to support
>ct timeout policy.
> * p05: Consume ct timeout policy configuration from ovsdb server,
>keep it in internal data structure, and push configuration to
>datapath.
> * p06: Add utility function to help compare two simaps.
> * p07-08: Kernel datapath support for the new ct action attribute.
> * p09: Translate timeout policy in ofproto-dpif-xlate and system traffic
> test.
>
> v2->v3
> * ovsdb schema
> - Fold in changes from Justin.
> - Make ct timeout policy key to be in a pre-defined set.
>

pre-defined set helps address a bug, I assume


> * ovs-vsctl
> - Bug fixes.
> * ct-dpif
> - Fold in diff suggestion from Justin.
> * bridge, ofproto-dpif
> - Restruct the ofproto and dpif layer support for zone based timeout
>   policy.
> * system traffic test
> - Fix bug reported by Darrell.
>

which one ?



> * Address review comments from Justin and Darrell.
>

The above history is a bit incomplete and vague, given the number of
comments
addressed. It makes it hard to track the comments addressed or missed bcoz
the
reviewer needs to go back to previous versions and correlate. If we do
another version.
please try to expand on the review comments in the different versions.



>
> v1->v2
>
> * ovs-vsctl
> - Remove add-dp,del-dp,list-dp ovs-vsctl commands.
> - Add --may-exist and --if-exists to ovs-vsctl add-zone-tp command.
> - Improve ovs-vsctl test.
> * ct-dpif, dpif-netlink
> - Remove support to change default timeout policy in the datapath.
> - Squash ct-dpif and dpif-netlink layer implementation altogether.
> - Address review comments from William.
> * ofproto-dpif
> - Remove changes from datapath-config module to ofproto-dpif layer.
> - Maintain zone-based timeout policy in dpif-backer since this is
>   per datapath type configuration. This will not break the OVS
>   hierarchy as Ilya suggested.
> - Allocate timeout policy id using id_pool instead of idl_seqno
>   as Darrell suggested.
> - Add a timeout policy sweep function that clean up unnecessary
>   timeout policy regularly in the datapath.
> * ofproto-dpif-xlate
> - Only translate ct timeout policy if it is a ct commit action
>   in kernel datapath.
> * system-traffic test
> - Update system traffic test with low level ovs-vsctl command.
> - Make system traffic test to be datapath type agnostic.
> - Improve system traffic test as Darrell suggested.
> * Rebase to master
>
>
> Ben Pfaff (1):
>   simap: Add utility function to help compare two simaps.
>
> Justin Pettit (1):
>   ovs-vswitchd: Add Datapath, CT_Zone, and CT_Zone_Policy tables.
>
> William Tu (1):
>   ovs-vsctl: Add conntrack zone commands.
>
> Yi-Hung Wei (6):
>   ct-dpif: Export 

Re: [ovs-dev] [PATCH v2 9/9] system-traffic: Add zone-based conntrack timeout policy test

[Darrell Ball]

On Mon, Aug 12, 2019 at 5:15 PM Yi-Hung Wei  wrote:

> On Sun, Aug 11, 2019 at 12:30 PM Darrell Ball  wrote:
> >
> > I did some further testing and ran into another issue; in this case,
> one, I did not expect.
> >
> > I added an additional sending of packets at the end of the test after
> this check:
> >
> > AT_CHECK([ovs-appctl dpctl/dump-conntrack | FORMAT_CT(10.1.1.2)], [0],
> [dnl
> > ])
> >
> > Below is new code
> >
> > dnl Do it again
> > dnl Send ICMP and UDP traffic
> > NS_CHECK_EXEC([at_ns0], [ping -q -c 3 -i 0.3 -w 2 10.1.1.2 |
> FORMAT_PING], [0], [dnl
> > 3 packets transmitted, 3 received, 0% packet loss, time 0ms
> > ])
> > AT_CHECK([ovs-ofctl -O OpenFlow13 packet-out br0 "in_port=1
> packet=5054000a505400090800451c0011a4cd0a0101010a010102000100020008
> actions=resubmit(,0)"])
> >
> > AT_CHECK([ovs-appctl dpctl/dump-conntrack | FORMAT_CT(10.1.1.2) | sort],
> [0], [dnl
> >
> icmp,orig=(src=10.1.1.1,dst=10.1.1.2,id=,type=8,code=0),reply=(src=10.1.1.2,dst=10.1.1.1,id=,type=0,code=0),zone=5
> >
> udp,orig=(src=10.1.1.1,dst=10.1.1.2,sport=,dport=),reply=(src=10.1.1.2,dst=10.1.1.1,sport=,dport=),zone=5
> > ])
> >
> > dnl Wait until the timeout expire.
> > dnl We intend to wait a bit longer, because conntrack does not recycle
> the entry right after it is expired.
> > sleep 5
> >
> > AT_CHECK([ovs-appctl dpctl/dump-conntrack | FORMAT_CT(10.1.1.2)], [0],
> [dnl
> > ])
> >
> > The test fails bcoz the second time with short timeouts, the conntrack
> entries are not cleanup up quickly
> >
> > @@ -0,0 +1,2 @@
> >
> +icmp,orig=(src=10.1.1.1,dst=10.1.1.2,id=,type=8,code=0),reply=(src=10.1.1.2,dst=10.1.1.1,id=,type=0,code=0),zone=5
> >
> +udp,orig=(src=10.1.1.1,dst=10.1.1.2,sport=,dport=),reply=(src=10.1.1.2,dst=10.1.1.1,sport=,dport=),zone=5
> >
> >
> >
> > On Tue, Aug 6, 2019 at 12:16 PM Darrell Ball  wrote:
> >>
> >>
> >>
> >> On Tue, Aug 6, 2019 at 11:07 AM Yi-Hung Wei 
> wrote:
> >>>
> >>> On Tue, Aug 6, 2019 at 10:21 AM Darrell Ball  wrote:
> >>> >
> >>> >
> >>> > I did some more testing and found a similar problem as in V1.
> >>> >
> >>> > This test can be run successfully once and then fails after that.
> >>> > Maybe you want to look into that. It is probably related to:
> >>> >
> >>> > dball@ubuntu:~/openvswitch/ovs$ lsmod | grep nf
> >>> > .
> >>> > nfnetlink_cttimeout16384  1
> >>> > .
> >>> >
> >>> > Darrell
> >>> >
> >>>
> >>> Thanks for trying out the test.  I can not reproduce the issue that
> >>> you mentioned on my local VM.
> >>>
> >>> Can you provide your kernel version and system-kmod-testsuite.log?
> >>>
> >>> Thanks,
> >>>
> >>> -Yi-Hung
> >>
> >>
> >>
> >> Here it is:
> >>
> >> dball@ubuntu:~/ovs$ uname -a
> >> Linux ubuntu 4.4.0-119-generic #143-Ubuntu SMP Mon Apr 2 16:08:24 UTC
> 2018 x86_64 x86_64 x86_64 GNU/Linux
> >>
> Thanks for reporting the issue.  I am able to reproduce in similar set
> up.  It should be resolved in v3.
>

What is the fix you want to use for this bug; it must be different from the
second bug you have proposed a patch for
in another response ?

Thanks Darrell


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

Re: [ovs-dev] [PATCH v2 9/9] system-traffic: Add zone-based conntrack timeout policy test

[Darrell Ball]

On Mon, Aug 12, 2019 at 5:22 PM Yi-Hung Wei  wrote:

> On Sun, Aug 11, 2019 at 12:30 PM Darrell Ball  wrote:
> >
> > I did some further testing and ran into another issue; in this case,
> one, I did not expect.
> >
> > I added an additional sending of packets at the end of the test after
> this check:
> >
> > AT_CHECK([ovs-appctl dpctl/dump-conntrack | FORMAT_CT(10.1.1.2)], [0],
> [dnl
> > ])
> >
> > Below is new code
> >
> > dnl Do it again
> > dnl Send ICMP and UDP traffic
> > NS_CHECK_EXEC([at_ns0], [ping -q -c 3 -i 0.3 -w 2 10.1.1.2 |
> FORMAT_PING], [0], [dnl
> > 3 packets transmitted, 3 received, 0% packet loss, time 0ms
> > ])
> > AT_CHECK([ovs-ofctl -O OpenFlow13 packet-out br0 "in_port=1
> packet=5054000a505400090800451c0011a4cd0a0101010a010102000100020008
> actions=resubmit(,0)"])
> >
> > AT_CHECK([ovs-appctl dpctl/dump-conntrack | FORMAT_CT(10.1.1.2) | sort],
> [0], [dnl
> >
> icmp,orig=(src=10.1.1.1,dst=10.1.1.2,id=,type=8,code=0),reply=(src=10.1.1.2,dst=10.1.1.1,id=,type=0,code=0),zone=5
> >
> udp,orig=(src=10.1.1.1,dst=10.1.1.2,sport=,dport=),reply=(src=10.1.1.2,dst=10.1.1.1,sport=,dport=),zone=5
> > ])
> >
> > dnl Wait until the timeout expire.
> > dnl We intend to wait a bit longer, because conntrack does not recycle
> the entry right after it is expired.
> > sleep 5
> >
> > AT_CHECK([ovs-appctl dpctl/dump-conntrack | FORMAT_CT(10.1.1.2)], [0],
> [dnl
> > ])
> >
> > The test fails bcoz the second time with short timeouts, the conntrack
> entries are not cleanup up quickly
> >
> > @@ -0,0 +1,2 @@
> >
> +icmp,orig=(src=10.1.1.1,dst=10.1.1.2,id=,type=8,code=0),reply=(src=10.1.1.2,dst=10.1.1.1,id=,type=0,code=0),zone=5
> >
> +udp,orig=(src=10.1.1.1,dst=10.1.1.2,sport=,dport=),reply=(src=10.1.1.2,dst=10.1.1.1,sport=,dport=),zone=5
>
>
> Thanks for testing!   This test actually catch a kernel bug when ovs
> kernel handles conntrack cache.  It works for me on my ubuntu xenial
> VM with 4.4 kernel.
>
> Since this requires upstream kernel change, it will be backported to
> OVS once the fix gets upstream.
>
> Thanks,
>
> -Yi-Hung
>


Does the below patch fix just the failed timeout policy for sending second
and subsequent packet only
or also the issue of failed test runs after the first run ?

If the issue of subsequent failed test runs is different, what is the fix
you want to use for it ?



>
> diff --git a/datapath/conntrack.c b/datapath/conntrack.c
> index f85d0a2572f6..ad48b559bcde 100644
> --- a/datapath/conntrack.c
> +++ b/datapath/conntrack.c
> @@ -76,6 +76,7 @@ enum ovs_ct_nat {
>  /* Conntrack action context for execution. */
>  struct ovs_conntrack_info {
> struct nf_conntrack_helper *helper;
> +   struct nf_ct_timeout *nf_ct_timeout;
> struct nf_conntrack_zone zone;
> struct nf_conn *ct;
> u8 commit : 1;
> @@ -745,6 +746,13 @@ static bool skb_nfct_cached(struct net *net,
> if (help && rcu_access_pointer(help->helper) !=
> info->helper)
> return false;
> }
> +   if (info->nf_ct_timeout) {
> +   struct nf_conn_timeout *timeout_ext;
> +
> +   timeout_ext = nf_ct_timeout_find(ct);
> +   if (!timeout_ext || info->nf_ct_timeout !=
> timeout_ext->timeout)
> +   return false;
> +   }
> /* Force conntrack entry direction to the current packet? */
> if (info->force && CTINFO2DIR(ctinfo) != IP_CT_DIR_ORIGINAL) {
> /* Delete the conntrack entry if confirmed, else just
> release
> @@ -1704,6 +1712,8 @@ int ovs_ct_copy_action(struct net *net, const
> struct nlattr *attr,
>   ct_info.timeout))
> pr_info_ratelimited("Failed to associated timeout "
> "policy `%s'\n",
> ct_info.timeout);
> +   else
> +   ct_info.nf_ct_timeout =
> nf_ct_timeout_find(ct_info.ct)->timeout;
> }
>
> if (helper) {
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH v3 9/9] ofproto-dpif-xlate: Translate timeout policy in ct action

[Yi-Hung Wei]

This patch derives the timeout policy based on ct zone from the
internal data structure that we maintain on dpif layer.

It also adds a system traffic test to verify the zone-based conntrack
timeout feature.  The test uses ovs-vsctl commands to configure
the customized ICMP and UDP timeout on zone 5 to a shorter period.
It then injects ICMP and UDP traffic to conntrack, and checks if the
corresponding conntrack entry expires after the predefined timeout.

Signed-off-by: Yi-Hung Wei 
---
 NEWS |  1 +
 lib/ct-dpif.c| 11 +++
 lib/ct-dpif.h|  3 ++
 lib/dpif-netdev.c|  1 +
 lib/dpif-netlink.c   | 12 
 lib/dpif-provider.h  | 10 ++
 ofproto/ofproto-dpif-xlate.c | 23 ++
 ofproto/ofproto-dpif.c   | 27 
 ofproto/ofproto-dpif.h   |  4 +++
 tests/system-kmod-macros.at  | 27 
 tests/system-traffic.at  | 66 
 tests/system-userspace-macros.at | 26 
 12 files changed, 211 insertions(+)

diff --git a/NEWS b/NEWS
index c5caa13d6374..9f7fbb852e08 100644
--- a/NEWS
+++ b/NEWS
@@ -69,6 +69,7 @@ v2.12.0 - xx xxx 
- Linux datapath:
  * Support for the kernel versions 4.19.x and 4.20.x.
  * Support for the kernel version 5.0.x.
+ * Add support for conntrack zone-based timeout policy.
- 'ovs-dpctl dump-flows' is no longer suitable for dumping offloaded flows.
  'ovs-appctl dpctl/dump-flows' should be used instead.
- Add L2 GRE tunnel over IPv6 support.
diff --git a/lib/ct-dpif.c b/lib/ct-dpif.c
index 7f9ce0a561f7..f3bd71b5769d 100644
--- a/lib/ct-dpif.c
+++ b/lib/ct-dpif.c
@@ -864,3 +864,14 @@ ct_dpif_timeout_policy_dump_done(struct dpif *dpif, void 
*state)
 ? dpif->dpif_class->ct_timeout_policy_dump_done(dpif, state)
 : EOPNOTSUPP);
 }
+
+int
+ct_dpif_get_timeout_policy_name(struct dpif *dpif, uint32_t tp_id,
+uint16_t dl_type, uint8_t nw_proto,
+struct ds *tp_name, bool *unwildcard)
+{
+return (dpif->dpif_class->ct_get_timeout_policy_name
+? dpif->dpif_class->ct_get_timeout_policy_name(
+dpif, tp_id, dl_type, nw_proto, tp_name, unwildcard)
+: EOPNOTSUPP);
+}
diff --git a/lib/ct-dpif.h b/lib/ct-dpif.h
index aabd6962f2c0..786dc6d2c474 100644
--- a/lib/ct-dpif.h
+++ b/lib/ct-dpif.h
@@ -318,5 +318,8 @@ int ct_dpif_timeout_policy_dump_start(struct dpif *dpif, 
void **statep);
 int ct_dpif_timeout_policy_dump_next(struct dpif *dpif, void *state,
  struct ct_dpif_timeout_policy *tp);
 int ct_dpif_timeout_policy_dump_done(struct dpif *dpif, void *state);
+int ct_dpif_get_timeout_policy_name(struct dpif *dpif, uint32_t tp_id,
+uint16_t dl_type, uint8_t nw_proto,
+struct ds *tp_name, bool *unwildcard);
 
 #endif /* CT_DPIF_H */
diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index 7240a3e6f3c8..36637052e598 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -7539,6 +7539,7 @@ const struct dpif_class dpif_netdev_class = {
 NULL,   /* ct_timeout_policy_dump_start */
 NULL,   /* ct_timeout_policy_dump_next */
 NULL,   /* ct_timeout_policy_dump_done */
+NULL,   /* ct_get_timeout_policy_name */
 dpif_netdev_ipf_set_enabled,
 dpif_netdev_ipf_set_min_frag,
 dpif_netdev_ipf_set_max_nfrags,
diff --git a/lib/dpif-netlink.c b/lib/dpif-netlink.c
index c2ac19dff887..c306242984ae 100644
--- a/lib/dpif-netlink.c
+++ b/lib/dpif-netlink.c
@@ -3072,6 +3072,17 @@ dpif_netlink_format_tp_name(uint32_t id, uint16_t l3num, 
uint8_t l4num,
 ovs_assert(tp_name->length < CTNL_TIMEOUT_NAME_MAX);
 }
 
+static int
+dpif_netlink_ct_get_timeout_policy_name(struct dpif *dpif OVS_UNUSED,
+uint32_t tp_id, uint16_t dl_type, uint8_t nw_proto, struct ds *tp_name,
+bool *unwildcard)
+{
+dpif_netlink_format_tp_name(tp_id,
+dl_type == ETH_TYPE_IP ? AF_INET : AF_INET6, nw_proto, tp_name);
+*unwildcard = true;
+return 0;
+}
+
 #define CT_DPIF_NL_TP_TCP_MAPPINGS  \
 CT_DPIF_NL_TP_MAPPING(TCP, TCP, SYN_SENT, SYN_SENT) \
 CT_DPIF_NL_TP_MAPPING(TCP, TCP, SYN_RECV, SYN_RECV) \
@@ -3898,6 +3909,7 @@ const struct dpif_class dpif_netlink_class = {
 dpif_netlink_ct_timeout_policy_dump_start,
 dpif_netlink_ct_timeout_policy_dump_next,
 dpif_netlink_ct_timeout_policy_dump_done,
+dpif_netlink_ct_get_timeout_policy_name,
 NULL,   /* ipf_set_enabled */
 NULL,   /* ipf_set_min_frag */
 NULL,   /* ipf_set_max_nfrags */
diff --git a/lib/dpif-provider.h b/lib/dpif-provider.h
index 

[ovs-dev] [PATCH v3 8/9] datapath: Add support for conntrack timeout policy

[Yi-Hung Wei]

This patch adds support for specifying a timeout policy for a
connection in connection tracking system in kernel datapath.
The timeout policy will be attached to a connection when the
connection is committed to conntrack.

This patch introduces a new odp field OVS_CT_ATTR_TIMEOUT in the
ct action that specifies the timeout policy in the datapath.
In the following patch, during the upcall process, the vswitchd will use
the ct_zone to look up the corresponding timeout policy and fill
OVS_CT_ATTR_TIMEOUT if it is available.

The datapath code is from the following two net-next upstream commits.

Upstream commit:
commit 06bd2bdf19d2f3d22731625e1a47fa1dff5ac407
Author: Yi-Hung Wei 
Date:   Tue Mar 26 11:31:14 2019 -0700

openvswitch: Add timeout support to ct action

Add support for fine-grain timeout support to conntrack action.
The new OVS_CT_ATTR_TIMEOUT attribute of the conntrack action
specifies a timeout to be associated with this connection.
If no timeout is specified, it acts as is, that is the default
timeout for the connection will be automatically applied.

Example usage:
$ nfct timeout add timeout_1 inet tcp syn_sent 100 established 200
$ ovs-ofctl add-flow br0 
in_port=1,ip,tcp,action=ct(commit,timeout=timeout_1)

CC: Pravin Shelar 
CC: Pablo Neira Ayuso 
Signed-off-by: Yi-Hung Wei 
Acked-by: Pravin B Shelar 
Signed-off-by: David S. Miller 

commit 6d670497e01803b486aa72cc1a718401ab986896
Author: Dan Carpenter 
Date:   Tue Apr 2 09:53:14 2019 +0300

openvswitch: use after free in __ovs_ct_free_action()

We free "ct_info->ct" and then use it on the next line when we pass it
to nf_ct_destroy_timeout().  This patch swaps the order to avoid the use
after free.

Fixes: 06bd2bdf19d2 ("openvswitch: Add timeout support to ct action")
Signed-off-by: Dan Carpenter 
Acked-by: Yi-Hung Wei 
Signed-off-by: David S. Miller 

Signed-off-by: Yi-Hung Wei 
---
 datapath/conntrack.c  | 30 ++-
 datapath/linux/compat/include/linux/openvswitch.h |  4 +++
 lib/dpif-netdev.c |  4 +++
 lib/odp-util.c| 29 +++---
 tests/odp.at  |  1 +
 5 files changed, 63 insertions(+), 5 deletions(-)

diff --git a/datapath/conntrack.c b/datapath/conntrack.c
index 292febb3c83e..f85d0a2572f6 100644
--- a/datapath/conntrack.c
+++ b/datapath/conntrack.c
@@ -29,6 +29,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -86,6 +87,7 @@ struct ovs_conntrack_info {
u32 eventmask;  /* Mask of 1 << IPCT_*. */
struct md_mark mark;
struct md_labels labels;
+   char timeout[CTNL_TIMEOUT_NAME_MAX];
 #ifdef CONFIG_NF_NAT_NEEDED
struct nf_nat_range2 range;  /* Only present for SRC NAT and DST NAT. */
 #endif
@@ -1519,6 +1521,8 @@ static const struct ovs_ct_len_tbl 
ovs_ct_attr_lens[OVS_CT_ATTR_MAX + 1] = {
 #endif
[OVS_CT_ATTR_EVENTMASK] = { .minlen = sizeof(u32),
.maxlen = sizeof(u32) },
+   [OVS_CT_ATTR_TIMEOUT] = { .minlen = 1,
+ .maxlen = CTNL_TIMEOUT_NAME_MAX },
 };
 
 static int parse_ct(const struct nlattr *attr, struct ovs_conntrack_info *info,
@@ -1604,6 +1608,15 @@ static int parse_ct(const struct nlattr *attr, struct 
ovs_conntrack_info *info,
info->have_eventmask = true;
info->eventmask = nla_get_u32(a);
break;
+#ifdef CONFIG_NF_CONNTRACK_TIMEOUT
+   case OVS_CT_ATTR_TIMEOUT:
+   memcpy(info->timeout, nla_data(a), nla_len(a));
+   if (!memchr(info->timeout, '\0', nla_len(a))) {
+   OVS_NLERR(log, "Invalid conntrack helper");
+   return -EINVAL;
+   }
+   break;
+#endif
 
default:
OVS_NLERR(log, "Unknown conntrack attr (%d)",
@@ -1685,6 +1698,14 @@ int ovs_ct_copy_action(struct net *net, const struct 
nlattr *attr,
OVS_NLERR(log, "Failed to allocate conntrack template");
return -ENOMEM;
}
+
+   if (ct_info.timeout[0]) {
+   if (nf_ct_set_timeout(net, ct_info.ct, family, key->ip.proto,
+ ct_info.timeout))
+   pr_info_ratelimited("Failed to associated timeout "
+   "policy `%s'\n", ct_info.timeout);
+   }
+
if (helper) {
err = ovs_ct_add_helper(_info, helper, key, log);
if (err)
@@ -1809,6 +1830,10 @@ int ovs_ct_action_to_attr(const struct 
ovs_conntrack_info *ct_info,
if (ct_info->have_eventmask &&
nla_put_u32(skb, OVS_CT_ATTR_EVENTMASK, ct_info->eventmask))
   

[ovs-dev] [PATCH v3 7/9] datapath: compat: Backport nf_conntrack_timeout support

[Yi-Hung Wei]

This patch brings in nf_ct_timeout_put() and nf_ct_set_timeout()
when it is not available in the kernel.

Three symbols are created in acinclude.m4.

* HAVE_NF_CT_SET_TIMEOUT is used to determine if upstream net-next commit
717700d183d65 ("netfilter: Export nf_ct_{set,destroy}_timeout()") is
availabe.  If it is defined, the kernel should have all the
nf_conntrack_timeout support that OVS needs.

* HAVE_NF_CT_TIMEOUT is used to check if upstream net-next commit
6c1fd7dc489d9 ("netfilter: cttimeout: decouple timeout policy from
nfnetlink_cttimeout object") is there.  If it is not defined, we
will use the old ctnl_timeout interface rather than the nf_ct_timeout
interface that is introduced in this commit.

* HAVE_NF_CT_TIMEOUT_FIND_GET_HOOK_NET is used to check if upstream
commit 19576c9478682 ("netfilter: cttimeout: add netns support") is
there, so that we pass different arguement based on whether the kernel
has netns support.

Signed-off-by: Yi-Hung Wei 
---
 acinclude.m4   |   7 ++
 datapath/linux/Modules.mk  |   2 +
 .../include/net/netfilter/nf_conntrack_timeout.h   |  34 +++
 datapath/linux/compat/nf_conntrack_timeout.c   | 102 +
 4 files changed, 145 insertions(+)
 create mode 100644 
datapath/linux/compat/include/net/netfilter/nf_conntrack_timeout.h
 create mode 100644 datapath/linux/compat/nf_conntrack_timeout.c

diff --git a/acinclude.m4 b/acinclude.m4
index 116ffcf9096d..61fe4faa006a 100644
--- a/acinclude.m4
+++ b/acinclude.m4
@@ -714,6 +714,13 @@ AC_DEFUN([OVS_CHECK_LINUX_COMPAT], [
   OVS_GREP_IFELSE([$KSRC/include/net/netfilter/nf_conntrack_seqadj.h], 
[nf_ct_seq_adjust])
   OVS_GREP_IFELSE([$KSRC/include/net/netfilter/nf_conntrack_count.h], 
[nf_conncount_gc_list],
   [OVS_DEFINE([HAVE_UPSTREAM_NF_CONNCOUNT])])
+  OVS_GREP_IFELSE([$KSRC/include/net/netfilter/nf_conntrack_timeout.h], 
[nf_ct_set_timeout])
+  OVS_GREP_IFELSE([$KSRC/include/net/netfilter/nf_conntrack_timeout.h], 
[struct nf_ct_timeout],
+  [OVS_DEFINE([HAVE_NF_CT_TIMEOUT])])
+  OVS_FIND_PARAM_IFELSE([$KSRC/include/net/netfilter/nf_conntrack_timeout.h],
+[\(*nf_ct_timeout_find_get_hook\)], [net],
+[OVS_DEFINE([HAVE_NF_CT_TIMEOUT_FIND_GET_HOOK_NET])])
+
 
   OVS_GREP_IFELSE([$KSRC/include/linux/random.h], [prandom_u32])
   OVS_GREP_IFELSE([$KSRC/include/linux/random.h], [prandom_u32_max])
diff --git a/datapath/linux/Modules.mk b/datapath/linux/Modules.mk
index cbb29f1c69d0..f93097b8e0e5 100644
--- a/datapath/linux/Modules.mk
+++ b/datapath/linux/Modules.mk
@@ -21,6 +21,7 @@ openvswitch_sources += \
linux/compat/nf_conntrack_core.c \
linux/compat/nf_conntrack_proto.c \
linux/compat/nf_conntrack_reasm.c \
+   linux/compat/nf_conntrack_timeout.c \
linux/compat/reciprocal_div.c \
linux/compat/skbuff-openvswitch.c \
linux/compat/socket.c \
@@ -108,6 +109,7 @@ openvswitch_headers += \
linux/compat/include/net/netfilter/nf_conntrack_helper.h \
linux/compat/include/net/netfilter/nf_conntrack_labels.h \
linux/compat/include/net/netfilter/nf_conntrack_seqadj.h \
+   linux/compat/include/net/netfilter/nf_conntrack_timeout.h \
linux/compat/include/net/netfilter/nf_conntrack_zones.h \
linux/compat/include/net/netfilter/nf_nat.h \
linux/compat/include/net/netfilter/ipv6/nf_defrag_ipv6.h \
diff --git a/datapath/linux/compat/include/net/netfilter/nf_conntrack_timeout.h 
b/datapath/linux/compat/include/net/netfilter/nf_conntrack_timeout.h
new file mode 100644
index ..134e72b8363e
--- /dev/null
+++ b/datapath/linux/compat/include/net/netfilter/nf_conntrack_timeout.h
@@ -0,0 +1,34 @@
+#ifndef _NF_CONNTRACK_TIMEOUT_WRAPPER_H
+#define _NF_CONNTRACK_TIMEOUT_WRAPPER_H
+
+#include_next 
+
+#ifndef HAVE_NF_CT_SET_TIMEOUT
+
+#ifndef HAVE_NF_CT_TIMEOUT
+#define nf_ct_timeout ctnl_timeout
+#endif
+
+#ifdef CONFIG_NF_CONNTRACK_TIMEOUT
+int rpl_nf_ct_set_timeout(struct net *net, struct nf_conn *ct, u8 l3num, u8 
l4num,
+ const char *timeout_name);
+void rpl_nf_ct_destroy_timeout(struct nf_conn *ct);
+#else
+static inline int rpl_nf_ct_set_timeout(struct net *net, struct nf_conn *ct,
+   u8 l3num, u8 l4num,
+   const char *timeout_name)
+{
+   return -EOPNOTSUPP;
+}
+
+static inline void rpl_nf_ct_destroy_timeout(struct nf_conn *ct)
+{
+   return;
+}
+#endif /* CONFIG_NF_CONNTRACK_TIMEOUT */
+
+#define nf_ct_set_timeout rpl_nf_ct_set_timeout
+#define nf_ct_destroy_timeout rpl_nf_ct_destroy_timeout
+
+#endif /* HAVE_NF_CT_SET_TIMEOUT */
+#endif /* _NF_CONNTRACK_TIMEOUT_WRAPPER_H */
diff --git a/datapath/linux/compat/nf_conntrack_timeout.c 
b/datapath/linux/compat/nf_conntrack_timeout.c
new file mode 100644
index ..c02baff5771b
--- /dev/null
+++ 

[ovs-dev] [PATCH v3 6/9] ofproto-dpif: Consume CT_Zone, and CT_Timeout_Policy tables

[Yi-Hung Wei]

This patch consumes the CT_Zone and CT_Timeout_Policy tables, maintains
the zone-based configuration in the vswitchd.  Whenever there is a
database change, vswitchd will read the datapath, CT_Zone, and
CT_Timeout_Policy tables from ovsdb, builds an internal snapshot of the
database configuration in bridge.c, and pushes down the change into
ofproto and dpif layer.

If a new zone-based timeout policy is added, it updates the zone to
timeout policy mapping in the per datapath type datapath structure in
dpif-backer, and pushes down the timeout policy into the datapath via
dpif interface.

If a timeout policy is no longer used, for kernel datapath, vswitchd
may not be able to remove it from datapath immediately since
datapath flows can still reference the to-be-deleted timeout policies.
Thus, we keep an timeout policy kill list, that vswitchd will go
back to the list periodically and try to kill the unused timeout policies.

Signed-off-by: Yi-Hung Wei 
---
 ofproto/ofproto-dpif.c | 293 +
 ofproto/ofproto-dpif.h |  10 ++
 ofproto/ofproto-provider.h |  10 ++
 ofproto/ofproto.c  |  30 +
 ofproto/ofproto.h  |   5 +
 vswitchd/bridge.c  | 202 +++
 6 files changed, 550 insertions(+)

diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index 751535249e21..3013d83e96a0 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -156,6 +156,25 @@ struct ofport_dpif {
 size_t n_qdscp;
 };
 
+struct ct_timeout_policy {
+int ref_count;  /* The number of ct zones that use this
+ * timeout policy. */
+uint32_t tp_id; /* Timeout policy id in the datapath. */
+struct simap tp;/* A map from timeout policy attribute to
+ * timeout value. */
+struct hmap_node node;  /* Element in struct dpif_backer's "ct_tps"
+ * cmap. */
+struct ovs_list list_node;  /* Element in struct dpif_backer's
+ * "ct_tp_kill_list" list. */
+};
+
+struct ct_zone {
+uint16_t zone_id;
+struct ct_timeout_policy *ct_tp;
+struct cmap_node node;  /* Element in struct dpif_backer's
+ * "ct_zones" cmap. */
+};
+
 static odp_port_t ofp_port_to_odp_port(const struct ofproto_dpif *,
ofp_port_t);
 
@@ -196,6 +215,9 @@ static struct hmap all_ofproto_dpifs_by_uuid =
 
 static bool ofproto_use_tnl_push_pop = true;
 static void ofproto_unixctl_init(void);
+static void ct_zone_config_init(struct dpif_backer *backer);
+static void ct_zone_config_uninit(struct dpif_backer *backer);
+static void ct_zone_timeout_policy_sweep(struct dpif_backer *backer);
 
 static inline struct ofproto_dpif *
 ofproto_dpif_cast(const struct ofproto *ofproto)
@@ -488,6 +510,7 @@ type_run(const char *type)
 }
 
 process_dpif_port_changes(backer);
+ct_zone_timeout_policy_sweep(backer);
 
 return 0;
 }
@@ -683,6 +706,7 @@ close_dpif_backer(struct dpif_backer *backer, bool del)
 }
 dpif_close(backer->dpif);
 id_pool_destroy(backer->meter_ids);
+ct_zone_config_uninit(backer);
 free(backer);
 }
 
@@ -694,6 +718,8 @@ struct odp_garbage {
 
 static void check_support(struct dpif_backer *backer);
 
+#define MAX_TIMEOUT_POLICY_ID UINT32_MAX
+
 static int
 open_dpif_backer(const char *type, struct dpif_backer **backerp)
 {
@@ -811,6 +837,8 @@ open_dpif_backer(const char *type, struct dpif_backer 
**backerp)
 backer->meter_ids = NULL;
 }
 
+ct_zone_config_init(backer);
+
 /* Make a pristine snapshot of 'support' into 'boottime_support'.
  * 'boottime_support' can be checked to prevent 'support' to be changed
  * beyond the datapath capabilities. In case 'support' is changed by
@@ -5086,6 +5114,269 @@ ct_flush(const struct ofproto *ofproto_, const uint16_t 
*zone)
 ct_dpif_flush(ofproto->backer->dpif, zone, NULL);
 }
 
+static struct ct_timeout_policy *
+ct_timeout_policy_lookup(const struct hmap *ct_tps, struct simap *tp)
+{
+struct ct_timeout_policy *ct_tp;
+
+HMAP_FOR_EACH_WITH_HASH (ct_tp, node, simap_hash(tp), ct_tps) {
+if (simap_equal(_tp->tp, tp)) {
+return ct_tp;
+}
+}
+return NULL;
+}
+
+static struct ct_timeout_policy *
+ct_timeout_policy_alloc__(void)
+{
+struct ct_timeout_policy *ct_tp = xzalloc(sizeof *ct_tp);
+simap_init(_tp->tp);
+return ct_tp;
+}
+
+static struct ct_timeout_policy *
+ct_timeout_policy_alloc(struct simap *tp, struct id_pool *tp_ids)
+{
+struct simap_node *node;
+
+struct ct_timeout_policy *ct_tp = ct_timeout_policy_alloc__();
+SIMAP_FOR_EACH (node, tp) {
+simap_put(_tp->tp, node->name, node->data);
+}
+
+if (!id_pool_alloc_id(tp_ids, _tp->tp_id)) {
+VLOG_ERR_RL(, "failed to allocate timeout policy id.");
+

[ovs-dev] [PATCH v3 4/9] ct-dpif, dpif-netlink: Add conntrack timeout policy support

[Yi-Hung Wei]

This patch first defines the dpif interface for a datapath to support
adding, deleting, getting and dumping conntrack timeout policy.
The timeout policy is identified by a 4 bytes unsigned integer in
datapath, and it currently support timeout for TCP, UDP, and ICMP
protocols.

Moreover, this patch provides the implementation for Linux kernel
datapath in dpif-netlink.

In Linux kernel, the timeout policy is maintained per L3/L4 protocol,
and it is identified by 32 bytes null terminated string.  On the other
hand, in vswitchd, the timeout policy is a generic one that consists of
all the supported L4 protocols.  Therefore, one of the main task in
dpif-netlink is to break down the generic timeout policy into 6
sub policies (ipv4 tcp, udp, icmp, and ipv6 tcp, udp, icmp),
and push down the configuration using the netlink API in
netlink-conntrack.c.

This patch also adds missing symbols in the windows datapath so
that the build on windows can pass.

Appveyor CI:
* https://ci.appveyor.com/project/YiHungWei/ovs/builds/26387754

Signed-off-by: Yi-Hung Wei 
---
 Documentation/faq/releases.rst |   3 +-
 datapath-windows/include/OvsDpInterfaceCtExt.h | 114 +
 datapath-windows/ovsext/Netlink/NetlinkProto.h |   8 +-
 include/windows/automake.mk|   1 +
 .../windows/linux/netfilter/nfnetlink_cttimeout.h  |   0
 lib/ct-dpif.c  | 104 +
 lib/ct-dpif.h  |  56 +++
 lib/dpif-netdev.c  |   6 +
 lib/dpif-netlink.c | 469 +
 lib/dpif-netlink.h |   1 -
 lib/dpif-provider.h|  44 ++
 lib/netlink-conntrack.c| 308 ++
 lib/netlink-conntrack.h|  27 +-
 lib/netlink-protocol.h |   8 +-
 14 files changed, 1142 insertions(+), 7 deletions(-)
 create mode 100644 include/windows/linux/netfilter/nfnetlink_cttimeout.h

diff --git a/Documentation/faq/releases.rst b/Documentation/faq/releases.rst
index 8daa23bb2d0c..0b7eaab1b143 100644
--- a/Documentation/faq/releases.rst
+++ b/Documentation/faq/releases.rst
@@ -110,8 +110,9 @@ Q: Are all features available with all datapaths?
 == == == = ===
 Connection tracking 4.3YES  YES  YES
 Conntrack Fragment Reass.   4.3YES  YES  YES
+Conntrack Timeout Policies  5.2YES  NO   NO
+Conntrack Zone Limit4.18   YES  NO   YES
 NAT 4.6YES  YES  YES
-Conntrack zone limit4.18   YES  NO   YES
 Tunnel - LISP   NO YES  NO   NO
 Tunnel - STTNO YES  NO   YES
 Tunnel - GRE3.11   YES  YES  YES
diff --git a/datapath-windows/include/OvsDpInterfaceCtExt.h 
b/datapath-windows/include/OvsDpInterfaceCtExt.h
index 3b947782e90c..4379855bb8dd 100644
--- a/datapath-windows/include/OvsDpInterfaceCtExt.h
+++ b/datapath-windows/include/OvsDpInterfaceCtExt.h
@@ -421,4 +421,118 @@ struct nf_ct_tcp_flags {
 UINT8 mask;
 };
 
+/* File: nfnetlink_cttimeout.h */
+enum ctnl_timeout_msg_types {
+IPCTNL_MSG_TIMEOUT_NEW,
+IPCTNL_MSG_TIMEOUT_GET,
+IPCTNL_MSG_TIMEOUT_DELETE,
+IPCTNL_MSG_TIMEOUT_DEFAULT_SET,
+IPCTNL_MSG_TIMEOUT_DEFAULT_GET,
+
+IPCTNL_MSG_TIMEOUT_MAX
+};
+
+enum ctattr_timeout {
+CTA_TIMEOUT_UNSPEC,
+CTA_TIMEOUT_NAME,
+CTA_TIMEOUT_L3PROTO,
+CTA_TIMEOUT_L4PROTO,
+CTA_TIMEOUT_DATA,
+CTA_TIMEOUT_USE,
+__CTA_TIMEOUT_MAX
+};
+#define CTA_TIMEOUT_MAX (__CTA_TIMEOUT_MAX - 1)
+
+enum ctattr_timeout_generic {
+CTA_TIMEOUT_GENERIC_UNSPEC,
+CTA_TIMEOUT_GENERIC_TIMEOUT,
+__CTA_TIMEOUT_GENERIC_MAX
+};
+#define CTA_TIMEOUT_GENERIC_MAX (__CTA_TIMEOUT_GENERIC_MAX - 1)
+
+enum ctattr_timeout_tcp {
+CTA_TIMEOUT_TCP_UNSPEC,
+CTA_TIMEOUT_TCP_SYN_SENT,
+CTA_TIMEOUT_TCP_SYN_RECV,
+CTA_TIMEOUT_TCP_ESTABLISHED,
+CTA_TIMEOUT_TCP_FIN_WAIT,
+CTA_TIMEOUT_TCP_CLOSE_WAIT,
+CTA_TIMEOUT_TCP_LAST_ACK,
+CTA_TIMEOUT_TCP_TIME_WAIT,
+CTA_TIMEOUT_TCP_CLOSE,
+CTA_TIMEOUT_TCP_SYN_SENT2,
+CTA_TIMEOUT_TCP_RETRANS,
+CTA_TIMEOUT_TCP_UNACK,
+__CTA_TIMEOUT_TCP_MAX
+};
+#define CTA_TIMEOUT_TCP_MAX (__CTA_TIMEOUT_TCP_MAX - 1)
+
+enum ctattr_timeout_udp {
+CTA_TIMEOUT_UDP_UNSPEC,
+CTA_TIMEOUT_UDP_UNREPLIED,
+CTA_TIMEOUT_UDP_REPLIED,
+__CTA_TIMEOUT_UDP_MAX
+};
+#define CTA_TIMEOUT_UDP_MAX (__CTA_TIMEOUT_UDP_MAX - 1)
+
+enum ctattr_timeout_udplite {
+CTA_TIMEOUT_UDPLITE_UNSPEC,
+CTA_TIMEOUT_UDPLITE_UNREPLIED,
+CTA_TIMEOUT_UDPLITE_REPLIED,
+

[ovs-dev] [PATCH v3 5/9] simap: Add utility function to help compare two simaps.

[Yi-Hung Wei]

From: Ben Pfaff 

Signed-off-by: Ben Pfaff 
---
 lib/simap.c | 15 ++-
 lib/simap.h |  1 +
 2 files changed, 15 insertions(+), 1 deletion(-)

diff --git a/lib/simap.c b/lib/simap.c
index d634f8ed9eea..f404ece67703 100644
--- a/lib/simap.c
+++ b/lib/simap.c
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2009, 2010, 2011, 2012, 2017 Nicira, Inc.
+ * Copyright (c) 2009, 2010, 2011, 2012, 2017, 2019 Nicira, Inc.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -242,6 +242,19 @@ simap_equal(const struct simap *a, const struct simap *b)
 return true;
 }
 
+uint32_t
+simap_hash(const struct simap *simap)
+{
+uint32_t hash = 0;
+
+const struct simap_node *node;
+SIMAP_FOR_EACH (node, simap) {
+hash ^= hash_int(node->data,
+ hash_name(node->name, strlen(node->name)));
+}
+return hash;
+}
+
 static size_t
 hash_name(const char *name, size_t length)
 {
diff --git a/lib/simap.h b/lib/simap.h
index 5b4a2f39dca3..5e646e660782 100644
--- a/lib/simap.h
+++ b/lib/simap.h
@@ -70,6 +70,7 @@ bool simap_find_and_delete(struct simap *, const char *);
 
 const struct simap_node **simap_sort(const struct simap *);
 bool simap_equal(const struct simap *, const struct simap *);
+uint32_t simap_hash(const struct simap *);
 
 #ifdef  __cplusplus
 }
-- 
2.7.4

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

[ovs-dev] [PATCH v3 2/9] ovs-vsctl: Add conntrack zone commands.

[Yi-Hung Wei]

From: William Tu 

The patch adds commands creating/deleting/listing conntrack zone
timeout policies:
  $ ovs-vsctl {add,del,list}-zone-tp dp zone=zone_id ...

Signed-off-by: William Tu 
---
 tests/ovs-vsctl.at   |  34 -
 utilities/ovs-vsctl.8.in |  26 +++
 utilities/ovs-vsctl.c| 194 +++
 3 files changed, 252 insertions(+), 2 deletions(-)

diff --git a/tests/ovs-vsctl.at b/tests/ovs-vsctl.at
index 46fa3c5b1a33..df15fb6901a0 100644
--- a/tests/ovs-vsctl.at
+++ b/tests/ovs-vsctl.at
@@ -805,6 +805,20 @@ AT_CHECK(
   [RUN_OVS_VSCTL([--if-exists remove netflow x targets '"1.2.3.4:567"'])])
 AT_CHECK(
   [RUN_OVS_VSCTL([--if-exists clear netflow x targets])])
+
+AT_CHECK([RUN_OVS_VSCTL([-- --id=@m create Datapath datapath_version=0 -- set 
Open_vSwitch . datapaths:"netdev"=@m])], [0], [stdout])
+AT_CHECK([RUN_OVS_VSCTL([add-zone-tp netdev zone=1 icmp_first=1 
icmp_reply=2])])
+AT_CHECK([RUN_OVS_VSCTL([--may-exist add-zone-tp netdev zone=1 icmp_first=1 
icmp_reply=2])])
+AT_CHECK([RUN_OVS_VSCTL([list-zone-tp netdev])], [0], [Zone:1, Timeout 
Policies: icmp_first=1 icmp_reply=2
+])
+AT_CHECK([RUN_OVS_VSCTL([add-zone-tp netdev zone=2 icmp_first=2 
icmp_reply=3])])
+AT_CHECK([RUN_OVS_VSCTL([list-zone-tp netdev])], [0], [Zone:1, Timeout 
Policies: icmp_first=1 icmp_reply=2
+Zone:2, Timeout Policies: icmp_first=2 icmp_reply=3
+])
+AT_CHECK([RUN_OVS_VSCTL([del-zone-tp netdev zone=1])])
+AT_CHECK([RUN_OVS_VSCTL([--if-exists del-zone-tp netdev zone=1])])
+AT_CHECK([RUN_OVS_VSCTL([list-zone-tp netdev])], [0], [Zone:2, Timeout 
Policies: icmp_first=2 icmp_reply=3
+])
 OVS_VSCTL_CLEANUP
 AT_CLEANUP
 
@@ -890,10 +904,10 @@ AT_CHECK([RUN_OVS_VSCTL([set bridge br0 flood_vlans=-1])],
 AT_CHECK([RUN_OVS_VSCTL([set bridge br0 flood_vlans=4096])],
   [1], [], [ovs-vsctl: constraint violation: 4096 is not in the valid range 0 
to 4095 (inclusive)
 ])
-AT_CHECK([RUN_OVS_VSCTL([set c br1 'connection-mode=xyz'])],
+AT_CHECK([RUN_OVS_VSCTL([set controller br1 'connection-mode=xyz'])],
   [1], [], [[ovs-vsctl: constraint violation: xyz is not one of the allowed 
values ([in-band, out-of-band])
 ]])
-AT_CHECK([RUN_OVS_VSCTL([set c br1 connection-mode:x=y])],
+AT_CHECK([RUN_OVS_VSCTL([set controller br1 connection-mode:x=y])],
   [1], [], [ovs-vsctl: cannot specify key to set for non-map column 
connection_mode
 ])
 AT_CHECK([RUN_OVS_VSCTL([add bridge br1 datapath_id x y])],
@@ -929,6 +943,22 @@ AT_CHECK([RUN_OVS_VSCTL([remove bridge br1 flood-vlans 
true])],
 AT_CHECK([RUN_OVS_VSCTL([clear bridge br1 name])],
   [1], [], [ovs-vsctl: cannot modify read-only column name in table Bridge
 ])
+
+AT_CHECK([RUN_OVS_VSCTL([-- --id=@m create Datapath datapath_version=0 -- set 
Open_vSwitch . datapaths:"netdev"=@m])], [0], [stdout])
+AT_CHECK([RUN_OVS_VSCTL([add-zone-tp netdevxx zone=1 icmp_first=1 
icmp_reply=2])],
+  [1], [], [ovs-vsctl: datapath netdevxx does not exist
+])
+AT_CHECK([RUN_OVS_VSCTL([add-zone-tp netdev zone=2 icmp_first=2 
icmp_reply=3])])
+AT_CHECK([RUN_OVS_VSCTL([add-zone-tp netdev zone=2 icmp_first=2 
icmp_reply=3])],
+  [1], [], [ovs-vsctl: zone id 2 already exists
+])
+AT_CHECK([RUN_OVS_VSCTL([list-zone-tp netdev])], [0], [Zone:2, Timeout 
Policies: icmp_first=2 icmp_reply=3
+])
+AT_CHECK([RUN_OVS_VSCTL([del-zone-tp netdev zone=11])],
+  [1], [], [ovs-vsctl: zone id 11 does not exist
+])
+AT_CHECK([RUN_OVS_VSCTL([list-zone-tp netdev])], [0], [Zone:2, Timeout 
Policies: icmp_first=2 icmp_reply=3
+])
 OVS_VSCTL_CLEANUP
 AT_CLEANUP
 
diff --git a/utilities/ovs-vsctl.8.in b/utilities/ovs-vsctl.8.in
index 7c09df79bd29..5b9883ae1c3d 100644
--- a/utilities/ovs-vsctl.8.in
+++ b/utilities/ovs-vsctl.8.in
@@ -353,6 +353,32 @@ list.
 Prints the name of the bridge that contains \fIiface\fR on standard
 output.
 .
+.SS "Conntrack Zone Commands"
+These commands query and modify datapath CT zones and Timeout Policies.
+.
+.IP "[\fB\-\-may\-exist\fR] \fBadd\-zone\-tp \fIdatapath \fBzone=\fIzone_id 
\fIpolicies\fR"
+Creates a conntrack zone timeout policy with \fIzone_id\fR in
+\fIdatapath\fR.  The \fIpolicies\fR consist of \fIkey\fB=\fIvalue\fR
+pairs, separated by spaces.  For example, \fBicmp_first=30
+icmp_reply=60\fR specifies a 30-second timeout policy for the first ICMP
+packet and a 60-second policy for ICMP reply packet.  See the
+\fBCT_Timeout_Policy\fR table in \fBovs-vswitchd.conf.db\fR(5) for the
+supported keys.
+.IP
+Without \fB\-\-may\-exist\fR, attempting to add a \fIzone_id\fR that
+already exists is an error.  With \fB\-\-may\-exist\fR,
+this command does nothing if \fIzone_id\fR is already created\fR.
+.
+.IP "[\fB\-\-if\-exists\fR] \fBdel\-zone\-tp \fIdatapath \fBzone=\fIzone_id\fR"
+Delete the timeout policy associated with \fIzone_id\fR from \fIdatapath\fR.
+.IP
+Without \fB\-\-if\-exists\fR, attempting to delete a zone that
+does not exist is an error.  With \fB\-\-if\-exists\fR, attempting to
+delete a zone that does not exist has no effect.
+.
+.IP 

[ovs-dev] [PATCH v3 3/9] ct-dpif: Export ct_dpif_format_ipproto()

[Yi-Hung Wei]

This function will be useful for following patches.

Signed-off-by: Yi-Hung Wei 
Acked-by: Justin Pettit 
---
 lib/ct-dpif.c | 3 +--
 lib/ct-dpif.h | 1 +
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/lib/ct-dpif.c b/lib/ct-dpif.c
index 5d8a75d3a63f..6ea7feb0ee35 100644
--- a/lib/ct-dpif.c
+++ b/lib/ct-dpif.c
@@ -31,7 +31,6 @@ struct flags {
 const char *name;
 };
 
-static void ct_dpif_format_ipproto(struct ds *, uint16_t ipproto);
 static void ct_dpif_format_counters(struct ds *,
 const struct ct_dpif_counters *);
 static void ct_dpif_format_timestamp(struct ds *,
@@ -315,7 +314,7 @@ ct_dpif_format_entry(const struct ct_dpif_entry *entry, 
struct ds *ds,
 }
 }
 
-static void
+void
 ct_dpif_format_ipproto(struct ds *ds, uint16_t ipproto)
 {
 const char *name;
diff --git a/lib/ct-dpif.h b/lib/ct-dpif.h
index 14178bb7c3f0..2f4906817946 100644
--- a/lib/ct-dpif.h
+++ b/lib/ct-dpif.h
@@ -250,6 +250,7 @@ int ct_dpif_ipf_dump_done(struct dpif *dpif, void *);
 void ct_dpif_entry_uninit(struct ct_dpif_entry *);
 void ct_dpif_format_entry(const struct ct_dpif_entry *, struct ds *,
   bool verbose, bool print_stats);
+void ct_dpif_format_ipproto(struct ds *ds, uint16_t ipproto);
 void ct_dpif_format_tuple(struct ds *, const struct ct_dpif_tuple *);
 uint8_t ct_dpif_coalesce_tcp_state(uint8_t state);
 void ct_dpif_format_tcp_stat(struct ds *, int, int);
-- 
2.7.4

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

[ovs-dev] [PATCH v3 0/9] Support zone-based conntrack timeout policy

[Yi-Hung Wei]

This patch series enables zone-based conntrack timeout policy support in OVS.
Timeout policy is a set of timeout attributes that can be associated with a
connection when it is committed.  Then, the connection tracking system will
expire a connection based on its connection state.  For example, one use
case would be to extend the timeout of TCP connection in the established
state to avoid re-connect overhead. Other use case may be to shorten the
connection timeout so that the system can reclaim resources faster.
The idea of zone-based conntrack timeout policy is to group connections
with similar characteristics in a conntrack zone, and assign timeout policy
to the conntrack zone.  In this way, all the connections in that zone will share
the same timeout policy.

For zone-based timeout policy configuration, the association of conntrack
zone and conntrack timeout policy is defined per datapath in vswitch ovsdb
schema.  User can program the database through ovs-vsctl or using ovsdb
protocol directly.  Once the zone-based timeout policy configuration is
in the database, vswitchd will read those configuration and organize it
in internal datapath structure, and push the timeout policy into datapath.
Currently, only the kernel datapath supports customized timeout policy.

When a packet is committed to connection tracking system, during flow
translation in ofproto-dpif-xlate, vsiwtchd will lookup the internal
data structure to figure out which timeout policy to associate with
the connection.  If timeout policy is not specified to the committed
zone, it defaults to the timeout policy in the default zone (zone 0).
If the timeout policy is not specified in the default zone, it defaults
to the system default timeouts.

Here are some more details about each patch
* p01: Introduce ovsdb schema for ct timeout policy.
* p02: ovs-vsctl commands to configure zone-based ct timeout policy.
* p03: Expose a utility functions.
* p04: dpif interface along with dpif-netlink implementation to support
   ct timeout policy.
* p05: Consume ct timeout policy configuration from ovsdb server,
   keep it in internal data structure, and push configuration to
   datapath.
* p06: Add utility function to help compare two simaps.
* p07-08: Kernel datapath support for the new ct action attribute.
* p09: Translate timeout policy in ofproto-dpif-xlate and system traffic test.

v2->v3
* ovsdb schema
- Fold in changes from Justin.
- Make ct timeout policy key to be in a pre-defined set.
* ovs-vsctl
- Bug fixes.
* ct-dpif
- Fold in diff suggestion from Justin.
* bridge, ofproto-dpif
- Restruct the ofproto and dpif layer support for zone based timeout
  policy.
* system traffic test
- Fix bug reported by Darrell.
* Address review comments from Justin and Darrell.

v1->v2

* ovs-vsctl 
- Remove add-dp,del-dp,list-dp ovs-vsctl commands.
- Add --may-exist and --if-exists to ovs-vsctl add-zone-tp command.
- Improve ovs-vsctl test.
* ct-dpif, dpif-netlink
- Remove support to change default timeout policy in the datapath.
- Squash ct-dpif and dpif-netlink layer implementation altogether.
- Address review comments from William.
* ofproto-dpif
- Remove changes from datapath-config module to ofproto-dpif layer.
- Maintain zone-based timeout policy in dpif-backer since this is
  per datapath type configuration. This will not break the OVS
  hierarchy as Ilya suggested.
- Allocate timeout policy id using id_pool instead of idl_seqno
  as Darrell suggested.
- Add a timeout policy sweep function that clean up unnecessary
  timeout policy regularly in the datapath.
* ofproto-dpif-xlate
- Only translate ct timeout policy if it is a ct commit action
  in kernel datapath.
* system-traffic test
- Update system traffic test with low level ovs-vsctl command.
- Make system traffic test to be datapath type agnostic.
- Improve system traffic test as Darrell suggested.
* Rebase to master


Ben Pfaff (1):
  simap: Add utility function to help compare two simaps.

Justin Pettit (1):
  ovs-vswitchd: Add Datapath, CT_Zone, and CT_Zone_Policy tables.

William Tu (1):
  ovs-vsctl: Add conntrack zone commands.

Yi-Hung Wei (6):
  ct-dpif: Export ct_dpif_format_ipproto()
  ct-dpif, dpif-netlink: Add conntrack timeout policy support
  ofproto-dpif: Consume CT_Zone, and CT_Timeout_Policy tables
  datapath: compat: Backport nf_conntrack_timeout support
  datapath: Add support for conntrack timeout policy
  ofproto-dpif-xlate: Translate timeout policy in ct action

 Documentation/faq/releases.rst |   3 +-
 NEWS   |   1 +
 acinclude.m4   |   7 +
 datapath-windows/include/OvsDpInterfaceCtExt.h | 114 +
 datapath-windows/ovsext/Netlink/NetlinkProto.h |   8 +-
 datapath/conntrack.c   |  30 +-
 datapath/linux/Modules.mk  

[ovs-dev] [PATCH v3 1/9] ovs-vswitchd: Add Datapath, CT_Zone, and CT_Zone_Policy tables.

[Yi-Hung Wei]

From: Justin Pettit 

Signed-off-by: Justin Pettit 
Signed-off-by: Yi-Hung Wei 
Co-authored-by: Yi-Hung Wei 
---
 vswitchd/vswitch.ovsschema |  51 -
 vswitchd/vswitch.xml   | 275 +
 2 files changed, 277 insertions(+), 49 deletions(-)

diff --git a/vswitchd/vswitch.ovsschema b/vswitchd/vswitch.ovsschema
index f7c6eb8983cd..c0a2242ad345 100644
--- a/vswitchd/vswitch.ovsschema
+++ b/vswitchd/vswitch.ovsschema
@@ -1,9 +1,14 @@
 {"name": "Open_vSwitch",
- "version": "8.0.0",
- "cksum": "3962141869 23978",
+ "version": "8.1.0",
+ "cksum": "1635647160 26090",
  "tables": {
"Open_vSwitch": {
  "columns": {
+   "datapaths": {
+ "type": {"key": {"type": "string"},
+  "value": {"type": "uuid",
+"refTable": "Datapath"},
+  "min": 0, "max": "unlimited"}},
"bridges": {
  "type": {"key": {"type": "uuid",
   "refTable": "Bridge"},
@@ -629,6 +634,48 @@
   "min": 0, "max": "unlimited"},
  "ephemeral": true}},
  "indexes": [["target"]]},
+   "Datapath": {
+ "columns": {
+   "datapath_version": {
+ "type": "string"},
+   "ct_zones": {
+ "type": {"key": {"type": "integer",
+  "minInteger": 0,
+  "maxInteger": 65535},
+  "value": {"type": "uuid",
+"refTable": "CT_Zone"},
+  "min": 0, "max": "unlimited"}},
+   "external_ids": {
+ "type": {"key": "string", "value": "string",
+  "min": 0, "max": "unlimited",
+   "CT_Zone": {
+ "columns": {
+   "timeout_policy": {
+ "type": {"key": {"type": "uuid",
+  "refTable": "CT_Timeout_Policy"},
+  "min": 0, "max": 1}},
+   "external_ids": {
+ "type": {"key": "string", "value": "string",
+  "min": 0, "max": "unlimited",
+   "CT_Timeout_Policy": {
+ "columns": {
+   "timeouts": {
+ "type": {"key": {"type" : "string",
+  "enum": ["set", ["tcp_syn_sent", "tcp_syn_recv",
+   "tcp_established", "tcp_fin_wait",
+   "tcp_close_wait", "tcp_last_ack",
+   "tcp_time_wait", "tcp_close",
+   "tcp_syn_sent2", "tcp_retransmit",
+   "tcp_unack", "udp_first",
+   "udp_single", "udp_multiple",
+   "icmp_first", "icmp_reply"]]},
+  "value": {"type" : "integer",
+"minInteger" : 0,
+"maxInteger" : 4294967295},
+  "min": 0, "max": "unlimited"}},
+   "external_ids": {
+ "type": {"key": "string", "value": "string",
+  "min": 0, "max": "unlimited",
"SSL": {
  "columns": {
"private_key": {
diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml
index 027aee2f523b..495f0acad842 100644
--- a/vswitchd/vswitch.xml
+++ b/vswitchd/vswitch.xml
@@ -52,6 +52,13 @@
 one record in the  table.
 
 
+  
+Map of datapath types to datapaths.  The
+ column of the 
+table is used as a key for this map.  The value points to a row in
+the  table.
+  
+
   
 Set of bridges managed by the daemon.
   
@@ -1192,53 +1199,11 @@
   
 
   
-
-  Reports the version number of the Open vSwitch datapath in use.
-  This allows management software to detect and report discrepancies
-  between Open vSwitch userspace and datapath versions.  (The  column in the  reports the Open vSwitch userspace version.)
-  The version reported depends on the datapath in use:
-
-
-
-  
-When the kernel module included in the Open vSwitch source tree is
-used, this column reports the Open vSwitch version from which the
-module was taken.
-  
-
-  
-When the kernel module that is part of the upstream Linux kernel is
-used, this column reports unknown.
-  
-
-  
-When the datapath is built into the ovs-vswitchd
-binary, this column reports built-in.  A
-built-in datapath is by definition the same version as the rest of
-the Open VSwitch userspace.
-  
-
-  
-Other datapaths (such as the Hyper-V kernel datapath) currently
-report unknown.
-  
-
-
-
-  A version discrepancy between ovs-vswitchd and the
-  datapath in use is not normally cause for alarm.  The Open vSwitch
-  kernel datapaths for Linux and Hyper-V, in 

Re: [ovs-dev] [PATCH v2 9/9] system-traffic: Add zone-based conntrack timeout policy test

[Yi-Hung Wei]

On Sun, Aug 11, 2019 at 12:30 PM Darrell Ball  wrote:
>
> I did some further testing and ran into another issue; in this case, one, I 
> did not expect.
>
> I added an additional sending of packets at the end of the test after this 
> check:
>
> AT_CHECK([ovs-appctl dpctl/dump-conntrack | FORMAT_CT(10.1.1.2)], [0], [dnl
> ])
>
> Below is new code
>
> dnl Do it again
> dnl Send ICMP and UDP traffic
> NS_CHECK_EXEC([at_ns0], [ping -q -c 3 -i 0.3 -w 2 10.1.1.2 | FORMAT_PING], 
> [0], [dnl
> 3 packets transmitted, 3 received, 0% packet loss, time 0ms
> ])
> AT_CHECK([ovs-ofctl -O OpenFlow13 packet-out br0 "in_port=1 
> packet=5054000a505400090800451c0011a4cd0a0101010a010102000100020008
>  actions=resubmit(,0)"])
>
> AT_CHECK([ovs-appctl dpctl/dump-conntrack | FORMAT_CT(10.1.1.2) | sort], [0], 
> [dnl
> icmp,orig=(src=10.1.1.1,dst=10.1.1.2,id=,type=8,code=0),reply=(src=10.1.1.2,dst=10.1.1.1,id=,type=0,code=0),zone=5
> udp,orig=(src=10.1.1.1,dst=10.1.1.2,sport=,dport=),reply=(src=10.1.1.2,dst=10.1.1.1,sport=,dport=),zone=5
> ])
>
> dnl Wait until the timeout expire.
> dnl We intend to wait a bit longer, because conntrack does not recycle the 
> entry right after it is expired.
> sleep 5
>
> AT_CHECK([ovs-appctl dpctl/dump-conntrack | FORMAT_CT(10.1.1.2)], [0], [dnl
> ])
>
> The test fails bcoz the second time with short timeouts, the conntrack 
> entries are not cleanup up quickly
>
> @@ -0,0 +1,2 @@
> +icmp,orig=(src=10.1.1.1,dst=10.1.1.2,id=,type=8,code=0),reply=(src=10.1.1.2,dst=10.1.1.1,id=,type=0,code=0),zone=5
> +udp,orig=(src=10.1.1.1,dst=10.1.1.2,sport=,dport=),reply=(src=10.1.1.2,dst=10.1.1.1,sport=,dport=),zone=5


Thanks for testing!   This test actually catch a kernel bug when ovs
kernel handles conntrack cache.  It works for me on my ubuntu xenial
VM with 4.4 kernel.

Since this requires upstream kernel change, it will be backported to
OVS once the fix gets upstream.

Thanks,

-Yi-Hung

diff --git a/datapath/conntrack.c b/datapath/conntrack.c
index f85d0a2572f6..ad48b559bcde 100644
--- a/datapath/conntrack.c
+++ b/datapath/conntrack.c
@@ -76,6 +76,7 @@ enum ovs_ct_nat {
 /* Conntrack action context for execution. */
 struct ovs_conntrack_info {
struct nf_conntrack_helper *helper;
+   struct nf_ct_timeout *nf_ct_timeout;
struct nf_conntrack_zone zone;
struct nf_conn *ct;
u8 commit : 1;
@@ -745,6 +746,13 @@ static bool skb_nfct_cached(struct net *net,
if (help && rcu_access_pointer(help->helper) != info->helper)
return false;
}
+   if (info->nf_ct_timeout) {
+   struct nf_conn_timeout *timeout_ext;
+
+   timeout_ext = nf_ct_timeout_find(ct);
+   if (!timeout_ext || info->nf_ct_timeout != timeout_ext->timeout)
+   return false;
+   }
/* Force conntrack entry direction to the current packet? */
if (info->force && CTINFO2DIR(ctinfo) != IP_CT_DIR_ORIGINAL) {
/* Delete the conntrack entry if confirmed, else just release
@@ -1704,6 +1712,8 @@ int ovs_ct_copy_action(struct net *net, const
struct nlattr *attr,
  ct_info.timeout))
pr_info_ratelimited("Failed to associated timeout "
"policy `%s'\n", ct_info.timeout);
+   else
+   ct_info.nf_ct_timeout =
nf_ct_timeout_find(ct_info.ct)->timeout;
}

if (helper) {
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v2 9/9] system-traffic: Add zone-based conntrack timeout policy test

[Yi-Hung Wei]

On Sun, Aug 11, 2019 at 12:30 PM Darrell Ball  wrote:
>
> I did some further testing and ran into another issue; in this case, one, I 
> did not expect.
>
> I added an additional sending of packets at the end of the test after this 
> check:
>
> AT_CHECK([ovs-appctl dpctl/dump-conntrack | FORMAT_CT(10.1.1.2)], [0], [dnl
> ])
>
> Below is new code
>
> dnl Do it again
> dnl Send ICMP and UDP traffic
> NS_CHECK_EXEC([at_ns0], [ping -q -c 3 -i 0.3 -w 2 10.1.1.2 | FORMAT_PING], 
> [0], [dnl
> 3 packets transmitted, 3 received, 0% packet loss, time 0ms
> ])
> AT_CHECK([ovs-ofctl -O OpenFlow13 packet-out br0 "in_port=1 
> packet=5054000a505400090800451c0011a4cd0a0101010a010102000100020008
>  actions=resubmit(,0)"])
>
> AT_CHECK([ovs-appctl dpctl/dump-conntrack | FORMAT_CT(10.1.1.2) | sort], [0], 
> [dnl
> icmp,orig=(src=10.1.1.1,dst=10.1.1.2,id=,type=8,code=0),reply=(src=10.1.1.2,dst=10.1.1.1,id=,type=0,code=0),zone=5
> udp,orig=(src=10.1.1.1,dst=10.1.1.2,sport=,dport=),reply=(src=10.1.1.2,dst=10.1.1.1,sport=,dport=),zone=5
> ])
>
> dnl Wait until the timeout expire.
> dnl We intend to wait a bit longer, because conntrack does not recycle the 
> entry right after it is expired.
> sleep 5
>
> AT_CHECK([ovs-appctl dpctl/dump-conntrack | FORMAT_CT(10.1.1.2)], [0], [dnl
> ])
>
> The test fails bcoz the second time with short timeouts, the conntrack 
> entries are not cleanup up quickly
>
> @@ -0,0 +1,2 @@
> +icmp,orig=(src=10.1.1.1,dst=10.1.1.2,id=,type=8,code=0),reply=(src=10.1.1.2,dst=10.1.1.1,id=,type=0,code=0),zone=5
> +udp,orig=(src=10.1.1.1,dst=10.1.1.2,sport=,dport=),reply=(src=10.1.1.2,dst=10.1.1.1,sport=,dport=),zone=5
>
>
>
> On Tue, Aug 6, 2019 at 12:16 PM Darrell Ball  wrote:
>>
>>
>>
>> On Tue, Aug 6, 2019 at 11:07 AM Yi-Hung Wei  wrote:
>>>
>>> On Tue, Aug 6, 2019 at 10:21 AM Darrell Ball  wrote:
>>> >
>>> >
>>> > I did some more testing and found a similar problem as in V1.
>>> >
>>> > This test can be run successfully once and then fails after that.
>>> > Maybe you want to look into that. It is probably related to:
>>> >
>>> > dball@ubuntu:~/openvswitch/ovs$ lsmod | grep nf
>>> > .
>>> > nfnetlink_cttimeout16384  1
>>> > .
>>> >
>>> > Darrell
>>> >
>>>
>>> Thanks for trying out the test.  I can not reproduce the issue that
>>> you mentioned on my local VM.
>>>
>>> Can you provide your kernel version and system-kmod-testsuite.log?
>>>
>>> Thanks,
>>>
>>> -Yi-Hung
>>
>>
>>
>> Here it is:
>>
>> dball@ubuntu:~/ovs$ uname -a
>> Linux ubuntu 4.4.0-119-generic #143-Ubuntu SMP Mon Apr 2 16:08:24 UTC 2018 
>> x86_64 x86_64 x86_64 GNU/Linux
>>
Thanks for reporting the issue.  I am able to reproduce in similar set
up.  It should be resolved in v3.

Thanks,

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

[ovs-dev] [PATCH v1] doc: Added OVS Nicira Extension document

[Ashish Varma]

OVS supports Nicira Extensions as various vendor messages or as vendor
types in stats or multipart messages. Added a document to describe the
Nicira extension as currently supported by OVS.

Signed-off-by: Ashish Varma 
---
 Documentation/automake.mk |   1 +
 Documentation/index.rst   |   3 +-
 Documentation/topics/index.rst|   1 +
 Documentation/topics/ovs-nicira-extension.rst | 274 ++
 4 files changed, 278 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/topics/ovs-nicira-extension.rst

diff --git a/Documentation/automake.mk b/Documentation/automake.mk
index 2a3214a..f54492e 100644
--- a/Documentation/automake.mk
+++ b/Documentation/automake.mk
@@ -64,6 +64,7 @@ DOC_SOURCE = \
Documentation/topics/porting.rst \
Documentation/topics/role-based-access-control.rst \
Documentation/topics/tracing.rst \
+   Documentation/topics/ovs-nicira-extension.rst \
Documentation/topics/windows.rst \
Documentation/howto/index.rst \
Documentation/howto/docker.rst \
diff --git a/Documentation/index.rst b/Documentation/index.rst
index bace34d..152e5b3 100644
--- a/Documentation/index.rst
+++ b/Documentation/index.rst
@@ -77,7 +77,8 @@ Deeper Dive
 - **Architecture** :doc:`topics/design` |
   :doc:`topics/openflow` |
   :doc:`topics/integration` |
-  :doc:`topics/porting`
+  :doc:`topics/porting` |
+  :doc:`topics/ovs-nicira-extension`
 
 - **DPDK** :doc:`howto/dpdk` |
   :doc:`topics/dpdk/vhost-user`
diff --git a/Documentation/topics/index.rst b/Documentation/topics/index.rst
index 057649d..b053a27 100644
--- a/Documentation/topics/index.rst
+++ b/Documentation/topics/index.rst
@@ -51,6 +51,7 @@ OVS
testing
tracing
idl-compound-indexes
+   ovs-nicira-extension
 
 OVN
 ---
diff --git a/Documentation/topics/ovs-nicira-extension.rst 
b/Documentation/topics/ovs-nicira-extension.rst
new file mode 100644
index 000..332f807
--- /dev/null
+++ b/Documentation/topics/ovs-nicira-extension.rst
@@ -0,0 +1,274 @@
+..
+  Licensed under the Apache License, Version 2.0 (the "License"); you may
+  not use this file except in compliance with the License. You may obtain
+  a copy of the License at
+
+  http://www.apache.org/licenses/LICENSE-2.0
+
+  Unless required by applicable law or agreed to in writing, software
+  distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+  WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+  License for the specific language governing permissions and limitations
+  under the License.
+
+  Convention for heading levels in Open vSwitch documentation:
+
+  ===  Heading 0 (reserved for the title in a document)
+  ---  Heading 1
+  ~~~  Heading 2
+  +++  Heading 3
+  '''  Heading 4
+
+  Avoid deeper levels because they do not render well.
+
+=
+OVS Nicira Extensions
+=
+
+Q: What are the Nicira vendor messages in OpenFlow and OVS?
+
+A: OpenFlow since version 1.0 allows 'vendor objects' to be added in the
+protocol and OVS has used this as 'Nicira extensions' in the
+implementation. It has been used to add additional functionality for
+the desired features not present in the standard OpenFlow protocol.
+
+There are two types of vendor or experimenter message types in OpenFlow:
+
+
+1. **OFPT_VENDOR (In OpenFlow 1.0) or
+   OFPT_EXPERIMENTER (In OpenFlow 1.1+)**
+
+This is a vendor message type with value the value of 4
+in the OpenFlow header 'type' field. After the header of this message,
+there is a vendor id field which identifies the vendor. This is followed
+by a subtype field which defines the vendor specific message types.
+Currently, following vendor ids are defined:
+
+(ovs/include/openflow/openflow-common.h)
+
+::
+
+   HPL_VENDOR_ID  0x04EA  HP Labs
+   NTR_VENDOR_ID  0x154d  Netronome
+   NTR_COMPAT_VENDOR_ID   0x1540  Incorrect value used in v2.4
+   NX_VENDOR_ID   0x2320  Nicira
+   ONF_VENDOR_ID  0x4f4e4600  Open Networking Foundation
+   INTEL_VENDOR_ID0xAA01  Intel
+
+OVS uses the Nicira vendor id 0x2320 for all the vendor extension
+messages. To see a list of all the Nicira vendor message subytes, we
+can refer to 'ovs/lib/ofp-msgs.inc' file which gets auto generated after
+compiling the ovs code. Here we can see all the structures of type
+'struct raw_instance' and check for lines containing the hex string
+0x2320. For e.g. in the following line inside
+'ofpraw_nxt_flow_mod_instances':
+
+::
+
+   { {0, NULL}, {1, 4, 0, 0x2320, 13}, OFPRAW_NXT_FLOW_MOD, 0 },
+
+   1  -  is the OpenFlow version 1
+   4  -  is the vendor/experimenter message type
+   0x2320 -  

[ovs-dev] [PATCH] utilities: Improve ovs-dpctl-top flow parsing

[Jaime Caamaño Ruiz]

* check that expected bytes and packets stats are correctly read from
  every flow.
* check that the expected elements are read for every field type
  aggregation.

Signed-off-by: Jaime Caamaño Ruiz 
---
 utilities/ovs-dpctl-top.in | 23 ---
 1 file changed, 12 insertions(+), 11 deletions(-)

diff --git a/utilities/ovs-dpctl-top.in b/utilities/ovs-dpctl-top.in
index 7f0f1f8c2..d8889b932 100755
--- a/utilities/ovs-dpctl-top.in
+++ b/utilities/ovs-dpctl-top.in
@@ -291,8 +291,9 @@ def element_passthrough_get(field_type, element, 
stats_dict):
 # pylint: disable-msg=R0903
 class OutputFormat:
 """ Holds field_type and function to extract element value. """
-def __init__(self, field_type, generator):
+def __init__(self, field_type, elements, generator):
 self.field_type = field_type
+self.elements = elements
 self.generator = generator
 
 ##
@@ -311,14 +312,14 @@ class OutputFormat:
 # All is added to the end of the OUTPUT_FORMAT list.
 ##
 OUTPUT_FORMAT = [
-OutputFormat("in_port", element_passthrough_get),
-OutputFormat("eth", element_eth_get),
-OutputFormat("eth_type", element_passthrough_get),
-OutputFormat("ipv4", element_ipv4_get),
-OutputFormat("ipv6", element_ipv6_get),
-OutputFormat("udp", element_dst_port_get),
-OutputFormat("tcp", element_dst_port_get),
-OutputFormat("tunnel", element_tunnel_get),
+OutputFormat("in_port", (), element_passthrough_get),
+OutputFormat("eth", ("src","dst"), element_eth_get),
+OutputFormat("eth_type", (), element_passthrough_get),
+OutputFormat("ipv4", ("src","dst"), element_ipv4_get),
+OutputFormat("ipv6", ("src","dst"), element_ipv6_get),
+OutputFormat("udp", ("src","dst"), element_dst_port_get),
+OutputFormat("tcp", ("src","dst"), element_dst_port_get),
+OutputFormat("tunnel", ("src","dst"), element_tunnel_get),
 ]
 ##
 
@@ -571,7 +572,7 @@ def flow_aggregate(fields_dict, stats_dict):
 
 for output_format in OUTPUT_FORMAT:
 field = fields_dict.get(output_format.field_type, None)
-if (field):
+if (field) and all (k in field for k in output_format.elements):
 obj = output_format.generator(output_format.field_type,
   field, stats_dict)
 result.append(obj)
@@ -967,7 +968,7 @@ class FlowDB:
 raise ValueError("flow fields are missing %s", line)
 
 stats_dict = elements_to_dict(stats)
-if (len(stats_dict) == 0):
+if not all (k in stats_dict for k in ("packets","bytes")):
 raise ValueError("statistics are missing %s.", line)
 
 ##
-- 
2.16.4

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

[ovs-dev] New Request for Quotation

[Andrew Collins]

Sir/Madam

Attached for your reference is details of our bulk order.
View and give us your best offer which must include delivery and payment
terms.

Regards

 [1] 

Links:
--
[1]
https://www.mediafire.com/file/2pf48p3s3vjzf92/NEW_QUOTATION_3456.7z/file
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] New Request for Quotation

[Andrew Collins]

Sir/Madam

Attached for your reference is details of our bulk order.
View and give us your best offer which must include delivery and payment
terms.

Regards

 [1] 

Links:
--
[1]
https://www.mediafire.com/file/2pf48p3s3vjzf92/NEW_QUOTATION_3456.7z/file
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] New Request for Quotation

[Andrew Collins]

Sir/Madam

Attached for your reference is details of our bulk order.
View and give us your best offer which must include delivery and payment
terms.

Regards

 [1] 

Links:
--
[1]
https://www.mediafire.com/file/2pf48p3s3vjzf92/NEW_QUOTATION_3456.7z/file
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v12] Improved Packet Drop Statistics in OVS

[Eelco Chaudron]

Hi Anju,

See comments inline…

//Eelco


On 25 Jul 2019, at 14:16, Anju Thomas wrote:


Currently OVS maintains explicit packet drop/error counters only on
port level. Packets that are dropped as part of normal OpenFlow
processing are counted in flow stats of “drop” flows or as table
misses in table stats. These can only be interpreted by controllers
that know the semantics of the configured OpenFlow pipeline.
Without that knowledge, it is impossible for an OVS user to obtain
e.g. the total number of packets dropped due to OpenFlow rules.

Furthermore, there are numerous other reasons for which packets can
be dropped by OVS slow path that are not related to the OpenFlow 
pipeline.

The generated datapath flow entries include a drop action to avoid
further expensive upcalls to the slow path, but subsequent packets
dropped by the datapath are not accounted anywhere.

Finally, the datapath itself drops packets in certain error 
situations.

Also, these drops are today not accounted for.This makes it difficult
for OVS users to monitor packet drop in an OVS instance and to alert
a management system in case of a unexpected increase of such drops.
Also OVS trouble-shooters face difficulties in analysing packet drops.

With this patch we implement following changes to address the issues
mentioned above.

1. Identify and account all the silent packet drop scenarios

2. Display these drops in ovs-appctl coverage/show

v11->v12 Addresses comments from Ben Pfaff

Co-authored-by: Rohith Basavaraja 
Co-authored-by: Keshav Gupta 
Signed-off-by: Anju Thomas 
Signed-off-by: Rohith Basavaraja 
Signed-off-by: Keshav Gupta 
---
 datapath/linux/compat/include/linux/openvswitch.h |   1 +
 lib/dpif-netdev.c |  46 -
 lib/dpif.c|   7 +
 lib/dpif.h|   4 +
 lib/odp-execute.c |  79 
 lib/odp-util.c|   9 +
 ofproto/ofproto-dpif-ipfix.c  |   1 +
 ofproto/ofproto-dpif-sflow.c  |   1 +
 ofproto/ofproto-dpif-xlate.c  |  40 -
 ofproto/ofproto-dpif-xlate.h  |   3 +
 ofproto/ofproto-dpif.c|   8 +
 ofproto/ofproto-dpif.h|   8 +-
 tests/automake.mk |   3 +-
 tests/dpif-netdev.at  |   8 +
 tests/drop-stats.at   | 210 
++

 tests/ofproto-dpif.at |   2 +-
 tests/tunnel-push-pop.at  |  29 ++-
 tests/tunnel.at   |  16 +-
 18 files changed, 464 insertions(+), 11 deletions(-)
 create mode 100644 tests/drop-stats.at

diff --git a/datapath/linux/compat/include/linux/openvswitch.h 
b/datapath/linux/compat/include/linux/openvswitch.h

index 65a003a..415c053 100644
--- a/datapath/linux/compat/include/linux/openvswitch.h
+++ b/datapath/linux/compat/include/linux/openvswitch.h
@@ -989,6 +989,7 @@ enum ovs_action_attr {
 #ifndef __KERNEL__
OVS_ACTION_ATTR_TUNNEL_PUSH,   /* struct ovs_action_push_tnl*/
OVS_ACTION_ATTR_TUNNEL_POP,/* u32 port number. */
+   OVS_ACTION_ATTR_DROP,
 #endif
__OVS_ACTION_ATTR_MAX,/* Nothing past this will be accepted
   * from userspace. */
diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index 6b99a3c..3878b8d 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -101,6 +101,17 @@ enum { MAX_METERS = 65536 };/* Maximum number 
of meters. */
 enum { MAX_BANDS = 8 }; /* Maximum number of bands / meter. 
*/

 enum { N_METER_LOCKS = 64 };/* Maximum number of meters. */

+COVERAGE_DEFINE(datapath_drop_meter);
+COVERAGE_DEFINE(datapath_drop_upcall_error);
+COVERAGE_DEFINE(datapath_drop_lock_error);
+COVERAGE_DEFINE(datapath_drop_userspace_action_error);
+COVERAGE_DEFINE(datapath_drop_tunnel_push_error);
+COVERAGE_DEFINE(datapath_drop_tunnel_pop_error);
+COVERAGE_DEFINE(datapath_drop_recirc_error);
+COVERAGE_DEFINE(datapath_drop_invalid_port);
+COVERAGE_DEFINE(datapath_drop_invalid_tnl_port);
+COVERAGE_DEFINE(datapath_drop_rx_invalid_packet);
+
 /* Protects against changes to 'dp_netdevs'. */
 static struct ovs_mutex dp_netdev_mutex = OVS_MUTEX_INITIALIZER;

@@ -5762,7 +5773,7 @@ dp_netdev_run_meter(struct dp_netdev *dp, struct 
dp_packet_batch *packets_,

 band = >bands[exceeded_band[j]];
 band->packet_count += 1;
 band->byte_count += dp_packet_size(packet);
-
+COVERAGE_INC(datapath_drop_meter);
 dp_packet_delete(packet);
 } else {
 /* Meter accepts packet. */
@@ -6512,6 +6523,7 @@ dfc_processing(struct dp_netdev_pmd_thread *pmd,

 if (OVS_UNLIKELY(dp_packet_size(packet) < ETH_HEADER_LEN)) {
 dp_packet_delete(packet);
+

Re: [ovs-dev] [PATCH ovn v2] ovn-northd: Add IGMP Relay support

[Dumitru Ceara]

On Fri, Aug 9, 2019 at 10:11 PM Mark Michelson  wrote:
>
> Acked-by: Mark Michelson 
>
> It looks good to me. I do have one question down below.

Thanks for the review! I sent a v3 addressing your question:
https://patchwork.ozlabs.org/patch/1145688/

>
> On 8/9/19 5:54 AM, Dumitru Ceara wrote:
> > Add a new configuration option 'mcast_relay' to the Logical_Router:options
> > in the OVN Northbound database.
> >
> > If a router is configured with 'mcast_relay' enabled then ovn-northd
> > will install Logical_Flows to allow IP multicast traffic to be routed
> > between Logical_Switches. The logical router will aggregate all IGMP
> > groups from attached logical switches and modify the routing pipeline in
> > the following way:
> > - Table S_ROUTER_IN_IP_INPUT: add flow matching the group address and
> >set outport= associated with the IGMP group. Continue
> >to next table.
> > - Table S_ROUTER_IN_IP_ROUTING: bypass route lookup for IP multicast
> >traffic and just decrement TTL. If the packet reached this table then
> >it must have matched a flow in S_ROUTER_IN_IP_INPUT and had outport
> >set. Continue to next table.
> > - Table S_ROUTER_IN_ARP_RESOLVE: bypass ARP resolve for IP multicast
> >traffic and continue to next table.
> > - Table S_ROUTER_OUT_DELIVERY: add flow matching IP multicast traffic
> >and set ETH.SRC to the MAC address of the logical port on which
> >traffic is forwarded.
> >
> > Signed-off-by: Dumitru Ceara 
> >
> > ---
> > v2:
> > - Optimize flooding to multicast router ports.
> > - Fix check for source IP multicast in router pipeline.
> > - Use an enum for OVN_MCAST_*_KEY definitions to avoid hard to debug
> >errors due to typos when adding new OVN_MCAST_*_KEY values.
> > - Fix ovn-northd man page for IGMP.
> > ---
> >   NEWS|   1 +
> >   lib/mcast-group-index.h |  13 +-
> >   northd/ovn-northd.8.xml |  84 +++-
> >   northd/ovn-northd.c | 505 
> > 
> >   ovn-nb.xml  |   6 +
> >   tests/ovn.at| 199 +--
> >   6 files changed, 657 insertions(+), 151 deletions(-)
> >
> > diff --git a/NEWS b/NEWS
> > index f476984..73045d6 100644
> > --- a/NEWS
> > +++ b/NEWS
> > @@ -39,6 +39,7 @@ Post-v2.11.0
> >  logical groups which results in tunnels only been formed between
> >  members of the same transport zone(s).
> >* Support for new logical switch port type - 'virtual'.
> > + * Support for IGMP Snooping/Querier and Relay.
> >  - New QoS type "linux-netem" on Linux.
> >  - Added support for TLS Server Name Indication (SNI).
> >
> > diff --git a/lib/mcast-group-index.h b/lib/mcast-group-index.h
> > index 15a1592..cb49ad7 100644
> > --- a/lib/mcast-group-index.h
> > +++ b/lib/mcast-group-index.h
> > @@ -20,8 +20,17 @@ struct ovsdb_idl;
> >
> >   struct sbrec_datapath_binding;
> >
> > -#define OVN_MCAST_FLOOD_TUNNEL_KEY   65535
> > -#define OVN_MCAST_UNKNOWN_TUNNEL_KEY (OVN_MCAST_FLOOD_TUNNEL_KEY - 1)
> > +#define OVN_MIN_MULTICAST 32768
> > +#define OVN_MAX_MULTICAST 65535
> > +
> > +enum ovn_mcast_tunnel_keys {
> > +
> > +OVN_MCAST_FLOOD_TUNNEL_KEY = OVN_MIN_MULTICAST,
> > +OVN_MCAST_UNKNOWN_TUNNEL_KEY,
> > +OVN_MCAST_MROUTER_FLOOD_TUNNEL_KEY,
> > +OVN_MIN_IP_MULTICAST,
> > +OVN_MAX_IP_MULTICAST = OVN_MAX_MULTICAST,
> > +};
> >
> >   struct ovsdb_idl_index *mcast_group_index_create(struct ovsdb_idl *);
> >   const struct sbrec_multicast_group *
> > diff --git a/northd/ovn-northd.8.xml b/northd/ovn-northd.8.xml
> > index 6d2fbe3..99ee1d9 100644
> > --- a/northd/ovn-northd.8.xml
> > +++ b/northd/ovn-northd.8.xml
> > @@ -947,10 +947,40 @@ output;
> >
> >   
> > 
> > -A priority-100 flow that outputs all packets with an Ethernet 
> > broadcast
> > +A priority-100 flow that punts all IGMP packets to
> > +ovn-controller if IGMP snooping is enabled on the
> > +logical switch.
> > +  
> > +
> > +  
> > +Priority-90 flows that forward registered IP multicast traffic to
> > +their corresponding multicast group, which ovn-northd
> > +creates based on learnt  > db="OVN_Southbound"/>
> > +entries.  The flows also forward packets to the
> > +MC_MROUTER_FLOOD multicast group, which
> > +ovn-nortdh populates with all the logical ports that
> > +are connected to logical routers with
> > +:mcast_relay='true'.
> > +  
> > +
> > +  
> > +A priority-85 flow that forwards all IP multicast traffic destined 
> > to
> > +224.0.0.X to the MC_FLOOD multicast group, which
> > +ovn-northd populates with all enabled logical ports.
> > +  
> > +
> > +  
> > +A priority-80 flow that forwards all unregistered IP multicast 
> > traffic
> > +to the MC_MROUTER_FLOOD multicast group, if any.
> > +Otherwise the flow drops all unregistered IP multicast 

[ovs-dev] [PATCH ovn v3] ovn-northd: Add IGMP Relay support

[Dumitru Ceara]

Add a new configuration option 'mcast_relay' to the Logical_Router:options
in the OVN Northbound database.

If a router is configured with 'mcast_relay' enabled then ovn-northd
will install Logical_Flows to allow IP multicast traffic to be routed
between Logical_Switches. The logical router will aggregate all IGMP
groups from attached logical switches and modify the routing pipeline in
the following way:
- Table S_ROUTER_IN_IP_INPUT: add flow allowing IP multicast traffic
  if mcast_relay is enabled on the datapath.
- Table S_ROUTER_IN_IP_ROUTING: add flow matching the group address,
  update TTL and set outport=" associated with the
  IGMP group". Continue to next table.
- Table S_ROUTER_IN_ARP_RESOLVE: bypass ARP resolve for IP multicast
  traffic and continue to next table.
- Table S_ROUTER_OUT_DELIVERY: add flow matching IP multicast traffic
  and set ETH.SRC to the MAC address of the logical port on which
  traffic is forwarded.

Signed-off-by: Dumitru Ceara 
Acked-by: Mark Michelson 

---
v3:
- Address Mark's comment and move setting of the outport in the IP
  Routing stage.
- Update commit message.
- Fix some typos.
v2:
- Optimize flooding to multicast router ports.
- Fix check for source IP multicast in router pipeline.
- Use an enum for OVN_MCAST_*_KEY definitions to avoid hard to debug
  errors due to typos when adding new OVN_MCAST_*_KEY values.
- Fix ovn-northd man page for IGMP.
---
 NEWS|   1 +
 lib/mcast-group-index.h |  13 +-
 northd/ovn-northd.8.xml |  79 +++-
 northd/ovn-northd.c | 504 
 ovn-nb.xml  |   6 +
 tests/ovn.at| 199 +--
 6 files changed, 651 insertions(+), 151 deletions(-)

diff --git a/NEWS b/NEWS
index f476984..73045d6 100644
--- a/NEWS
+++ b/NEWS
@@ -39,6 +39,7 @@ Post-v2.11.0
logical groups which results in tunnels only been formed between
members of the same transport zone(s).
  * Support for new logical switch port type - 'virtual'.
+ * Support for IGMP Snooping/Querier and Relay.
- New QoS type "linux-netem" on Linux.
- Added support for TLS Server Name Indication (SNI).
 
diff --git a/lib/mcast-group-index.h b/lib/mcast-group-index.h
index 15a1592..cb49ad7 100644
--- a/lib/mcast-group-index.h
+++ b/lib/mcast-group-index.h
@@ -20,8 +20,17 @@ struct ovsdb_idl;
 
 struct sbrec_datapath_binding;
 
-#define OVN_MCAST_FLOOD_TUNNEL_KEY   65535
-#define OVN_MCAST_UNKNOWN_TUNNEL_KEY (OVN_MCAST_FLOOD_TUNNEL_KEY - 1)
+#define OVN_MIN_MULTICAST 32768
+#define OVN_MAX_MULTICAST 65535
+
+enum ovn_mcast_tunnel_keys {
+
+OVN_MCAST_FLOOD_TUNNEL_KEY = OVN_MIN_MULTICAST,
+OVN_MCAST_UNKNOWN_TUNNEL_KEY,
+OVN_MCAST_MROUTER_FLOOD_TUNNEL_KEY,
+OVN_MIN_IP_MULTICAST,
+OVN_MAX_IP_MULTICAST = OVN_MAX_MULTICAST,
+};
 
 struct ovsdb_idl_index *mcast_group_index_create(struct ovsdb_idl *);
 const struct sbrec_multicast_group *
diff --git a/northd/ovn-northd.8.xml b/northd/ovn-northd.8.xml
index 6d2fbe3..d45bb15 100644
--- a/northd/ovn-northd.8.xml
+++ b/northd/ovn-northd.8.xml
@@ -947,10 +947,40 @@ output;
 
 
   
-A priority-100 flow that outputs all packets with an Ethernet broadcast
+A priority-100 flow that punts all IGMP packets to
+ovn-controller if IGMP snooping is enabled on the
+logical switch.
+  
+
+  
+Priority-90 flows that forward registered IP multicast traffic to
+their corresponding multicast group, which ovn-northd
+creates based on learnt 
+entries.  The flows also forward packets to the
+MC_MROUTER_FLOOD multicast group, which
+ovn-nortdh populates with all the logical ports that
+are connected to logical routers with
+:mcast_relay='true'.
+  
+
+  
+A priority-85 flow that forwards all IP multicast traffic destined to
+224.0.0.X to the MC_FLOOD multicast group, which
+ovn-northd populates with all enabled logical ports.
+  
+
+  
+A priority-80 flow that forwards all unregistered IP multicast traffic
+to the MC_MROUTER_FLOOD multicast group, if any.
+Otherwise the flow drops all unregistered IP multicast packets.  This
+flow is added only if :mcast_flood_unregistered='false'.
+  
+
+  
+A priority-70 flow that outputs all packets with an Ethernet broadcast
 or multicast eth.dst to the MC_FLOOD
-multicast group, which ovn-northd populates with all
-enabled logical ports.
+multicast group.
   
 
   
@@ -1228,6 +1258,14 @@ output;
 
   
 
+  A priority-95 flow allows IP multicast traffic if
+  :mcast_relay='true',
+  otherwise drops it.
+
+  
+
+  
+
   ICMP echo reply.  These flows reply to ICMP echo requests received
   for the router's IP address.  Let A be an IP address
   owned by a router port. 

Re: [ovs-dev] [PATCH] ofproto-dpif: Fix for recirc issue with mpls traffic with dp_hash

[Rudra Surya Bhaskara Rao via dev]

Hi All,

Please consider this as second gentle reminder.
Could you please review this fix and provide your valuable suggestions and 
comments.

Thanks & Regards,
Surya.

-Original Message-
From: Rudra Surya Bhaskara Rao  
Sent: 31 July 2019 10:41 AM
To: 'ovs-dev@openvswitch.org' 
Subject: RE: [PATCH] ofproto-dpif: Fix for recirc issue with mpls traffic with 
dp_hash

Hi All,

Please consider this as gentle reminder.

Thanks & Regards,
Surya.

-Original Message-
From: Surya Rudra  
Sent: 19 July 2019 11:05 AM
To: ovs-dev@openvswitch.org
Cc: Surya Rudra 
Subject: [PATCH] ofproto-dpif: Fix for recirc issue with mpls traffic with 
dp_hash

Fix infinite recirculation loop for MPLS packets sent to dp_hash-based  select 
group

Issue:
When a MPLS encapsulated packet is received, the MPLS header is removed, a 
recirculation id assigned and then recirculated into the pipeline.
If the flow rules require the packet to be then sent over DP-HASH based select 
group buckets, the packet has to be recirculated again. However, the same 
recirculation id was used and this resulted in the packet being repeatedly 
recirculated until it got dropped because the maximum recirculation limit was 
hit.

Fix:
Include the  “was_mpls” boolean which indicates whether the packet was MPLS 
encapsulated when computing the hash. After popping the MPLS header this will 
result in a  different hash value than before and new recirculation id will get 
generated.

DPCTL flows with and without the fix are shown below Without Fix:
recirc_id(0x1),dp_hash(0x5194bf18/0xf),in_port(2),packet_type(ns=0,id=0),
eth_type(0x0800),ipv4(frag=no), packets:20, bytes:1960, used:0.329s, actions:1 
recirc_id(0x1),in_port(2),packet_type(ns=0,id=0),eth_type(0x0800),
ipv4(frag=no), packets:20, bytes:1960, used:0.329s,
actions:hash(sym_l4(0)),recirc(0x1)
recirc_id(0),in_port(2),packet_type(ns=0,id=0),eth_type(0x8847),
mpls(label=22/0xf,tc=0/0,ttl=64/0x0,bos=1/1), packets:20, bytes:2040, 
used:0.329s, actions:pop_mpls(eth_type=0x800),recirc(0x1)

With Fix:
recirc_id(0x2),dp_hash(0x5194bf18/0xf),in_port(3),packet_type(ns=0,id=0),
eth_type(0x0800),ipv4(frag=no), packets:12481, bytes:1223138, used:0.588s, 
actions:1 recirc_id(0x1),in_port(3),packet_type(ns=0,id=0),eth_type(0x0800),
ipv4(frag=no), packets:74431, bytes:7294238, used:0.386s,
actions:hash(sym_l4(0)),recirc(0x2)
recirc_id(0x2),dp_hash(0xb952470d/0xf),in_port(3),packet_type(ns=0,id=0),
eth_type(0x0800),ipv4(frag=no), packets:12441, bytes:1219218, used:0.482s, 
actions:1 
recirc_id(0x2),dp_hash(0xeff6ad76/0xf),in_port(3),packet_type(ns=0,id=0),
eth_type(0x0800),ipv4(frag=no), packets:12385, bytes:1213730, used:0.908s, 
actions:1 recirc_id(0),in_port(3),packet_type(ns=0,id=0),eth_type(0x8847),
mpls(label=22/0xf,tc=0/0,ttl=64/0x0,bos=1/1), packets:74431, bytes:7591962, 
used:0.386s, actions:pop_mpls(eth_type=0x800),recirc(0x1)
recirc_id(0x2),dp_hash(0xb6233fbe/0xf),in_port(3),packet_type(ns=0,id=0),
eth_type(0x0800),ipv4(frag=no), packets:12369, bytes:1212162, used:0.386s, 
actions:1 
recirc_id(0x2),dp_hash(0xa3670459/0xf),in_port(3),packet_type(ns=0,id=0),
eth_type(0x0800),ipv4(frag=no), packets:24751, bytes:2425598, used:0.483s, 
actions:1

Signed-off-by: Surya Rudra 
---
 ofproto/ofproto-dpif-rid.c   | 2 ++
 ofproto/ofproto-dpif-rid.h   | 1 +
 ofproto/ofproto-dpif-xlate.c | 2 ++
 3 files changed, 5 insertions(+)

diff --git a/ofproto/ofproto-dpif-rid.c b/ofproto/ofproto-dpif-rid.c index 
79412c2..29aafc2 100644
--- a/ofproto/ofproto-dpif-rid.c
+++ b/ofproto/ofproto-dpif-rid.c
@@ -130,6 +130,7 @@ frozen_state_hash(const struct frozen_state *state)
 hash = hash_bytes64((const uint64_t *) >metadata,
 sizeof state->metadata, hash);
 hash = hash_boolean(state->conntracked, hash);
+hash = hash_boolean(state->was_mpls, hash);
 if (state->stack && state->stack_size) {
 hash = hash_bytes(state->stack, state->stack_size, hash);
 }
@@ -158,6 +159,7 @@ frozen_state_equal(const struct frozen_state *a, const 
struct frozen_state *b)
 && !memcmp(a->stack, b->stack, a->stack_size)
 && a->mirrors == b->mirrors
 && a->conntracked == b->conntracked
+&& a->was_mpls == b->was_mpls
 && ofpacts_equal(a->ofpacts, a->ofpacts_len,
  b->ofpacts, b->ofpacts_len)
 && ofpacts_equal(a->action_set, a->action_set_len, diff --git 
a/ofproto/ofproto-dpif-rid.h b/ofproto/ofproto-dpif-rid.h index 
ab5b87a..147ef9c 100644
--- a/ofproto/ofproto-dpif-rid.h
+++ b/ofproto/ofproto-dpif-rid.h
@@ -143,6 +143,7 @@ struct frozen_state {
 size_t stack_size;
 mirror_mask_t mirrors;/* Mirrors already output. */
 bool conntracked; /* Conntrack occurred prior to freeze. */
+bool was_mpls;/* MPLS packet */
 struct uuid xport_uuid;   /* UUID of 1st port packet received on. */
 
 /* Actions to be translated when thawing. */ 

Re: [ovs-dev] [PATCH ovn] Add support for using OVN specific rundirs

[Dumitru Ceara]

On Fri, Aug 9, 2019 at 8:24 PM  wrote:
>
> From: Numan Siddique 
>
> Until now, OVN uses the openvswitch rundirs (rundir, logdir, etcdir).
> The commit [1] changed the package name from openvswitch to ovn, but
> it didn't take into the account the effects of it. When "make install"
> is run ovn-ctl utility is copied to /usr/local/share/ovn/scripts folder.
> ovn-ctl depends on 'ovs-lib' and it is not present in this scripts foler.
> Because of which we cannot start OVN services using ovn-ctl.
>
> This patch addresses all these issues. It changes the rundir to
> ovn specific ones. (i.e /usr/local/var/run/ovn, /usr/local/var/log/ovn,
> /usr/local/etc/ovn with default configuration).
>
> [1] - 7795e0e28dce("Change the package name from openvswitch to ovn in 
> AC_INIT()")
>
> Signed-off-by: Numan Siddique 

Hi Numan,

I tried out your patch. Compiling, check, install worked fine:

./boot.sh
./configure --prefix=/home/dceara/local-builds/usr
--localstatedir=/home/dceara/local-builds/var
--sysconfdir=/home/dceara/local-builds/etc CFLAGS="-g" --enable-Werror
--enable-sparse
make -j8 check-am TESTSUITEFLAGS="-j8"
make install

While running OVN I noticed the following:
a. if we don't export the path to ovn-ctl and ovs-ctl and try to run
the scripts from their path we get:

$ cd /home/dceara/local-builds/usr/share/ovn/scripts/
$ /home/dceara/local-builds/usr/share/openvswitch/scripts/ovs-ctl
start --system-id=local && ./ovn-ctl start_nb_ovsdb && ./ovn-ctl
start_sb_ovsdb && ./ovn-ctl start_northd && ./ovn-ctl start_controller
Starting ovsdb-server  [  OK  ]
Configuring Open vSwitch system IDs[  OK  ]
Starting ovs-vswitchd  [  OK  ]
Enabling remote OVSDB managers [  OK  ]
./ovn-ctl: line 23: ./openvswitch/scripts/ovs-lib: No such file or directory

This happens because of the change in ovn-ctl to determine $ovsdir.
But I guess that's not a big issue as it's not really expected for a
user to run it like that, right?
If I use the full path it works fine:

$ /home/dceara/local-builds/usr/share/openvswitch/scripts/ovs-ctl
start --system-id=local && $PWD/ovn-ctl start_nb_ovsdb && $PWD/ovn-ctl
start_sb_ovsdb && $PWD/ovn-ctl start_northd && $PWD/ovn-ctl
start_controller
Starting ovsdb-server  [  OK  ]
Configuring Open vSwitch system IDs[  OK  ]
Starting ovs-vswitchd  [  OK  ]
Enabling remote OVSDB managers [  OK  ]
Starting ovn-northd[  OK  ]
Starting ovn-controller[  OK  ]

b. The user has to be careful if setting paths to the ov[sn]-ctl
scripts. This is because we still install ovn-ctl from the ovs subtree
in the usr/share/openvswitch/scripts directory. E.g.:

$ export 
PATH=/home/dceara/local-builds/usr/share/openvswitch/scripts/:/home/dceara/local-builds/usr/share/ovn/scripts/:/home/dceara/local-builds/usr/bin/:$PATH
$ ovs-ctl start --system-id=local && ovn-ctl start_nb_ovsdb && ovn-ctl
start_sb_ovsdb && ovn-ctl start_northd && ovn-ctl start_controller
Starting ovsdb-server  [  OK  ]
Configuring Open vSwitch system IDs[  OK  ]
Starting ovs-vswitchd  [  OK  ]
Enabling remote OVSDB managers [  OK  ]
ovn-nbctl: unix:/home/dceara/local-builds/var/run/ovn/ovnnb_db.sock:
database connection failed (Connection refused)

<<< Stuck here because we're actually using ovn-ctl from
usr/share/openvswitch/scripts

I guess this will be fixed once the OVN code is removed from the OVS
repo. Until then, specifying the paths in the correct order (OVN
first, then OVS) works fine:

$ export 
PATH=/home/dceara/local-builds/usr/share/ovn/scripts/:/home/dceara/local-builds/usr/share/openvswitch/scripts/:/home/dceara/local-builds/usr/bin/:$PATH
$ ovs-ctl start --system-id=local && ovn-ctl start_nb_ovsdb && ovn-ctl
start_sb_ovsdb && ovn-ctl start_northd && ovn-ctl start_controller
Starting ovsdb-server  [  OK  ]
Configuring Open vSwitch system IDs[  OK  ]
Starting ovs-vswitchd  [  OK  ]
Enabling remote OVSDB managers [  OK  ]
Starting ovn-northd[  OK  ]
Starting ovn-controller[  OK  ]

So, with the small issues above that might need to be documented, the
patch works fine for me:

Tested-by: Dumitru Ceara 

Thanks,
Dumitru


> ---
>  Makefile.am |   5 +
>  configure.ac|   1 +
>  controller/ovn-controller.c |   4 +-
>  lib/automake.mk |  20 +++-
>  lib/ovn-dirs.c.in   | 112 
>  lib/ovn-dirs.h  |  35 +++
>  lib/ovn-util.c  

Re: [ovs-dev] [PATCH] checkpatch: Fix regexp for if, while, etc inside macros.

[Ilya Maximets]

On 09.08.2019 17:04, Aaron Conole wrote:
> Ilya Maximets  writes:
> 
>> This allows to use a one-character expression inside the 'if'
>> statement and multiple spaces before the line continuation character.
>>
>> Fixes false positive in case like this:
>>
>>   #define MACRO(ARG) \
>>   if (a) {   \
>>   do_work(ARG);  \
>>   }
>>
>> Fixes: 16770c6d9179 ("checkpatch: support macro continuation")
>> Signed-off-by: Ilya Maximets 
>> ---
> 
> Acked-by: Aaron Conole 

Thanks! Applied to master.

Best regards, Ilya Maximets.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] 答复: Re: 答复: [ovs-discuss] why action "meter" only can be specified once?

[杨燚]

Thanks Ilya, your proposal works for me, not bad :-)

-邮件原件-
发件人: Ilya Maximets [mailto:i.maxim...@samsung.com] 
发送时间: 2019年8月12日 14:48
收件人: ovs-dev@openvswitch.org; Yi Yang (杨燚)-云服务集团 
抄送: William Tu ; Ben Pfaff 
主题: Re: Re: [ovs-dev] 答复: [ovs-discuss] why action "meter" only can be 
specified once?

Hi.
If you'll try to use more recent OVS version, you'll get more informative error 
message:
ovs-ofctl: duplicate meter instruction not allowed, for OpenFlow 1.1+ 
compatibility

So, it seems like an issue between different versions of OF standards.

Anyway, you may overcome this issue by splitting your flows in two parts:

table=0,ip,nw_src=10.0.0.0/24 actions=meter:4,resubmit:1
table=1,ip,nw_src=10.0.0.2actions=meter:1, ...other-actions
table=1,ip,nw_src=10.0.0.3actions=meter:2, ...other-actions
table=1,ip,nw_src=10.0.0.4actions=meter:3, ...other-actions

Best regards, Ilya Maximets.

> William, I have several flows to share a meter, at the time, every flow has 
> its own meter, they look like the below:
> 
> table=0,ip,nw_src=10.0.0.2 actions=meter:1,meter:4, ...other-actions
> table=0,ip,nw_src=10.0.0.3 actions=meter:2,meter:4, ...other-actions
> table=0,ip,nw_src=10.0.0.3 actions=meter:3,meter:4, ...other-actions
> 
> meter 4 are shared by three flows so that we can let tenants leverage their 
> bandwidth more efficient between three flows, so total limit is 100Mbps, 
> every one limit is 50Mbps, if flow 3 has lower bandwidth utilization rate, 
> flow 1 and 2 can leverage it. I can get every flow stats and also can get all 
> the flows stats by meter 4, that is what I'm saying.
> 
> I think supporting it isn't a big technical issue, it will be great if you 
> can add this feature, I think it is very helpful, we have real use cases 
> which is called shared bandwith.
> 
> -邮件原件-
> 发件人: William Tu [mailto:u9012063 at gmail.com]
> 发送时间: 2019年8月10日 0:54
> 收件人: Yi Yang (杨燚)-云服务集团 
> 抄送: ovs-dev at openvswitch.org; ovs-discuss at openvswitch.org
> 主题: Re: [ovs-discuss] why action "meter" only can be specified once?
> 
> On Mon, Aug 5, 2019 at 12:39 AM Yi Yang (杨燚)-云服务集团  
> wrote:
>>
>> Hi, all
>>
>>
>>
>> I was told meter only can be specified once, but actually there is such case 
>> existing, i.e. multiple flows share a total bandwidth, but every flow also 
>> has its own bandwidth limit, by two meters, we can not only get every flow 
>> stats but also get total stats, I think this is very reasonable user 
>> scenario.
>>
> 
> I don't understand your use case.
> You can create multiple meters and each flow can use it own meter to rate 
> limit, right?
>>
>>
>> ovs-ofctl: instruction meter may be specified only once
> How do you get this error?
> 
> Thanks
> William

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

Re: [ovs-dev] 答复: [ovs-discuss] why action "meter" only can be specified once?

[Ilya Maximets]

CC: ovs-discuss

On 12.08.2019 9:47, Ilya Maximets wrote:
> Hi.
> If you'll try to use more recent OVS version, you'll get more informative
> error message:
> ovs-ofctl: duplicate meter instruction not allowed, for OpenFlow 1.1+ 
> compatibility
> 
> So, it seems like an issue between different versions of OF standards.
> 
> Anyway, you may overcome this issue by splitting your flows in two parts:
> 
> table=0,ip,nw_src=10.0.0.0/24 actions=meter:4,resubmit:1
> table=1,ip,nw_src=10.0.0.2actions=meter:1, ...other-actions
> table=1,ip,nw_src=10.0.0.3actions=meter:2, ...other-actions
> table=1,ip,nw_src=10.0.0.4actions=meter:3, ...other-actions
> 
> Best regards, Ilya Maximets.
> 
>> William, I have several flows to share a meter, at the time, every flow has 
>> its own meter, they look like the below:
>>
>> table=0,ip,nw_src=10.0.0.2 actions=meter:1,meter:4, ...other-actions
>> table=0,ip,nw_src=10.0.0.3 actions=meter:2,meter:4, ...other-actions
>> table=0,ip,nw_src=10.0.0.3 actions=meter:3,meter:4, ...other-actions
>>
>> meter 4 are shared by three flows so that we can let tenants leverage their 
>> bandwidth more efficient between three flows, so total limit is 100Mbps, 
>> every one limit is 50Mbps, if flow 3 has lower bandwidth utilization rate, 
>> flow 1 and 2 can leverage it. I can get every flow stats and also can get 
>> all the flows stats by meter 4, that is what I'm saying.
>>
>> I think supporting it isn't a big technical issue, it will be great if you 
>> can add this feature, I think it is very helpful, we have real use cases 
>> which is called shared bandwith.
>>
>> -邮件原件-
>> 发件人: William Tu [mailto:u9012063 at gmail.com] 
>> 发送时间: 2019年8月10日 0:54
>> 收件人: Yi Yang (杨燚)-云服务集团 
>> 抄送: ovs-dev at openvswitch.org; ovs-discuss at openvswitch.org
>> 主题: Re: [ovs-discuss] why action "meter" only can be specified once?
>>
>> On Mon, Aug 5, 2019 at 12:39 AM Yi Yang (杨燚)-云服务集团  
>> wrote:
>>>
>>> Hi, all
>>>
>>>
>>>
>>> I was told meter only can be specified once, but actually there is such 
>>> case existing, i.e. multiple flows share a total bandwidth, but every flow 
>>> also has its own bandwidth limit, by two meters, we can not only get every 
>>> flow stats but also get total stats, I think this is very reasonable user 
>>> scenario.
>>>
>>
>> I don't understand your use case.
>> You can create multiple meters and each flow can use it own meter to rate 
>> limit, right?
>>>
>>>
>>> ovs-ofctl: instruction meter may be specified only once
>> How do you get this error?
>>
>> Thanks
>> William
> 
> 
> 
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] 答复: [ovs-discuss] why action "meter" only can be specified once?

[Ilya Maximets]

Hi.
If you'll try to use more recent OVS version, you'll get more informative
error message:
ovs-ofctl: duplicate meter instruction not allowed, for OpenFlow 1.1+ 
compatibility

So, it seems like an issue between different versions of OF standards.

Anyway, you may overcome this issue by splitting your flows in two parts:

table=0,ip,nw_src=10.0.0.0/24 actions=meter:4,resubmit:1
table=1,ip,nw_src=10.0.0.2actions=meter:1, ...other-actions
table=1,ip,nw_src=10.0.0.3actions=meter:2, ...other-actions
table=1,ip,nw_src=10.0.0.4actions=meter:3, ...other-actions

Best regards, Ilya Maximets.

> William, I have several flows to share a meter, at the time, every flow has 
> its own meter, they look like the below:
> 
> table=0,ip,nw_src=10.0.0.2 actions=meter:1,meter:4, ...other-actions
> table=0,ip,nw_src=10.0.0.3 actions=meter:2,meter:4, ...other-actions
> table=0,ip,nw_src=10.0.0.3 actions=meter:3,meter:4, ...other-actions
> 
> meter 4 are shared by three flows so that we can let tenants leverage their 
> bandwidth more efficient between three flows, so total limit is 100Mbps, 
> every one limit is 50Mbps, if flow 3 has lower bandwidth utilization rate, 
> flow 1 and 2 can leverage it. I can get every flow stats and also can get all 
> the flows stats by meter 4, that is what I'm saying.
> 
> I think supporting it isn't a big technical issue, it will be great if you 
> can add this feature, I think it is very helpful, we have real use cases 
> which is called shared bandwith.
> 
> -邮件原件-
> 发件人: William Tu [mailto:u9012063 at gmail.com] 
> 发送时间: 2019年8月10日 0:54
> 收件人: Yi Yang (杨燚)-云服务集团 
> 抄送: ovs-dev at openvswitch.org; ovs-discuss at openvswitch.org
> 主题: Re: [ovs-discuss] why action "meter" only can be specified once?
> 
> On Mon, Aug 5, 2019 at 12:39 AM Yi Yang (杨燚)-云服务集团  
> wrote:
>>
>> Hi, all
>>
>>
>>
>> I was told meter only can be specified once, but actually there is such case 
>> existing, i.e. multiple flows share a total bandwidth, but every flow also 
>> has its own bandwidth limit, by two meters, we can not only get every flow 
>> stats but also get total stats, I think this is very reasonable user 
>> scenario.
>>
> 
> I don't understand your use case.
> You can create multiple meters and each flow can use it own meter to rate 
> limit, right?
>>
>>
>> ovs-ofctl: instruction meter may be specified only once
> How do you get this error?
> 
> Thanks
> William

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