Re: [ovs-dev] [PATCH v11 2/2] conntrack: prefer dst port range during unique tuple search

[Paolo Valerio]

Hello wenxu,

we...@ucloud.cn writes:

> From: wenxu 
>
> This commit splits the nested loop used to search the unique ports for
> the reverse tuple.
> It affects only the dnat action, giving more precedence to the dnat
> range, similarly to the kernel dp, instead of searching through the
> default ephemeral source range for each destination port.
>
> Signed-off-by: wenxu 
> ---

the patch LGTM.

Acked-by: Paolo Valerio 

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

[ovs-dev] [PATCH ovn] northd: allow explicit nat-addresses for distributed gw ports

[Lorenzo Bianconi]

Allow the CMS to explicitly configure nat addresses to use in
GARP advertising even if the logical switch port is connected
to a distributed gateway router port.

Signed-off-by: Lorenzo Bianconi 
---
 northd/northd.c | 13 +++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/northd/northd.c b/northd/northd.c
index 861812259..4f9c10648 100644
--- a/northd/northd.c
+++ b/northd/northd.c
@@ -3373,6 +3373,8 @@ ovn_port_update_sbrec(struct northd_input *input_data,
"nat-addresses");
 size_t n_nats = 0;
 char **nats = NULL;
+bool l3dgw_ports = op->peer && op->peer->od &&
+   op->peer->od->n_l3dgw_ports;
 if (nat_addresses && !strcmp(nat_addresses, "router")) {
 if (op->peer && op->peer->od
 && (chassis || op->peer->od->n_l3dgw_ports)) {
@@ -3380,17 +3382,24 @@ ovn_port_update_sbrec(struct northd_input *input_data,
 }
 /* Only accept manual specification of ethernet address
  * followed by IPv4 addresses on type "l3gateway" ports. */
-} else if (nat_addresses && chassis) {
+} else if (nat_addresses && (chassis || l3dgw_ports)) {
 struct lport_addresses laddrs;
 if (!extract_lsp_addresses(nat_addresses, &laddrs)) {
 static struct vlog_rate_limit rl =
 VLOG_RATE_LIMIT_INIT(1, 1);
 VLOG_WARN_RL(&rl, "Error extracting nat-addresses.");
 } else {
+struct ds data = DS_EMPTY_INITIALIZER;
+ds_put_format(&data, "%s", nat_addresses);
+if (l3dgw_ports) {
+ds_put_format(&data, " is_chassis_resident(%s)",
+op->peer->od->l3dgw_ports[0]->cr_port->json_key);
+}
 destroy_lport_addresses(&laddrs);
 n_nats = 1;
 nats = xcalloc(1, sizeof *nats);
-nats[0] = xstrdup(nat_addresses);
+nats[0] = xstrdup(ds_cstr(&data));
+ds_destroy(&data);
 }
 }
 
-- 
2.35.1

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

[ovs-dev] ovsdb with ovn-controller-vtep question

[Vladislav Odintsov]

Hi,

I was debugging a strange behaviour of ovn-controller-vtep daemon and need some 
assistance about how ovsdb transactions and in-memory objects work.

ovn-controller-vtep, when a new vlan binding appears tries to commit in SB DB 
type=vtep Port_Binding "chassis" column value [0].
Based on the chassis column existence it commits tunnel_key value: 
port_binding->datapath->tunnel_key if chassis exists [1], 0 otherwise [2].

I see that during the main loop iteration, where ovn-controller-vtep tries to 
set chassis to port_binding, this port_binding is modified in memory and 
functions, which run after this setting in same main loop iteration, work with 
a modified port-binding object, which was not yet committed and which was not 
yet acknowledged by SBDB. This triggers ovn-controller-vtep to set vtep lswitch 
tunnel_key column to appropriate value (non-zero).
On the next iteration I see that this port-binding object returns to previous 
state (actual to DB contents — no chassis in port binding) and 
ovn-controller-vtep resets vtep lswitch tunnel_key to 0.
After SB DB responses with an update to last transation with an update3 method, 
which contains a change for port_binding: set chassis.
Now ovn-controller-vtep again sets tunnel_key for vtep logical-switch to 
non-zero value.

Is it an expected behaviour? How to avoid such flapping?

0: https://github.com/ovn-org/ovn/blob/v21.12.0/controller-vtep/binding.c#L113
1: https://github.com/ovn-org/ovn/blob/v21.12.0/controller-vtep/vtep.c#L241
2: https://github.com/ovn-org/ovn/blob/v21.12.0/controller-vtep/vtep.c#L261


Regards,
Vladislav Odintsov

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

Re: [ovs-dev] [PATCH v2] dpif-netdev-dpcls: Make subtable reprobe thread-safe.

[Stokes, Ian]

> >> Thanks for the patch, in general this looks ok to me, but I realize Ilya 
> >> had a
> few comments on the v1. I think these are addressed here but maybe Ilya would
> like to confirm before sign off?
> >
> > Thanks, Ian.
> > I'll take a look at this patch tomorrow.
> >
> >>
> >> @Ilya Maximets should this go into branch 2.17 as well before release?
> >
> > I'd say, if it applies cleanly, we should try to backport it down to 2.14,
> > because the issue may cause OVS crash.
> 
> I didn't test it, but the change looks correct to me.  Thanks!
> 
> Acked-by: Ilya Maximets 

Thanks Ilya, applied to master and backported as far as 2.14.

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

[ovs-dev] [PATCH ovn v5] acl-log: Log the direction (logical pipeline) of the matching ACL.

[Dumitru Ceara]

It's useful to differentiate between ingress and egress pipelines in the
ACL logs.  To achieve this we determine the direction by interpreting the
openflow table ID when processing packets punted to pinctrl by "log()"
action.

Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=1992641
Acked-by: Numan Siddique 
Signed-off-by: Dumitru Ceara 
---
v5:
- Rebased and fixed minor conflict in NEWS.
v4:
- Log direction as from-lport/to-lport.
- Add Numan's ack.
v3: Simplify the patch as suggested by Numan; detect pipeline based on
openflow table id.
v2: Add the "direction" after the "verdict" and "severity" fields in the
ACL logs.
---
 NEWS  |  2 ++
 controller/pinctrl.c  |  4 ++-
 lib/acl-log.c |  8 +++--
 lib/acl-log.h |  3 +-
 tests/ovn.at  | 68 ++-
 utilities/ovn-trace.c |  9 --
 6 files changed, 73 insertions(+), 21 deletions(-)

diff --git a/NEWS b/NEWS
index 984e9977e9..a309fe8df7 100644
--- a/NEWS
+++ b/NEWS
@@ -10,6 +10,8 @@ Post v21.12.0
   - Support version pinning between ovn-northd and ovn-controller-vtep as an
 option. If the option is enabled and the versions don't match,
 ovn-controller-vtep will not process any DB changes.
+  - When configured to log packtes matching ACLs, log the direction (logical
+pipeline) too.
 
 OVN v21.12.0 - 22 Dec 2021
 --
diff --git a/controller/pinctrl.c b/controller/pinctrl.c
index 1b8b47523f..2dd1bc7bd5 100644
--- a/controller/pinctrl.c
+++ b/controller/pinctrl.c
@@ -3166,7 +3166,9 @@ process_packet_in(struct rconn *swconn, const struct 
ofp_header *msg)
 break;
 
 case ACTION_OPCODE_LOG:
-handle_acl_log(&headers, &userdata);
+handle_acl_log(&headers, &userdata,
+   pin.table_id < OFTABLE_LOG_EGRESS_PIPELINE
+   ? "from-lport" : "to-lport");
 break;
 
 case ACTION_OPCODE_PUT_ND_RA_OPTS:
diff --git a/lib/acl-log.c b/lib/acl-log.c
index 220b6dc30e..9530dd7635 100644
--- a/lib/acl-log.c
+++ b/lib/acl-log.c
@@ -76,7 +76,8 @@ log_severity_from_string(const char *name)
 }
 
 void
-handle_acl_log(const struct flow *headers, struct ofpbuf *userdata)
+handle_acl_log(const struct flow *headers, struct ofpbuf *userdata,
+   const char *direction)
 {
 if (!VLOG_IS_INFO_ENABLED()) {
 return;
@@ -94,9 +95,10 @@ handle_acl_log(const struct flow *headers, struct ofpbuf 
*userdata)
 struct ds ds = DS_EMPTY_INITIALIZER;
 ds_put_cstr(&ds, "name=");
 json_string_escape(name_len ? name : "", &ds);
-ds_put_format(&ds, ", verdict=%s, severity=%s: ",
+ds_put_format(&ds, ", verdict=%s, severity=%s, direction=%s: ",
   log_verdict_to_string(lph->verdict),
-  log_severity_to_string(lph->severity));
+  log_severity_to_string(lph->severity),
+  direction);
 flow_format(&ds, headers, NULL);
 
 VLOG_INFO("%s", ds_cstr(&ds));
diff --git a/lib/acl-log.h b/lib/acl-log.h
index 4f23f790dd..da7fa2f02c 100644
--- a/lib/acl-log.h
+++ b/lib/acl-log.h
@@ -49,6 +49,7 @@ const char *log_verdict_to_string(uint8_t verdict);
 const char *log_severity_to_string(uint8_t severity);
 uint8_t log_severity_from_string(const char *name);
 
-void handle_acl_log(const struct flow *headers, struct ofpbuf *userdata);
+void handle_acl_log(const struct flow *headers, struct ofpbuf *userdata,
+const char *direction);
 
 #endif /* lib/acl-log.h */
diff --git a/tests/ovn.at b/tests/ovn.at
index 184594831a..47e3e18a99 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -8965,33 +8965,59 @@ ovn-nbctl lsp-set-addresses lp2 $lp2_mac
 ovn-nbctl --wait=sb sync
 wait_for_ports_up
 
-ovn-nbctl acl-add lsw0 to-lport 1000 'tcp.dst==80' drop
-ovn-nbctl --log --severity=alert --name=drop-flow acl-add lsw0 to-lport 1000 
'tcp.dst==81' drop
+ovn-nbctl acl-add lsw0 from-lport 1000 'tcp.dst==80' drop
+ovn-nbctl --log --severity=alert --name=drop-flow acl-add lsw0 from-lport 1000 
'tcp.dst==81' drop
+
+ovn-nbctl acl-add lsw0 to-lport 1000 'tcp.dst==180' drop
+ovn-nbctl --log --severity=alert --name=drop-flow acl-add lsw0 to-lport 1000 
'tcp.dst==181' drop
+
+ovn-nbctl acl-add lsw0 from-lport 1000 'tcp.dst==82' allow
+ovn-nbctl --log --severity=info --name=allow-flow acl-add lsw0 from-lport 1000 
'tcp.dst==83' allow
 
 ovn-nbctl acl-add lsw0 to-lport 1000 'tcp.dst==82' allow
 ovn-nbctl --log --severity=info --name=allow-flow acl-add lsw0 to-lport 1000 
'tcp.dst==83' allow
 
+ovn-nbctl acl-add lsw0 from-lport 1000 'tcp.dst==84' allow-related
+ovn-nbctl --log acl-add lsw0 from-lport 1000 'tcp.dst==85' allow-related
+
 ovn-nbctl acl-add lsw0 to-lport 1000 'tcp.dst==84' allow-related
 ovn-nbctl --log acl-add lsw0 to-lport 1000 'tcp.dst==85' allow-related
 
-ovn-nbctl acl-add lsw0 to-lport 1000 'tcp.dst==86' reject
-ovn-nbctl --wait=hv --log --severity=alert --name=reject-flow acl-add lsw0 
to-lport 1000 'tcp.dst==87' re

Re: [ovs-dev] [PATCH v3 00/14] Fix UndefinedBehaviorSanitizer reported issues and enable it in CI.

[Ilya Maximets]

On 2/15/22 18:06, Dumitru Ceara wrote:
> On 2/15/22 14:22, Ilya Maximets wrote:
>> On 1/24/22 21:10, Dumitru Ceara wrote:
>>> On 1/24/22 19:40, Adrian Moreno wrote:


 On 1/24/22 18:49, Jeffrey Walton wrote:
> On Mon, Jan 24, 2022 at 9:17 AM Dumitru Ceara  wrote:
>>
>> As privately reported by Aaron Conole, and by Jeffrey Walton [0]
>> there's currently a number of undefined behavior instances in
>> the OVS code base.  Running the OVS (and OVN) tests with UBSan [1]
>> enabled uncovers these.
>> ...
>
>> Note: depending on the order of reviews, if Adrian's "Fix undefined
>> behavior in loop macros" series [2] (or a follow up) is accepted first,
>> then patch 12/14 ("util: Avoid false positive UB when iterating
>> collections.") can be skipped.  Adrian's series seems to be the more
>> correct way of fixing the issue.
>
> One small nit... UBsan (and Asan) do not produce false positives. They
> operate on real data, and when they produce a finding it is valid.
> That's also why a complete set of self tests are important. The
> complete set of tests are important because UBsan and Asan need real
> data.
>

 I agree, it's not a false positive. Furthermore, the patch that Dumitru
 is referring to ("util: Avoid false positive UB when iterating
 collections") reduces UBsan's sensitivity by changing some pointer
 arithmetics to integer arithmetics. This silences the UBsan but
 according to the discussion with the compiler folks [1], this can still
 yield UB.

 Therefore, I think it would be safer to keep the pointer arithmetics
 (and UBsan's high-sensitivity), fix the actual callers (i.e: the
 iterator macros), and run UBsan in the CI to spot all future errors
 (which Dumitru's series does).

 [1]https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103964

>>>
>>> Yes, I probably should've rephrased the commit title of patch 12/14.
>>> It's not a false positive.  I just kept it for now until Adrian's series
>>> [2] is merged.  Otherwise jobs would've failed in CI, and it's
>>> technically not worse than before.
>>>
>>> But I completely agree, once Adrian's changes get accepted, the safest
>>> way is to keep the pointer arithmetic and rely on UBSan to complain if
>>> undefined behavior is detected.
>>>
>>> [2]
>>> https://patchwork.ozlabs.org/project/openvswitch/list/?series=282481&state=*
>>>
>>
>> For now I applied 7 out of 14 patches from this series and replied with
>> a comments to a few of the remaining patches.
> 
> Thanks a lot for this!
> 
>> I still have a couple of ofp patches that I didn't look close enough yet.
>>
> 
> I'll send out a v4 addressing the current review comments and we can
> continue the review of the ofp part there.  Does that sound OK to you?

Yep.  Sure.

> 
>> Best regards, Ilya Maximets.
>>
> 
> Regards,
> Dumitru
> 

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

Re: [ovs-dev] [PATCH v3 00/14] Fix UndefinedBehaviorSanitizer reported issues and enable it in CI.

[Dumitru Ceara]

On 2/15/22 14:22, Ilya Maximets wrote:
> On 1/24/22 21:10, Dumitru Ceara wrote:
>> On 1/24/22 19:40, Adrian Moreno wrote:
>>>
>>>
>>> On 1/24/22 18:49, Jeffrey Walton wrote:
 On Mon, Jan 24, 2022 at 9:17 AM Dumitru Ceara  wrote:
>
> As privately reported by Aaron Conole, and by Jeffrey Walton [0]
> there's currently a number of undefined behavior instances in
> the OVS code base.  Running the OVS (and OVN) tests with UBSan [1]
> enabled uncovers these.
> ...

> Note: depending on the order of reviews, if Adrian's "Fix undefined
> behavior in loop macros" series [2] (or a follow up) is accepted first,
> then patch 12/14 ("util: Avoid false positive UB when iterating
> collections.") can be skipped.  Adrian's series seems to be the more
> correct way of fixing the issue.

 One small nit... UBsan (and Asan) do not produce false positives. They
 operate on real data, and when they produce a finding it is valid.
 That's also why a complete set of self tests are important. The
 complete set of tests are important because UBsan and Asan need real
 data.

>>>
>>> I agree, it's not a false positive. Furthermore, the patch that Dumitru
>>> is referring to ("util: Avoid false positive UB when iterating
>>> collections") reduces UBsan's sensitivity by changing some pointer
>>> arithmetics to integer arithmetics. This silences the UBsan but
>>> according to the discussion with the compiler folks [1], this can still
>>> yield UB.
>>>
>>> Therefore, I think it would be safer to keep the pointer arithmetics
>>> (and UBsan's high-sensitivity), fix the actual callers (i.e: the
>>> iterator macros), and run UBsan in the CI to spot all future errors
>>> (which Dumitru's series does).
>>>
>>> [1]https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103964
>>>
>>
>> Yes, I probably should've rephrased the commit title of patch 12/14.
>> It's not a false positive.  I just kept it for now until Adrian's series
>> [2] is merged.  Otherwise jobs would've failed in CI, and it's
>> technically not worse than before.
>>
>> But I completely agree, once Adrian's changes get accepted, the safest
>> way is to keep the pointer arithmetic and rely on UBSan to complain if
>> undefined behavior is detected.
>>
>> [2]
>> https://patchwork.ozlabs.org/project/openvswitch/list/?series=282481&state=*
>>
> 
> For now I applied 7 out of 14 patches from this series and replied with
> a comments to a few of the remaining patches.

Thanks a lot for this!

> I still have a couple of ofp patches that I didn't look close enough yet.
> 

I'll send out a v4 addressing the current review comments and we can
continue the review of the ofp part there.  Does that sound OK to you?

> Best regards, Ilya Maximets.
> 

Regards,
Dumitru

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

Re: [ovs-dev] [PATCH ovn v2] controller: add ovn-set-local-ip option

[Vladislav Odintsov]

Hi Numan,

I’ve submitted the v2, please check this out:
https://patchwork.ozlabs.org/project/ovn/patch/20220215145442.2868060-1-odiv...@gmail.com/

Regards,
Vladislav Odintsov

> On 11 Feb 2022, at 22:12, Numan Siddique  wrote:
> 
> On Wed, Feb 9, 2022 at 6:33 PM Vladislav Odintsov  > wrote:
>> 
>> When transport node has multiple interfaces (vlans) and
>> ovn-encap-ip on different hosts need to be configured
>> from different VLANs source IP for encapsulated packet
>> can be not the same, which is expected by remote system.
>> 
>> Explicitely setting local_ip resolves such problem.
>> 
>> Signed-off-by: Vladislav Odintsov > >
> 
> Hi Vladislav,
> 
> Can you please add the code to copy the new added config from OVS
> database to the
> other_config column of Chassis table in Southbound db ?
> 
> Please take a look at "struct ovs_chassis_cfg" in controller/chassis.c
> 
> Thanks
> Numan
> 
>> ---
>> controller/encaps.c | 37 +
>> controller/ovn-controller.8.xml |  7 +++
>> tests/ovn-controller.at |  9 
>> 3 files changed, 40 insertions(+), 13 deletions(-)
>> 
>> diff --git a/controller/encaps.c b/controller/encaps.c
>> index 66e0cd8cd..3b0c92931 100644
>> --- a/controller/encaps.c
>> +++ b/controller/encaps.c
>> @@ -23,6 +23,7 @@
>> #include "openvswitch/vlog.h"
>> #include "lib/ovn-sb-idl.h"
>> #include "ovn-controller.h"
>> +#include "smap.h"
>> 
>> VLOG_DEFINE_THIS_MODULE(encaps);
>> 
>> @@ -176,8 +177,31 @@ tunnel_add(struct tunnel_ctx *tc, const struct 
>> sbrec_sb_global *sbg,
>> smap_add(&options, "dst_port", dst_port);
>> }
>> 
>> +const struct ovsrec_open_vswitch *cfg =
>> +ovsrec_open_vswitch_table_first(ovs_table);
>> +
>> +bool set_local_ip = false;
>> +if (cfg) {
>> +/* If the tos option is configured, get it */
>> +const char *encap_tos = smap_get_def(&cfg->external_ids,
>> +   "ovn-encap-tos", "none");
>> +
>> +if (encap_tos && strcmp(encap_tos, "none")) {
>> +smap_add(&options, "tos", encap_tos);
>> +}
>> +
>> +/* If ovn-set-local-ip option is configured, get it */
>> +set_local_ip = smap_get_bool(&cfg->external_ids, "ovn-set-local-ip",
>> + false);
>> +}
>> +
>> /* Add auth info if ipsec is enabled. */
>> if (sbg->ipsec) {
>> +set_local_ip = true;
>> +smap_add(&options, "remote_name", new_chassis_id);
>> +}
>> +
>> +if (set_local_ip) {
>> const struct sbrec_chassis *this_chassis = tc->this_chassis;
>> const char *local_ip = NULL;
>> 
>> @@ -200,19 +224,6 @@ tunnel_add(struct tunnel_ctx *tc, const struct 
>> sbrec_sb_global *sbg,
>> if (local_ip) {
>> smap_add(&options, "local_ip", local_ip);
>> }
>> -smap_add(&options, "remote_name", new_chassis_id);
>> -}
>> -
>> -const struct ovsrec_open_vswitch *cfg =
>> -ovsrec_open_vswitch_table_first(ovs_table);
>> -/* If the tos option is configured, get it */
>> -if (cfg) {
>> -const char *encap_tos = smap_get_def(&cfg->external_ids,
>> -   "ovn-encap-tos", "none");
>> -
>> -if (encap_tos && strcmp(encap_tos, "none")) {
>> -smap_add(&options, "tos", encap_tos);
>> -}
>> }
>> 
>> /* If there's an existing chassis record that does not need any change,
>> diff --git a/controller/ovn-controller.8.xml 
>> b/controller/ovn-controller.8.xml
>> index e9708fe64..cc9a7d1c2 100644
>> --- a/controller/ovn-controller.8.xml
>> +++ b/controller/ovn-controller.8.xml
>> @@ -304,6 +304,13 @@
>> of how many entries there are in the cache.  By default this is set 
>> to
>> 3 (30 seconds).
>>   
>> +  external_ids:ovn-set-local-ip
>> +  
>> +The boolean flag indicates if ovn-controller when 
>> create
>> +tunnel ports should set local_ip parameter.  Can be
>> +heplful to pin source outer IP for the tunnel when multiple 
>> interfaces
>> +are used on the host for overlay traffic.
>> +  
>> 
>> 
>> 
>> diff --git a/tests/ovn-controller.at b/tests/ovn-controller.at
>> index 2f39e5f3e..9e6302e5a 100644
>> --- a/tests/ovn-controller.at
>> +++ b/tests/ovn-controller.at
>> @@ -298,6 +298,15 @@ OVS_WAIT_UNTIL([check_tunnel_property type geneve])
>> ovs-vsctl del-port ovn-fakech-0
>> OVS_WAIT_UNTIL([check_tunnel_property type geneve])
>> 
>> +# set `ovn-set-local-ip` option to true and check if tunnel parameters
>> +OVS_WAIT_WHILE([check_tunnel_property options:local_ip "\"192.168.0.1\""])
>> +ovs-vsctl set open . external_ids:ovn-set-local-ip=true
>> +OVS_WAIT_UNTIL([check_tunnel_property options:local_ip "\"192.168.0.1\""])
>> +
>> +# Change the local_ip on the OVS side and check than OVN fixes it
>> +ovs-vsctl set interface ovn-fakech-0 options:local_ip="1.1.1.1"
>> +OVS_WAIT_UNTIL([check_tunnel

[ovs-dev] [PATCH ovn v2] controller: add ovn-set-local-ip option

[Vladislav Odintsov]

When transport node has multiple interfaces (vlans) and
ovn-encap-ip on different hosts need to be configured
from different VLANs source IP for encapsulated packet
can be not the same, which is expected by remote system.

Explicitely setting local_ip resolves such problem.

Signed-off-by: Vladislav Odintsov 
---
 controller/chassis.c| 12 +++
 controller/encaps.c | 37 +
 controller/ovn-controller.8.xml |  7 +++
 ovn-sb.xml  |  9 
 tests/ovn-controller.at |  9 
 5 files changed, 61 insertions(+), 13 deletions(-)

diff --git a/controller/chassis.c b/controller/chassis.c
index 8a1559653..1a3341079 100644
--- a/controller/chassis.c
+++ b/controller/chassis.c
@@ -64,6 +64,8 @@ struct ovs_chassis_cfg {
 struct ds iface_types;
 /* Is this chassis an interconnection gateway. */
 bool is_interconn;
+/* Whether we should configure local_ip tunnel port option.*/
+bool set_local_ip;
 };
 
 static void
@@ -192,6 +194,12 @@ get_is_interconn(const struct smap *ext_ids)
 return smap_get_bool(ext_ids, "ovn-is-interconn", false);
 }
 
+static bool
+get_set_local_ip(const struct smap *ext_ids)
+{
+return smap_get_bool(ext_ids, "ovn-set-local-ip", false);
+}
+
 static void
 update_chassis_transport_zones(const struct sset *transport_zones,
const struct sbrec_chassis *chassis_rec)
@@ -324,6 +332,8 @@ chassis_parse_ovs_config(const struct 
ovsrec_open_vswitch_table *ovs_table,
 
 ovs_cfg->is_interconn = get_is_interconn(&cfg->external_ids);
 
+ovs_cfg->set_local_ip = get_set_local_ip(&cfg->external_ids);
+
 return true;
 }
 
@@ -350,6 +360,8 @@ chassis_build_other_config(const struct ovs_chassis_cfg 
*ovs_cfg,
 smap_replace(config, "is-interconn",
  ovs_cfg->is_interconn ? "true" : "false");
 smap_replace(config, OVN_FEATURE_PORT_UP_NOTIF, "true");
+smap_replace(config, "ovn-set-local-ip",
+ ovs_cfg->set_local_ip ? "true" : "false");
 }
 
 /*
diff --git a/controller/encaps.c b/controller/encaps.c
index 66e0cd8cd..3b0c92931 100644
--- a/controller/encaps.c
+++ b/controller/encaps.c
@@ -23,6 +23,7 @@
 #include "openvswitch/vlog.h"
 #include "lib/ovn-sb-idl.h"
 #include "ovn-controller.h"
+#include "smap.h"
 
 VLOG_DEFINE_THIS_MODULE(encaps);
 
@@ -176,8 +177,31 @@ tunnel_add(struct tunnel_ctx *tc, const struct 
sbrec_sb_global *sbg,
 smap_add(&options, "dst_port", dst_port);
 }
 
+const struct ovsrec_open_vswitch *cfg =
+ovsrec_open_vswitch_table_first(ovs_table);
+
+bool set_local_ip = false;
+if (cfg) {
+/* If the tos option is configured, get it */
+const char *encap_tos = smap_get_def(&cfg->external_ids,
+   "ovn-encap-tos", "none");
+
+if (encap_tos && strcmp(encap_tos, "none")) {
+smap_add(&options, "tos", encap_tos);
+}
+
+/* If ovn-set-local-ip option is configured, get it */
+set_local_ip = smap_get_bool(&cfg->external_ids, "ovn-set-local-ip",
+ false);
+}
+
 /* Add auth info if ipsec is enabled. */
 if (sbg->ipsec) {
+set_local_ip = true;
+smap_add(&options, "remote_name", new_chassis_id);
+}
+
+if (set_local_ip) {
 const struct sbrec_chassis *this_chassis = tc->this_chassis;
 const char *local_ip = NULL;
 
@@ -200,19 +224,6 @@ tunnel_add(struct tunnel_ctx *tc, const struct 
sbrec_sb_global *sbg,
 if (local_ip) {
 smap_add(&options, "local_ip", local_ip);
 }
-smap_add(&options, "remote_name", new_chassis_id);
-}
-
-const struct ovsrec_open_vswitch *cfg =
-ovsrec_open_vswitch_table_first(ovs_table);
-/* If the tos option is configured, get it */
-if (cfg) {
-const char *encap_tos = smap_get_def(&cfg->external_ids,
-   "ovn-encap-tos", "none");
-
-if (encap_tos && strcmp(encap_tos, "none")) {
-smap_add(&options, "tos", encap_tos);
-}
 }
 
 /* If there's an existing chassis record that does not need any change,
diff --git a/controller/ovn-controller.8.xml b/controller/ovn-controller.8.xml
index e9708fe64..cc9a7d1c2 100644
--- a/controller/ovn-controller.8.xml
+++ b/controller/ovn-controller.8.xml
@@ -304,6 +304,13 @@
 of how many entries there are in the cache.  By default this is set to
 3 (30 seconds).
   
+  external_ids:ovn-set-local-ip
+  
+The boolean flag indicates if ovn-controller when create
+tunnel ports should set local_ip parameter.  Can be
+heplful to pin source outer IP for the tunnel when multiple interfaces
+are used on the host for overlay traffic.
+  
 
 
 
diff --git a/ovn-sb.xml b/ovn-sb.xml
index 9ddacdf09..46b8192d8 100644
--- a/ovn-sb.xml
+++ b/ovn-sb.xml
@@ -300,6 +300,15 @@
   See ovn-controller(8) fo

Re: [ovs-dev] [PATCH v2] dpif-netdev-dpcls: Make subtable reprobe thread-safe.

[Ilya Maximets]

On 2/14/22 18:35, Ilya Maximets wrote:
> On 2/14/22 17:39, Stokes, Ian wrote:
>>> The subtable search function can be used at any time by a PMD thread.
>>> Setting the subtable search function should be done atomically to
>>> prevent garbage data from being read.
>>>
>>> A dpcls_subtable_lookup_reprobe() call can happen at the same time that
>>> DPCLS subtables are being sorted. This could lead to modifications done
>>> by the reprobe() to be lost. Prevent this from happening by locking on
>>> pmd->flow_mutex. After this change both the reprobe function and a
>>> subtable sort will share the flow_mutex preventing modifications by
>>> either one from being lost.
>>>
>>> Also remove the pvector_publish() call. The pvector is not being changed
>>> in dpcls_subtable_lookup_reprobe(), only the data pointed to by pointers
>>> in the vector are being changed.
>>
>> Hi Cian,
>>
>> Thanks for the patch, in general this looks ok to me, but I realize Ilya had 
>> a few comments on the v1. I think these are addressed here but maybe Ilya 
>> would like to confirm before sign off?
> 
> Thanks, Ian.
> I'll take a look at this patch tomorrow.
> 
>>
>> @Ilya Maximets should this go into branch 2.17 as well before release?
> 
> I'd say, if it applies cleanly, we should try to backport it down to 2.14,
> because the issue may cause OVS crash.

I didn't test it, but the change looks correct to me.  Thanks!

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

Re: [ovs-dev] [PATCH v3 06/14] bfd: lldp: stp: Fix misaligned packet field access.

[Ilya Maximets]

On 1/24/22 15:18, Dumitru Ceara wrote:
> UB Sanitizer reports:
>   lib/bfd.c:748:16: runtime error: member access within misaligned address 
> 0x01f0d6ea for type 'struct msg', which requires 4 byte alignment
>   0x01f0d6ea: note: pointer points here
>00 20  00 00 20 40 03 18 93 f9  0a 6e 00 00 00 00 00 0f  42 40 00 0f 42 40 
> 00 00  00 00 cc 00 00 00
> ^
>   #0 0x59008e in bfd_process_packet lib/bfd.c:748
>   #1 0x52a240 in process_special ofproto/ofproto-dpif-xlate.c:3370
>   #2 0x553452 in xlate_actions ofproto/ofproto-dpif-xlate.c:7766
>   #3 0x4fc9e6 in upcall_xlate ofproto/ofproto-dpif-upcall.c:1237
>   #4 0x4fdecc in process_upcall ofproto/ofproto-dpif-upcall.c:1456
>   #5 0x4fd936 in upcall_cb ofproto/ofproto-dpif-upcall.c:1358
>   [...]
>   lib/stp.c:754:15: runtime error: member access within misaligned address 
> 0x02c4ea61 for type 'const   struct stp_bpdu_header', which requires 2 
> byte alignment
>   0x02c4ea61: note: pointer points here
>26 42 42  03 00 00 00 00 00 80 00  aa 66 aa 66 00 01 00 00  00 00 80 00 aa 
> 66 aa 66  00 01 80 02 00
> ^
>   #0 0x8a2bce in stp_received_bpdu lib/stp.c:754
>   #1 0x51e603 in stp_process_packet ofproto/ofproto-dpif-xlate.c:1788
>   #2 0x52a96d in process_special ofproto/ofproto-dpif-xlate.c:3394
>   #3 0x5534df in xlate_actions ofproto/ofproto-dpif-xlate.c:7766
>   #4 0x4fcb49 in upcall_xlate ofproto/ofproto-dpif-upcall.c:1237
>   [...]
>   lib/lldp/lldp.c:149:10: runtime error: load of misaligned address 
> 0x7ffcc0ae72bd for type 'ovs_be16', which requires 2 byte alignment
>   0x7ffcc0ae72bd: note: pointer points here
>8e e7 84 ad 04 00 05  46 61 73 74 45 74 68 65  72 6e 65 74 20 31 2f 35  e0 
> 91 07 c9 3e 7f 00 00  80
>^
>   #0 0x718d63 in lldp_tlv_end lib/lldp/lldp.c:149
>   #1 0x7191de in lldp_send lib/lldp/lldp.c:184
>   #2 0x484d6c in test_aa_send tests/test-aa.c:238
>   [...]
> 
> Acked-by: Aaron Conole 
> Signed-off-by: Dumitru Ceara 
> ---
> v3: Added Aaron's ack.
> ---
>  lib/bfd.c   |   51 ---
>  lib/lldp/lldp.c |4 +++-
>  lib/stp.c   |   16 
>  3 files changed, 39 insertions(+), 32 deletions(-)


Thanks!  Applied and backported down to 2.13.

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

Re: [ovs-dev] [PATCH v3 00/14] Fix UndefinedBehaviorSanitizer reported issues and enable it in CI.

[Ilya Maximets]

On 1/24/22 21:10, Dumitru Ceara wrote:
> On 1/24/22 19:40, Adrian Moreno wrote:
>>
>>
>> On 1/24/22 18:49, Jeffrey Walton wrote:
>>> On Mon, Jan 24, 2022 at 9:17 AM Dumitru Ceara  wrote:

 As privately reported by Aaron Conole, and by Jeffrey Walton [0]
 there's currently a number of undefined behavior instances in
 the OVS code base.  Running the OVS (and OVN) tests with UBSan [1]
 enabled uncovers these.
 ...
>>>
 Note: depending on the order of reviews, if Adrian's "Fix undefined
 behavior in loop macros" series [2] (or a follow up) is accepted first,
 then patch 12/14 ("util: Avoid false positive UB when iterating
 collections.") can be skipped.  Adrian's series seems to be the more
 correct way of fixing the issue.
>>>
>>> One small nit... UBsan (and Asan) do not produce false positives. They
>>> operate on real data, and when they produce a finding it is valid.
>>> That's also why a complete set of self tests are important. The
>>> complete set of tests are important because UBsan and Asan need real
>>> data.
>>>
>>
>> I agree, it's not a false positive. Furthermore, the patch that Dumitru
>> is referring to ("util: Avoid false positive UB when iterating
>> collections") reduces UBsan's sensitivity by changing some pointer
>> arithmetics to integer arithmetics. This silences the UBsan but
>> according to the discussion with the compiler folks [1], this can still
>> yield UB.
>>
>> Therefore, I think it would be safer to keep the pointer arithmetics
>> (and UBsan's high-sensitivity), fix the actual callers (i.e: the
>> iterator macros), and run UBsan in the CI to spot all future errors
>> (which Dumitru's series does).
>>
>> [1]https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103964
>>
> 
> Yes, I probably should've rephrased the commit title of patch 12/14.
> It's not a false positive.  I just kept it for now until Adrian's series
> [2] is merged.  Otherwise jobs would've failed in CI, and it's
> technically not worse than before.
> 
> But I completely agree, once Adrian's changes get accepted, the safest
> way is to keep the pointer arithmetic and rely on UBSan to complain if
> undefined behavior is detected.
> 
> [2]
> https://patchwork.ozlabs.org/project/openvswitch/list/?series=282481&state=*
> 

For now I applied 7 out of 14 patches from this series and replied with
a comments to a few of the remaining patches.
I still have a couple of ofp patches that I didn't look close enough yet.

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

Re: [ovs-dev] [PATCH v3 13/14] ci: Fix typo in variable name.

[Ilya Maximets]

On 2/3/22 20:07, Paolo Valerio wrote:
> Dumitru Ceara  writes:
> 
>> Reported-by: David Marchand 
>> Signed-off-by: Dumitru Ceara 
>> ---
>>  .ci/linux-build.sh |4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/.ci/linux-build.sh b/.ci/linux-build.sh
>> index 8d111b6d7f6b..6cd38ff3efb5 100755
>> --- a/.ci/linux-build.sh
>> +++ b/.ci/linux-build.sh
>> @@ -246,8 +246,8 @@ if [ "$ASAN" ]; then
>>  export ASAN_OPTIONS='detect_leaks=1'
>>  # -O2 generates few false-positive memory leak reports in test-ovsdb
>>  # application, so lowering optimizations to -O1 here.
>> -CLFAGS_ASAN="-O1 -fno-omit-frame-pointer -fno-common -fsanitize=address"
>> -CFLAGS_FOR_OVS="${CFLAGS_FOR_OVS} ${CLFAGS_ASAN}"
>> +CFLAGS_ASAN="-O1 -fno-omit-frame-pointer -fno-common -fsanitize=address"
>> +CFLAGS_FOR_OVS="${CFLAGS_FOR_OVS} ${CFLAGS_ASAN}"
>>  fi
>>  
>>  save_OPTS="${OPTS} $*"
>>
> 
> Acked-by: Paolo Valerio 
> 


Thanks!  Applied and backported down to 2.16.

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

Re: [ovs-dev] [PATCH v3 07/14] dp-packet: Ensure packet base is always non-NULL.

[Ilya Maximets]

On 2/3/22 19:59, Paolo Valerio wrote:
> Dumitru Ceara  writes:
> 
>> UB Sanitizer report:
>>   lib/dp-packet.h:297:39: runtime error: applying zero offset to null pointer
>>   #0 0x7946f5 in dp_packet_tail /root/ovs/./lib/dp-packet.h:297:39
>>   #1 0x794331 in dp_packet_tailroom /root/ovs/./lib/dp-packet.h:325:49
>>   #2 0x7942a0 in dp_packet_prealloc_tailroom 
>> /root/ovs/lib/dp-packet.c:297:16
>>   #3 0xc347cf in eth_compose /root/ovs/lib/packets.c:1061:5
>>   [...]
>>
>> Signed-off-by: Dumitru Ceara 
>> ---
>> v3: Implement Aaron's suggestion instead and fix
>> dp_packet_prealloc_tailroom().
>> ---
>>  lib/dp-packet.c |2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
> 
> Acked-by: Paolo Valerio 
> 


Thanks!  Applied and backported down to 2.13.

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

Re: [ovs-dev] [PATCH v3 05/14] ovsdb-idlc: Avoid accessing member within NULL idl index cursors.

[Ilya Maximets]

On 2/3/22 20:05, Paolo Valerio wrote:
> Dumitru Ceara  writes:
> 
>> Reported by UndefinedBehaviorSanitizer:
>>   tests/idltest.c:3602:12: runtime error: member access within null pointer 
>> of type 'const struct idltest_simple'
>>   #0 0x4295af in idltest_simple_cursor_first_ge tests/idltest.c:3602
>>   #1 0x41c81b in test_idl_compound_index_single_column 
>> tests/test-ovsdb.c:3128
>>   #2 0x41e035 in do_idl_compound_index tests/test-ovsdb.c:3277
>>   #3 0x4cf640 in ovs_cmdl_run_command__ lib/command-line.c:247
>>   #4 0x4cf79f in ovs_cmdl_run_command lib/command-line.c:278
>>   #5 0x4072f7 in main tests/test-ovsdb.c:79
>>   #6 0x7fa858675b74 in __libc_start_main (/lib64/libc.so.6+0x27b74)
>>   #7 0x4060ed in _start (/root/ovs/tests/test-ovsdb+0x4060ed)
>>
>> Acked-by: Aaron Conole 
>> Signed-off-by: Dumitru Ceara 
>> ---
> 
> Acked-by: Paolo Valerio 
> 

Thanks!  Applied and backported down to 2.13.

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

Re: [ovs-dev] [PATCH v3 03/14] stopwatch: Fix buffer underflow when computing percentiles.

[Ilya Maximets]

On 2/3/22 20:02, Paolo Valerio wrote:
> Dumitru Ceara  writes:
> 
>> UB Sanitizer report:
>>   lib/stopwatch.c:119:22: runtime error: index 18446744073709551615 out of 
>> bounds for type 'long long unsigned int [50]'
>>   #0 0x698358 in calc_percentile lib/stopwatch.c:119
>>   #1 0x69ada1 in add_sample lib/stopwatch.c:231
>>   #2 0x69c086 in stopwatch_end_sample_protected lib/stopwatch.c:386
>>   #3 0x69c522 in stopwatch_thread lib/stopwatch.c:441
>>   #4 0x684bae in ovsthread_wrapper lib/ovs-thread.c:383
>>   #5 0x7f042838b298 in start_thread (/lib64/libpthread.so.0+0x9298)
>>   #6 0x7f04277f2352 in clone (/lib64/libc.so.6+0x100352)
>>
>> Acked-by: Aaron Conole 
>> Signed-off-by: Dumitru Ceara 
>> ---
> 
> LGTM,
> 
> Acked-by: Paolo Valerio 
> 

Thanks!  Applied and backported down to 2.13.

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

Re: [ovs-dev] [PATCH v3 02/14] dpif-netdev: Fix misaligned access.

[Ilya Maximets]

On 2/3/22 18:13, Paolo Valerio wrote:
> Dumitru Ceara  writes:
> 
>> Remove the forced cache-line size alignment markers from
>> struct dp_netdev_pmd_thread and struct dp_netdev as discussed
>> at [0].  They don't seem to add any benefit and cause 64 byte
>> alignment requirements.
>>
>> UB Sanitizer report:
>>   lib/dpif-netdev.c:6758:13: runtime error: member access within misaligned 
>> address 0x7f7f24d25010 for type 'struct dp_netdev_pmd_thread', which 
>> requires 64 byte alignment
>>   0x7f7f24d25010: note: pointer points here
>>00 00 00 00  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  00 00 00 
>> 00 00 00 00 00  00 00 00 00
>> ^
>>  #0 0x5fbfde in dp_netdev_configure_pmd lib/dpif-netdev.c:6758
>>  #1 0x5fbde9 in dp_netdev_set_nonpmd lib/dpif-netdev.c:6715
>>  #2 0x5d6fdd in create_dp_netdev lib/dpif-netdev.c:1769
>>  #3 0x5d72d0 in dpif_netdev_open lib/dpif-netdev.c:1807
>>  #4 0x61c83f in do_open lib/dpif.c:347
>>  [...]
>>   lib/dpif-netdev.c:1724:6: runtime error: member access within misaligned 
>> address 0x02005eb0 for type 'struct dp_netdev', which requires 64 byte 
>> alignment
>>   0x02005eb0: note: pointer points here
>>00 00 00 00  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  00 00 00 
>> 00 00 00 00 00  00 00 00 00
>> ^
>>   #0 0x5d6660 in create_dp_netdev lib/dpif-netdev.c:1724
>>   #1 0x5d72d0 in dpif_netdev_open lib/dpif-netdev.c:1807
>>   #2 0x61c846 in do_open lib/dpif.c:347
>>   #3 0x61ca9c in dpif_create lib/dpif.c:402
>>   #4 0x61cac9 in dpif_create_and_open lib/dpif.c:415
>>   #5 0x48f235 in open_dpif_backer ofproto/ofproto-dpif.c:776
>>   [...]
>>
>> [0] https://mail.openvswitch.org/pipermail/ovs-dev/2021-December/390256.html
>>
>> Suggested-by: Ilya Maximets 
>> Signed-off-by: Dumitru Ceara 
>> ---
> 
> Acked-by: Paolo Valerio 
> 

Thanks!  Applied to master and 2.17.

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

Re: [ovs-dev] [PATCH v3 01/14] treewide: Don't pass NULL to library functions that expect non-NULL.

[Ilya Maximets]

On 2/3/22 17:33, Paolo Valerio wrote:
> Dumitru Ceara  writes:
> 
>> It's actually undefined behavior to pass NULL to standard library
>> functions that manipulate arrays (e.g., qsort, memcpy, memcmp), even if
>> the passed number of items is 0.
>>
>> UB Sanitizer reports:
>>   ovsdb/monitor.c:408:9: runtime error: null pointer passed as argument 1, 
>> which is declared to never be null
>>   #0 0x406ae1 in ovsdb_monitor_columns_sort ovsdb/monitor.c:408
>>   #1 0x406ae1 in ovsdb_monitor_add ovsdb/monitor.c:1683
>>   [...]
>>   lib/ovsdb-data.c:1970:5: runtime error: null pointer passed as argument 2, 
>> which is declared to never be null
>>   #0 0x4071c8 in ovsdb_datum_push_unsafe lib/ovsdb-data.c:1970
>>   #1 0x471cd0 in ovsdb_datum_apply_diff_in_place lib/ovsdb-data.c:2345
>>   [...]
>>   ofproto/ofproto-dpif-rid.c:159:17: runtime error: null pointer passed as 
>> argument 1, which is declared to never be null
>>   #0 0x4df5d8 in frozen_state_equal ofproto/ofproto-dpif-rid.c:159
>>   #1 0x4dfd27 in recirc_find_equal ofproto/ofproto-dpif-rid.c:179
>>   [...]
>>
>> Acked-by: Aaron Conole 
>> Signed-off-by: Dumitru Ceara 
>> ---
> 
> Acked-by: Paolo Valerio 
> 

Thanks!  Applied to master and 2.17.

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

Re: [ovs-dev] [PATCH v2 05/16] list: use short version of safe loops if possible

[Ilya Maximets]

On 2/15/22 13:58, Adrian Moreno wrote:
> 
> 
> On 2/15/22 13:54, Ilya Maximets wrote:
>> On 2/15/22 13:49, Adrian Moreno wrote:
>>>
>>>
>>> On 2/15/22 13:42, Adrian Moreno wrote:


 On 2/14/22 18:30, Ilya Maximets wrote:
> On 2/14/22 11:13, Adrian Moreno wrote:
>> Using the SHORT version of the *_SAFE loops makes the code cleaner
>> and less error-prone. So, use the SHORT version and remove the extra
>> variable when possible.
>>
>> In order to be able to use both long and short versions without changing
>> the name of the macro for all the clients, overload the existing name
>> and select the appropriate version depending on the number of arguments.
>>
>> Signed-off-by: Adrian Moreno 
>> ---
>>    include/openvswitch/list.h   | 32 +--
>>    lib/conntrack.c  |  4 ++--
>>    lib/fat-rwlock.c |  4 ++--
>>    lib/id-fpool.c   |  3 +--
>>    lib/ipf.c    | 22 ++---
>>    lib/lldp/lldpd-structs.c |  7 +++
>>    lib/lldp/lldpd.c |  8 
>>    lib/mcast-snooping.c | 12 ++--
>>    lib/netdev-afxdp.c   |  4 ++--
>>    lib/netdev-dpdk.c    |  4 ++--
>>    lib/ofp-msgs.c   |  4 ++--
>>    lib/ovs-lldp.c   | 12 ++--
>>    lib/ovsdb-idl.c  | 30 ++---
>>    lib/seq.c    |  4 ++--
>>    lib/tnl-ports.c  | 16 
>>    lib/unixctl.c    |  8 
>>    lib/vconn.c  |  4 ++--
>>    ofproto/connmgr.c    |  8 
>>    ofproto/ofproto-dpif-ipfix.c |  4 ++--
>>    ofproto/ofproto-dpif-trace.c |  4 ++--
>>    ofproto/ofproto-dpif-xlate.c |  4 ++--
>>    ofproto/ofproto-dpif.c   | 24 +++
>>    ovsdb/jsonrpc-server.c   | 16 
>>    ovsdb/monitor.c  | 24 +++
>>    ovsdb/ovsdb.c    |  4 ++--
>>    ovsdb/raft.c | 15 +++
>>    ovsdb/transaction-forward.c  |  6 +++---
>>    ovsdb/transaction.c  | 28 +--
>>    ovsdb/trigger.c  |  4 ++--
>>    tests/test-list.c    | 37 
>>    utilities/ovs-ofctl.c    |  4 ++--
>>    utilities/ovs-vsctl.c    |  8 
>>    vswitchd/bridge.c    | 16 
>>    vtep/vtep-ctl.c  | 12 ++--
>>    34 files changed, 228 insertions(+), 168 deletions(-)
>>
>> diff --git a/include/openvswitch/list.h b/include/openvswitch/list.h
>> index 997afc0e4..c6941e896 100644
>> --- a/include/openvswitch/list.h
>> +++ b/include/openvswitch/list.h
>> @@ -93,7 +93,8 @@ static inline bool ovs_list_is_short(const struct 
>> ovs_list *);
>>     CONDITION_MULTIVAR(VAR, MEMBER, ITER_VAR(VAR) != (LIST));
>>     \
>>     UPDATE_MULTIVAR(VAR, (ITER_VAR(VAR) = ITER_VAR(VAR)->prev)))
>> -#define LIST_FOR_EACH_REVERSE_SAFE(VAR, PREV, MEMBER, LIST) 
>>   \
>> +/* LONG version of SAFE iterators */
>> +#define LIST_FOR_EACH_REVERSE_SAFE_LONG(VAR, PREV, MEMBER, LIST)
>>  \
>>    for (INIT_MULTIVAR_SAFE_LONG(VAR, PREV, MEMBER, (LIST)->prev);
>>     \
>>     CONDITION_MULTIVAR_SAFE_LONG(VAR, PREV, MEMBER,  
>>     \
>>  ITER_VAR(VAR) != (LIST),
>>     \
>> @@ -101,7 +102,7 @@ static inline bool ovs_list_is_short(const struct 
>> ovs_list *);
>>  ITER_VAR(PREV) != (LIST));  
>>     \
>>     UPDATE_MULTIVAR_SAFE_LONG(VAR, PREV))
>> -#define LIST_FOR_EACH_SAFE(VAR, NEXT, MEMBER, LIST) 
>>   \
>> +#define LIST_FOR_EACH_SAFE_LONG(VAR, NEXT, MEMBER, LIST)
>>   \
>>    for (INIT_MULTIVAR_SAFE_LONG(VAR, NEXT, MEMBER, (LIST)->next);
>>     \
>>     CONDITION_MULTIVAR_SAFE_LONG(VAR, NEXT, MEMBER,  
>>     \
>>  ITER_VAR(VAR) != (LIST),
>>     \
>> @@ -109,6 +110,33 @@ static inline bool ovs_list_is_short(const struct 
>> ovs_list *);
>>  ITER_VAR(NEXT) != (LIST));  
>>     \
>>     UPDATE_MULTIVAR_SAFE_LONG(VAR, NEXT))
>> +/* SHORT version of SAFE iterators */
>> +#define LIST_FOR_EACH_REVERSE_SAFE_SHORT(VAR, MEMBER, LIST) 
>>   \
>> +    for (INIT_MULTIVAR_SAFE_SHORT(VAR, MEMBER, (LIST)->prev);   
>>   \
>> + CO

Re: [ovs-dev] [PATCH v2 05/16] list: use short version of safe loops if possible

[Adrian Moreno]

On 2/15/22 13:54, Ilya Maximets wrote:

On 2/15/22 13:49, Adrian Moreno wrote:



On 2/15/22 13:42, Adrian Moreno wrote:



On 2/14/22 18:30, Ilya Maximets wrote:

On 2/14/22 11:13, Adrian Moreno wrote:

Using the SHORT version of the *_SAFE loops makes the code cleaner
and less error-prone. So, use the SHORT version and remove the extra
variable when possible.

In order to be able to use both long and short versions without changing
the name of the macro for all the clients, overload the existing name
and select the appropriate version depending on the number of arguments.

Signed-off-by: Adrian Moreno 
---
   include/openvswitch/list.h   | 32 +--
   lib/conntrack.c  |  4 ++--
   lib/fat-rwlock.c |  4 ++--
   lib/id-fpool.c   |  3 +--
   lib/ipf.c    | 22 ++---
   lib/lldp/lldpd-structs.c |  7 +++
   lib/lldp/lldpd.c |  8 
   lib/mcast-snooping.c | 12 ++--
   lib/netdev-afxdp.c   |  4 ++--
   lib/netdev-dpdk.c    |  4 ++--
   lib/ofp-msgs.c   |  4 ++--
   lib/ovs-lldp.c   | 12 ++--
   lib/ovsdb-idl.c  | 30 ++---
   lib/seq.c    |  4 ++--
   lib/tnl-ports.c  | 16 
   lib/unixctl.c    |  8 
   lib/vconn.c  |  4 ++--
   ofproto/connmgr.c    |  8 
   ofproto/ofproto-dpif-ipfix.c |  4 ++--
   ofproto/ofproto-dpif-trace.c |  4 ++--
   ofproto/ofproto-dpif-xlate.c |  4 ++--
   ofproto/ofproto-dpif.c   | 24 +++
   ovsdb/jsonrpc-server.c   | 16 
   ovsdb/monitor.c  | 24 +++
   ovsdb/ovsdb.c    |  4 ++--
   ovsdb/raft.c | 15 +++
   ovsdb/transaction-forward.c  |  6 +++---
   ovsdb/transaction.c  | 28 +--
   ovsdb/trigger.c  |  4 ++--
   tests/test-list.c    | 37 
   utilities/ovs-ofctl.c    |  4 ++--
   utilities/ovs-vsctl.c    |  8 
   vswitchd/bridge.c    | 16 
   vtep/vtep-ctl.c  | 12 ++--
   34 files changed, 228 insertions(+), 168 deletions(-)

diff --git a/include/openvswitch/list.h b/include/openvswitch/list.h
index 997afc0e4..c6941e896 100644
--- a/include/openvswitch/list.h
+++ b/include/openvswitch/list.h
@@ -93,7 +93,8 @@ static inline bool ovs_list_is_short(const struct ovs_list *);
    CONDITION_MULTIVAR(VAR, MEMBER, ITER_VAR(VAR) != (LIST));   
 \
    UPDATE_MULTIVAR(VAR, (ITER_VAR(VAR) = ITER_VAR(VAR)->prev)))
-#define LIST_FOR_EACH_REVERSE_SAFE(VAR, PREV, MEMBER, LIST)   \
+/* LONG version of SAFE iterators */
+#define LIST_FOR_EACH_REVERSE_SAFE_LONG(VAR, PREV, MEMBER, LIST) \
   for (INIT_MULTIVAR_SAFE_LONG(VAR, PREV, MEMBER, (LIST)->prev);   
 \
    CONDITION_MULTIVAR_SAFE_LONG(VAR, PREV, MEMBER, 
 \
     ITER_VAR(VAR) != (LIST),   
 \
@@ -101,7 +102,7 @@ static inline bool ovs_list_is_short(const struct ovs_list 
*);
     ITER_VAR(PREV) != (LIST)); 
 \
    UPDATE_MULTIVAR_SAFE_LONG(VAR, PREV))
-#define LIST_FOR_EACH_SAFE(VAR, NEXT, MEMBER, LIST)   \
+#define LIST_FOR_EACH_SAFE_LONG(VAR, NEXT, MEMBER, LIST)  \
   for (INIT_MULTIVAR_SAFE_LONG(VAR, NEXT, MEMBER, (LIST)->next);   
 \
    CONDITION_MULTIVAR_SAFE_LONG(VAR, NEXT, MEMBER, 
 \
     ITER_VAR(VAR) != (LIST),   
 \
@@ -109,6 +110,33 @@ static inline bool ovs_list_is_short(const struct ovs_list 
*);
     ITER_VAR(NEXT) != (LIST)); 
 \
    UPDATE_MULTIVAR_SAFE_LONG(VAR, NEXT))
+/* SHORT version of SAFE iterators */
+#define LIST_FOR_EACH_REVERSE_SAFE_SHORT(VAR, MEMBER, LIST)   \
+    for (INIT_MULTIVAR_SAFE_SHORT(VAR, MEMBER, (LIST)->prev); \
+ CONDITION_MULTIVAR_SAFE_SHORT(VAR, MEMBER,   \
+   ITER_VAR(VAR) != (LIST),   \
+ ITER_NEXT_VAR(VAR) = ITER_VAR(VAR)->prev);   \
+ UPDATE_MULTIVAR_SAFE_SHORT(VAR))
+
+#define LIST_FOR_EACH_SAFE_SHORT(VAR, MEMBER, LIST)   \
+    for (INIT_MULTIVAR_SAFE_SHORT(VAR, MEMBER, (LIST)->next); \
+ CONDITION_MULTIVAR_SAFE_SHORT(VAR, MEMBER,   \
+   ITER_VAR(VAR) != (LIST),   \
+ ITER_NEXT_VAR(VAR) = ITER_VAR(VAR)->next);   \
+ UPDATE_MULTIVAR_SAFE_SHORT(V

Re: [ovs-dev] [PATCH v2 05/16] list: use short version of safe loops if possible

[Ilya Maximets]

On 2/15/22 13:49, Adrian Moreno wrote:
> 
> 
> On 2/15/22 13:42, Adrian Moreno wrote:
>>
>>
>> On 2/14/22 18:30, Ilya Maximets wrote:
>>> On 2/14/22 11:13, Adrian Moreno wrote:
 Using the SHORT version of the *_SAFE loops makes the code cleaner
 and less error-prone. So, use the SHORT version and remove the extra
 variable when possible.

 In order to be able to use both long and short versions without changing
 the name of the macro for all the clients, overload the existing name
 and select the appropriate version depending on the number of arguments.

 Signed-off-by: Adrian Moreno 
 ---
   include/openvswitch/list.h   | 32 +--
   lib/conntrack.c  |  4 ++--
   lib/fat-rwlock.c |  4 ++--
   lib/id-fpool.c   |  3 +--
   lib/ipf.c    | 22 ++---
   lib/lldp/lldpd-structs.c |  7 +++
   lib/lldp/lldpd.c |  8 
   lib/mcast-snooping.c | 12 ++--
   lib/netdev-afxdp.c   |  4 ++--
   lib/netdev-dpdk.c    |  4 ++--
   lib/ofp-msgs.c   |  4 ++--
   lib/ovs-lldp.c   | 12 ++--
   lib/ovsdb-idl.c  | 30 ++---
   lib/seq.c    |  4 ++--
   lib/tnl-ports.c  | 16 
   lib/unixctl.c    |  8 
   lib/vconn.c  |  4 ++--
   ofproto/connmgr.c    |  8 
   ofproto/ofproto-dpif-ipfix.c |  4 ++--
   ofproto/ofproto-dpif-trace.c |  4 ++--
   ofproto/ofproto-dpif-xlate.c |  4 ++--
   ofproto/ofproto-dpif.c   | 24 +++
   ovsdb/jsonrpc-server.c   | 16 
   ovsdb/monitor.c  | 24 +++
   ovsdb/ovsdb.c    |  4 ++--
   ovsdb/raft.c | 15 +++
   ovsdb/transaction-forward.c  |  6 +++---
   ovsdb/transaction.c  | 28 +--
   ovsdb/trigger.c  |  4 ++--
   tests/test-list.c    | 37 
   utilities/ovs-ofctl.c    |  4 ++--
   utilities/ovs-vsctl.c    |  8 
   vswitchd/bridge.c    | 16 
   vtep/vtep-ctl.c  | 12 ++--
   34 files changed, 228 insertions(+), 168 deletions(-)

 diff --git a/include/openvswitch/list.h b/include/openvswitch/list.h
 index 997afc0e4..c6941e896 100644
 --- a/include/openvswitch/list.h
 +++ b/include/openvswitch/list.h
 @@ -93,7 +93,8 @@ static inline bool ovs_list_is_short(const struct 
 ovs_list *);
    CONDITION_MULTIVAR(VAR, MEMBER, ITER_VAR(VAR) != (LIST));   
  \
    UPDATE_MULTIVAR(VAR, (ITER_VAR(VAR) = ITER_VAR(VAR)->prev)))
 -#define LIST_FOR_EACH_REVERSE_SAFE(VAR, PREV, MEMBER, LIST)   
     \
 +/* LONG version of SAFE iterators */
 +#define LIST_FOR_EACH_REVERSE_SAFE_LONG(VAR, PREV, MEMBER, LIST)  
    \
   for (INIT_MULTIVAR_SAFE_LONG(VAR, PREV, MEMBER, (LIST)->prev);   
  \
    CONDITION_MULTIVAR_SAFE_LONG(VAR, PREV, MEMBER, 
  \
     ITER_VAR(VAR) != (LIST),   
  \
 @@ -101,7 +102,7 @@ static inline bool ovs_list_is_short(const struct 
 ovs_list *);
     ITER_VAR(PREV) != (LIST)); 
  \
    UPDATE_MULTIVAR_SAFE_LONG(VAR, PREV))
 -#define LIST_FOR_EACH_SAFE(VAR, NEXT, MEMBER, LIST)   
     \
 +#define LIST_FOR_EACH_SAFE_LONG(VAR, NEXT, MEMBER, LIST)  
     \
   for (INIT_MULTIVAR_SAFE_LONG(VAR, NEXT, MEMBER, (LIST)->next);   
  \
    CONDITION_MULTIVAR_SAFE_LONG(VAR, NEXT, MEMBER, 
  \
     ITER_VAR(VAR) != (LIST),   
  \
 @@ -109,6 +110,33 @@ static inline bool ovs_list_is_short(const struct 
 ovs_list *);
     ITER_VAR(NEXT) != (LIST)); 
  \
    UPDATE_MULTIVAR_SAFE_LONG(VAR, NEXT))
 +/* SHORT version of SAFE iterators */
 +#define LIST_FOR_EACH_REVERSE_SAFE_SHORT(VAR, MEMBER, LIST)   
     \
 +    for (INIT_MULTIVAR_SAFE_SHORT(VAR, MEMBER, (LIST)->prev); 
     \
 + CONDITION_MULTIVAR_SAFE_SHORT(VAR, MEMBER,   
     \
 +   ITER_VAR(VAR) != (LIST),   
     \
 + ITER_NEXT_VAR(VAR) = 
 ITER_VAR(VAR)->prev);   \
 + UPDATE_MULTIVAR_SAFE_SHORT(VAR))
 +
 +#define LIST

Re: [ovs-dev] [PATCH v2 05/16] list: use short version of safe loops if possible

[Adrian Moreno]

On 2/15/22 13:42, Adrian Moreno wrote:



On 2/14/22 18:30, Ilya Maximets wrote:

On 2/14/22 11:13, Adrian Moreno wrote:

Using the SHORT version of the *_SAFE loops makes the code cleaner
and less error-prone. So, use the SHORT version and remove the extra
variable when possible.

In order to be able to use both long and short versions without changing
the name of the macro for all the clients, overload the existing name
and select the appropriate version depending on the number of arguments.

Signed-off-by: Adrian Moreno 
---
  include/openvswitch/list.h   | 32 +--
  lib/conntrack.c  |  4 ++--
  lib/fat-rwlock.c |  4 ++--
  lib/id-fpool.c   |  3 +--
  lib/ipf.c    | 22 ++---
  lib/lldp/lldpd-structs.c |  7 +++
  lib/lldp/lldpd.c |  8 
  lib/mcast-snooping.c | 12 ++--
  lib/netdev-afxdp.c   |  4 ++--
  lib/netdev-dpdk.c    |  4 ++--
  lib/ofp-msgs.c   |  4 ++--
  lib/ovs-lldp.c   | 12 ++--
  lib/ovsdb-idl.c  | 30 ++---
  lib/seq.c    |  4 ++--
  lib/tnl-ports.c  | 16 
  lib/unixctl.c    |  8 
  lib/vconn.c  |  4 ++--
  ofproto/connmgr.c    |  8 
  ofproto/ofproto-dpif-ipfix.c |  4 ++--
  ofproto/ofproto-dpif-trace.c |  4 ++--
  ofproto/ofproto-dpif-xlate.c |  4 ++--
  ofproto/ofproto-dpif.c   | 24 +++
  ovsdb/jsonrpc-server.c   | 16 
  ovsdb/monitor.c  | 24 +++
  ovsdb/ovsdb.c    |  4 ++--
  ovsdb/raft.c | 15 +++
  ovsdb/transaction-forward.c  |  6 +++---
  ovsdb/transaction.c  | 28 +--
  ovsdb/trigger.c  |  4 ++--
  tests/test-list.c    | 37 
  utilities/ovs-ofctl.c    |  4 ++--
  utilities/ovs-vsctl.c    |  8 
  vswitchd/bridge.c    | 16 
  vtep/vtep-ctl.c  | 12 ++--
  34 files changed, 228 insertions(+), 168 deletions(-)

diff --git a/include/openvswitch/list.h b/include/openvswitch/list.h
index 997afc0e4..c6941e896 100644
--- a/include/openvswitch/list.h
+++ b/include/openvswitch/list.h
@@ -93,7 +93,8 @@ static inline bool ovs_list_is_short(const struct ovs_list *);
   CONDITION_MULTIVAR(VAR, MEMBER, ITER_VAR(VAR) != 
(LIST));    \

   UPDATE_MULTIVAR(VAR, (ITER_VAR(VAR) = ITER_VAR(VAR)->prev)))
-#define LIST_FOR_EACH_REVERSE_SAFE(VAR, PREV, MEMBER, LIST)   \
+/* LONG version of SAFE iterators */
+#define LIST_FOR_EACH_REVERSE_SAFE_LONG(VAR, PREV, MEMBER, LIST) \
  for (INIT_MULTIVAR_SAFE_LONG(VAR, PREV, MEMBER, 
(LIST)->prev);    \
   CONDITION_MULTIVAR_SAFE_LONG(VAR, PREV, 
MEMBER,  \
    ITER_VAR(VAR) != 
(LIST),    \
@@ -101,7 +102,7 @@ static inline bool ovs_list_is_short(const struct 
ovs_list *);
    ITER_VAR(PREV) != 
(LIST));  \

   UPDATE_MULTIVAR_SAFE_LONG(VAR, PREV))
-#define LIST_FOR_EACH_SAFE(VAR, NEXT, MEMBER, LIST)   \
+#define LIST_FOR_EACH_SAFE_LONG(VAR, NEXT, MEMBER, LIST)  \
  for (INIT_MULTIVAR_SAFE_LONG(VAR, NEXT, MEMBER, 
(LIST)->next);    \
   CONDITION_MULTIVAR_SAFE_LONG(VAR, NEXT, 
MEMBER,  \
    ITER_VAR(VAR) != 
(LIST),    \
@@ -109,6 +110,33 @@ static inline bool ovs_list_is_short(const struct 
ovs_list *);
    ITER_VAR(NEXT) != 
(LIST));  \

   UPDATE_MULTIVAR_SAFE_LONG(VAR, NEXT))
+/* SHORT version of SAFE iterators */
+#define LIST_FOR_EACH_REVERSE_SAFE_SHORT(VAR, MEMBER, LIST)   \
+    for (INIT_MULTIVAR_SAFE_SHORT(VAR, MEMBER, (LIST)->prev); \
+ CONDITION_MULTIVAR_SAFE_SHORT(VAR, MEMBER,   \
+   ITER_VAR(VAR) != (LIST),   \
+ ITER_NEXT_VAR(VAR) = ITER_VAR(VAR)->prev);   \
+ UPDATE_MULTIVAR_SAFE_SHORT(VAR))
+
+#define LIST_FOR_EACH_SAFE_SHORT(VAR, MEMBER, LIST)   \
+    for (INIT_MULTIVAR_SAFE_SHORT(VAR, MEMBER, (LIST)->next); \
+ CONDITION_MULTIVAR_SAFE_SHORT(VAR, MEMBER,   \
+   ITER_VAR(VAR) != (LIST),   \
+ ITER_NEXT_VAR(VAR) = ITER_VAR(VAR)->next);   \
+ UPDATE_MULTIVAR_SAFE_SHORT(VAR))
+
+/* Select the right SAFE macro depending on the number of arguments .*/
+#define LIST_GET_SAFE_MACRO(_1, _2, _3, _4, N

Re: [ovs-dev] [PATCH v2 05/16] list: use short version of safe loops if possible

[Adrian Moreno]

On 2/14/22 18:30, Ilya Maximets wrote:

On 2/14/22 11:13, Adrian Moreno wrote:

Using the SHORT version of the *_SAFE loops makes the code cleaner
and less error-prone. So, use the SHORT version and remove the extra
variable when possible.

In order to be able to use both long and short versions without changing
the name of the macro for all the clients, overload the existing name
and select the appropriate version depending on the number of arguments.

Signed-off-by: Adrian Moreno 
---
  include/openvswitch/list.h   | 32 +--
  lib/conntrack.c  |  4 ++--
  lib/fat-rwlock.c |  4 ++--
  lib/id-fpool.c   |  3 +--
  lib/ipf.c| 22 ++---
  lib/lldp/lldpd-structs.c |  7 +++
  lib/lldp/lldpd.c |  8 
  lib/mcast-snooping.c | 12 ++--
  lib/netdev-afxdp.c   |  4 ++--
  lib/netdev-dpdk.c|  4 ++--
  lib/ofp-msgs.c   |  4 ++--
  lib/ovs-lldp.c   | 12 ++--
  lib/ovsdb-idl.c  | 30 ++---
  lib/seq.c|  4 ++--
  lib/tnl-ports.c  | 16 
  lib/unixctl.c|  8 
  lib/vconn.c  |  4 ++--
  ofproto/connmgr.c|  8 
  ofproto/ofproto-dpif-ipfix.c |  4 ++--
  ofproto/ofproto-dpif-trace.c |  4 ++--
  ofproto/ofproto-dpif-xlate.c |  4 ++--
  ofproto/ofproto-dpif.c   | 24 +++
  ovsdb/jsonrpc-server.c   | 16 
  ovsdb/monitor.c  | 24 +++
  ovsdb/ovsdb.c|  4 ++--
  ovsdb/raft.c | 15 +++
  ovsdb/transaction-forward.c  |  6 +++---
  ovsdb/transaction.c  | 28 +--
  ovsdb/trigger.c  |  4 ++--
  tests/test-list.c| 37 
  utilities/ovs-ofctl.c|  4 ++--
  utilities/ovs-vsctl.c|  8 
  vswitchd/bridge.c| 16 
  vtep/vtep-ctl.c  | 12 ++--
  34 files changed, 228 insertions(+), 168 deletions(-)

diff --git a/include/openvswitch/list.h b/include/openvswitch/list.h
index 997afc0e4..c6941e896 100644
--- a/include/openvswitch/list.h
+++ b/include/openvswitch/list.h
@@ -93,7 +93,8 @@ static inline bool ovs_list_is_short(const struct ovs_list *);
   CONDITION_MULTIVAR(VAR, MEMBER, ITER_VAR(VAR) != (LIST));
\
   UPDATE_MULTIVAR(VAR, (ITER_VAR(VAR) = ITER_VAR(VAR)->prev)))
  
-#define LIST_FOR_EACH_REVERSE_SAFE(VAR, PREV, MEMBER, LIST)   \

+/* LONG version of SAFE iterators */
+#define LIST_FOR_EACH_REVERSE_SAFE_LONG(VAR, PREV, MEMBER, LIST) \
  for (INIT_MULTIVAR_SAFE_LONG(VAR, PREV, MEMBER, (LIST)->prev);
\
   CONDITION_MULTIVAR_SAFE_LONG(VAR, PREV, MEMBER,  
\
ITER_VAR(VAR) != (LIST),
\
@@ -101,7 +102,7 @@ static inline bool ovs_list_is_short(const struct ovs_list 
*);
ITER_VAR(PREV) != (LIST));  
\
   UPDATE_MULTIVAR_SAFE_LONG(VAR, PREV))
  
-#define LIST_FOR_EACH_SAFE(VAR, NEXT, MEMBER, LIST)   \

+#define LIST_FOR_EACH_SAFE_LONG(VAR, NEXT, MEMBER, LIST)  \
  for (INIT_MULTIVAR_SAFE_LONG(VAR, NEXT, MEMBER, (LIST)->next);
\
   CONDITION_MULTIVAR_SAFE_LONG(VAR, NEXT, MEMBER,  
\
ITER_VAR(VAR) != (LIST),
\
@@ -109,6 +110,33 @@ static inline bool ovs_list_is_short(const struct ovs_list 
*);
ITER_VAR(NEXT) != (LIST));  
\
   UPDATE_MULTIVAR_SAFE_LONG(VAR, NEXT))
  
+/* SHORT version of SAFE iterators */

+#define LIST_FOR_EACH_REVERSE_SAFE_SHORT(VAR, MEMBER, LIST)   \
+for (INIT_MULTIVAR_SAFE_SHORT(VAR, MEMBER, (LIST)->prev); \
+ CONDITION_MULTIVAR_SAFE_SHORT(VAR, MEMBER,   \
+   ITER_VAR(VAR) != (LIST),   \
+ ITER_NEXT_VAR(VAR) = ITER_VAR(VAR)->prev);   \
+ UPDATE_MULTIVAR_SAFE_SHORT(VAR))
+
+#define LIST_FOR_EACH_SAFE_SHORT(VAR, MEMBER, LIST)   \
+for (INIT_MULTIVAR_SAFE_SHORT(VAR, MEMBER, (LIST)->next); \
+ CONDITION_MULTIVAR_SAFE_SHORT(VAR, MEMBER,   \
+   ITER_VAR(VAR) != (LIST),   \
+ ITER_NEXT_VAR(VAR) = ITER_VAR(VAR)->next);   \
+ UPDATE_MULTIVAR_SAFE_SHORT(VAR))
+
+/* Select the right SAFE macro depending on the number of arguments .*/
+#define LIST_GET_SAFE_MACRO(_1, _2, _3, _4, NAME, ...) NAME
+#define LIST_FOR_

[ovs-dev] [PATCH v2 1/1] ovs-monitor-ipsec: Migration from ipsec.conf to swanctl.conf

[Emeel Hakim via dev]

As strongswan moved to the modern vici-based interface,this patch
modifies ovs-monitor-ipsec to use strongswan's vici-based
configuration instead of the legacy stroke-based configuration.

Reviewed-by: Raed Salem 
Signed-off-by: Emeel Hakim 
---

v2:
fixed Reviewed-by in the commit message.
addressed comments from Mohammad Heib 

 ipsec/ovs-monitor-ipsec.in | 459 ++---
 1 file changed, 325 insertions(+), 134 deletions(-)

diff --git a/ipsec/ovs-monitor-ipsec.in b/ipsec/ovs-monitor-ipsec.in
index a8b0705d9..582c96132 100755
--- a/ipsec/ovs-monitor-ipsec.in
+++ b/ipsec/ovs-monitor-ipsec.in
@@ -32,52 +32,6 @@ import ovs.vlog
 
 
 FILE_HEADER = "# Generated by ovs-monitor-ipsec...do not modify by hand!\n\n"
-transp_tmpl = {"gre": Template("""\
-conn $ifname-$version
-$auth_section
-leftprotoport=gre
-rightprotoport=gre
-
-"""), "gre64": Template("""\
-conn $ifname-$version
-$auth_section
-leftprotoport=gre
-rightprotoport=gre
-
-"""), "geneve": Template("""\
-conn $ifname-in-$version
-$auth_section
-leftprotoport=udp/6081
-rightprotoport=udp
-
-conn $ifname-out-$version
-$auth_section
-leftprotoport=udp
-rightprotoport=udp/6081
-
-"""), "stt": Template("""\
-conn $ifname-in-$version
-$auth_section
-leftprotoport=tcp/7471
-rightprotoport=tcp
-
-conn $ifname-out-$version
-$auth_section
-leftprotoport=tcp
-rightprotoport=tcp/7471
-
-"""), "vxlan": Template("""\
-conn $ifname-in-$version
-$auth_section
-leftprotoport=udp/4789
-rightprotoport=udp
-
-conn $ifname-out-$version
-$auth_section
-leftprotoport=udp
-rightprotoport=udp/4789
-
-""")}
 vlog = ovs.vlog.Vlog("ovs-monitor-ipsec")
 exiting = False
 monitor = None
@@ -160,72 +114,249 @@ charon {
 }
 """ % (FILE_HEADER)
 
-CONF_HEADER = """%s
-config setup
-uniqueids=yes
+SWANCTL_CONF_HEADER = """%s
+conn-defaults {
+  unique = replace
+  reauth_time = 0
+  version = 2
+  proposals = aes128-sha256-x25519
+}
 
-conn %%default
-keyingtries=%%forever
-type=transport
-keyexchange=ikev2
-auto=route
-ike=aes256gcm16-sha256-modp2048
-esp=aes256gcm16-modp2048
+child-defaults {
+  esp_proposals = aes256gcm16-modp2048-esn
+  mode = transport
+  policies_fwd_out = yes
+  start_action = start
+}
 
 """ % (FILE_HEADER)
 
-CA_SECTION = """ca ca_auth
-cacert=%s
+CA_SECTION = """authorities {
+ca_auth {
+cacert=%s
+}
+}
 
 """
 
-SHUNT_POLICY = """conn prevent_unencrypted_gre
-type=drop
-leftprotoport=gre
-mark={0}
+SHUNT_POLICY = """connections {{
+   shunts {{
+  children {{
+ prevent_unencrypted_gre {{
+local_ts = 0.0.0.0/0 [gre]
+mark_in = {0}
+mark_out = {0}
+mode = drop
+start_action = trap
+ }}
+ prevent_unencrypted_gre_ipv6 {{
+local_ts = ::/0 [gre]
+mark_in = {0}
+mark_out = {0}
+mode = drop
+start_action = trap
+ }}
+ prevent_unencrypted_geneve {{
+local_ts = 0.0.0.0/0 [udp/6081]
+mark_in = {0}
+mark_out = {0}
+mode = drop
+start_action = trap
+ }}
+ prevent_unencrypted_geneve_ipv6 {{
+local_ts = ::/0 [udp/6081]
+mark_in = {0}
+mark_out = {0}
+mode = drop
+start_action = trap
+ }}
+ prevent_unencrypted_stt {{
+local_ts = 0.0.0.0/0 [tcp/7471]
+mark_in = {0}
+mark_out = {0}
+mode = drop
+start_action = trap
+ }}
+ prevent_unencrypted_stt_ipv6 {{
+local_ts = ::/0 [tcp/7471]
+mark_in = {0}
+mark_out = {0}
+mode = drop
+start_action = trap
+ }}
+ prevent_unencrypted_vxlan {{
+local_ts = 0.0.0.0/0 [udp/4789]
+mark_in = {0}
+mark_out = {0}
+mode = drop
+start_action = trap
+ }}
+ prevent_unencrypted_vxlan_ipv6 {{
+local_ts = ::/0 [udp/4789]
+mark_in = {0}
+mark_out = {0}
+mode = drop
+start_action = trap
+ }}
+  }}
+   }}
+}}
+"""
+auth_tmpl = {"psk": Template("""\
+local {
+auth = psk
+id = $local_ip
+ }
+ remote {
+auth = psk
+id = $remote_ip
+ }"""),
+ "pki_remote": Template("""\
+local {
+auth = pubkey
+id = $local_name
+certs = $certificate
+}
+remote {
+   auth = pubkey
+   id = $remote_name
+   certs = $remote_cert
+}"""),
+ "pki_ca": Template("""\
+local {
+auth = pubkey
+id = $local_name
+certs = $certificate
+}
+rem

Re: [ovs-dev] [PATCH 1/1] ovs-monitor-ipsec: Migration from ipsec.conf to swanctl.conf

[Emeel Hakim via dev]

Hi Mohammad,
thanks for your quick reply.
I will address the comments and send a V2

I just want to mention that there is a weird mistake in my submission
the Reviewed-by in the commit message is wrong!
im really confused here since I added Raed Salem as a Reviewed-by
looks like it was an innocent mistake of using a script with some default values
I will fix this in the Version 2 and  sorry again for the inconvenience.




From: Mohammad Heib 
Sent: Monday, February 14, 2022 5:49 PM
To: Emeel Hakim ; d...@openvswitch.org
Cc: Raed Salem ; Roi Dayan 
Subject: Re: [PATCH 1/1] ovs-monitor-ipsec: Migration from ipsec.conf to 
swanctl.conf

External email: Use caution opening links or attachments


Hi Emeel,
Thank you for submitting this patch.

please see few comments below


On 2/14/22 16:21, Emeel Hakim wrote:

As strongswan moved to the modern vici-based interface,this patch

modifies ovs-monitor-ipsec to use strongswan's vici-based

configuration instead of the legacy stroke-based configuration.



Signed-off-by: Emeel Hakim 

Reviewed-by: Nick Desaulniers 
mailto:ndesaulni...@google.com>

---

 ipsec/ovs-monitor-ipsec.in | 454 ++---

 1 file changed, 321 insertions(+), 133 deletions(-)



diff --git a/ipsec/ovs-monitor-ipsec.in b/ipsec/ovs-monitor-ipsec.in

index a8b0705d9..dc070f36e 100755

--- a/ipsec/ovs-monitor-ipsec.in

+++ b/ipsec/ovs-monitor-ipsec.in

@@ -32,52 +32,6 @@ import ovs.vlog





 FILE_HEADER = "# Generated by ovs-monitor-ipsec...do not modify by hand!\n\n"

-transp_tmpl = {"gre": Template("""\

-conn $ifname-$version

-$auth_section

-leftprotoport=gre

-rightprotoport=gre

-

-"""), "gre64": Template("""\

-conn $ifname-$version

-$auth_section

-leftprotoport=gre

-rightprotoport=gre

-

-"""), "geneve": Template("""\

-conn $ifname-in-$version

-$auth_section

-leftprotoport=udp/6081

-rightprotoport=udp

-

-conn $ifname-out-$version

-$auth_section

-leftprotoport=udp

-rightprotoport=udp/6081

-

-"""), "stt": Template("""\

-conn $ifname-in-$version

-$auth_section

-leftprotoport=tcp/7471

-rightprotoport=tcp

-

-conn $ifname-out-$version

-$auth_section

-leftprotoport=tcp

-rightprotoport=tcp/7471

-

-"""), "vxlan": Template("""\

-conn $ifname-in-$version

-$auth_section

-leftprotoport=udp/4789

-rightprotoport=udp

-

-conn $ifname-out-$version

-$auth_section

-leftprotoport=udp

-rightprotoport=udp/4789

-

-""")}

 vlog = ovs.vlog.Vlog("ovs-monitor-ipsec")

 exiting = False

 monitor = None

@@ -160,72 +114,249 @@ charon {

 }

 """ % (FILE_HEADER)



-CONF_HEADER = """%s

-config setup

-uniqueids=yes

+SWANCTL_CONF_HEADER = """%s

+conn-defaults {

+  unique = replace

+  reauth_time = 0

+  version = 2

+  proposals = aes128-sha256-x25519

+}



-conn %%default

-keyingtries=%%forever

-type=transport

-keyexchange=ikev2

-auto=route

-ike=aes256gcm16-sha256-modp2048

-esp=aes256gcm16-modp2048

+child-defaults {

+  esp_proposals = aes256gcm16-modp2048-esn

+  mode = transport

+  policies_fwd_out = yes

+  start_action = start

+}



 """ % (FILE_HEADER)



-CA_SECTION = """ca ca_auth

-cacert=%s

+CA_SECTION = """authorities {

+ca_auth {

+cacert=%s

+}

+}



 """



-SHUNT_POLICY = """conn prevent_unencrypted_gre

-type=drop

-leftprotoport=gre

-mark={0}

+SHUNT_POLICY = """connections {{

+   shunts {{

+  children {{

+ prevent_unencrypted_gre {{

+local_ts = 0.0.0.0/0 [gre]

+mark_in = {0}

+mark_out = {0}

+mode = drop

+start_action = trap

+ }}

+ prevent_unencrypted_gre_ipv6 {{

+local_ts = ::/0 [gre]

+mark_in = {0}

+mark_out = {0}

+mode = drop

+start_action = trap

+ }}

+ prevent_unencrypted_geneve {{

+local_ts = 0.0.0.0/0 [udp/6081]

+mark_in = {0}

+mark_out = {0}

+mode = drop

+start_action = trap

+ }}

+ prevent_unencrypted_geneve_ipv6 {{

+local_ts = ::/0 [udp/6081]

+mark_in = {0}

+mark_out = {0}

+mode = drop

+start_action = trap

+ }}

+ prevent_unencrypted_stt {{

+local_ts = 0.0.0.0/0 [tcp/7471]

+mark_in = {0}

+mark_out = {0}

+mode = drop

+start_action = trap

+ }}

+ prevent_unencrypted_stt_ipv6 {{

+local_ts = ::/0 [tcp/7471]

+mark_in = {0}

+mark_out = {0}

+mode = drop

+start_action = trap

+ }}

+ prevent_unencrypted_vxlan {{

+local_ts = 0.0.0.0/0 [udp/4789]

+mark_in = {0}

+mark_out = {0}

Re: [ovs-dev] [ovs-dev v6] ofproto: add refcount to ofproto to fix ofproto use-after-free

[Peng He]

Adrian Moreno  于2022年2月15日周二 17:19写道：

> Hi Peng,
>
> On 2/13/22 03:14, Peng He wrote:
> >  From hepeng:
> >
> https://patchwork.ozlabs.org/project/openvswitch/patch/20200717015041.82746-1-hepeng.0...@bytedance.com/#2487473
> >
> > also from guohongzhi :
> >
> http://patchwork.ozlabs.org/project/openvswitch/patch/20200306130555.19884-1-guohongz...@huawei.com/
> >
> > also from a discussion about the mixing use of RCU and refcount in the
> mail
> > list with Ilya Maximets, William Tu, Ben Pfaf, and Gaëtan Rivet.
> >
> > A summary, as quoted from Ilya:
> >
> > "
> > RCU for ofproto was introduced for one
> > and only one reason - to avoid freeing ofproto while rules are still
> > alive.  This was done in commit f416c8d61601 ("ofproto: RCU postpone
> > rule destruction.").  The goal was to allow using rules without
> > refcounting them within a single grace period.  And that forced us
> > to postpone destruction of the ofproto for a single grace period.
> > Later commit 39c9459355b6 ("Use classifier versioning.") made it
> > possible for rules to be alive for more than one grace period, so
> > the commit made ofproto wait for 2 grace periods by double postponing.
> > As we can see now, that wasn't enough and we have to wait for more
> > than 2 grace periods in certain cases.
> > "
> >
> > In a short, the ofproto should have a longer life time than rule, if
> > the rule lasts for more than 2 grace periods, the ofproto should live
> > longer to ensure rule->ofproto is valid. It's hard to predict how long
> > a ofproto should live, thus we need to use refcount on ofproto to make
> > things easy. The controversial part is that we have already used RCU
> postpone
> > to delay ofproto destrution, if we have to add refcount, is it simpler to
> > use just refcount without RCU postpone?
> >
> > IMO, I think going back to the pure refcount solution is more
> > complicated than mixing using both.
> >
> > Gaëtan Rive asks some questions on guohongzhi's v2 patch:
> >
> > during ofproto_rule_create, should we use ofproto_ref
> > or ofproto_try_ref? how can we make sure the ofproto is alive?
> >
> > By using RCU, ofproto has three states:
> >
> > state 1: alive, with refcount >= 1
> > state 2: dying, with refcount == 0, however pointer is valid
> > state 3: died, memory freed, pointer might be dangling.
> >
> > Without using RCU, there is no state 2, thus, we have to be very careful
> > every time we see a ofproto pointer. In contrast, with RCU, we can be
> sure
> > that it's alive at least in this grace peroid, so we can just check if
> > it is dying by ofproto_try_ref.
> >
> > This shows that by mixing use of RCU and refcount we can save a lot of
> work
> > worrying if ofproto is dangling.
> >
> > In short, the RCU part makes sure the ofproto is alive when we use it,
> > and the refcount part makes sure it lives longer enough.
> >
> > In this patch, I have merged guohongzhi's patch and mine, and fixes
> > accoring to the previous comments.
> >
> > v4->v5:
> > * fix the commits, remove the ref to wangyunjian's patch and
> > remove the comments describing the previous ofproto destruction code.
> > * fix group alloc leak issues.
> >
> > v5->v6:
> > * fix ofproto unref leak
> > * fix comments
> >
> > Signed-off-by: Peng He 
> > Signed-off-by: guohongzhi 
> > Acked-by: Mike Pattrick 
> > ---
> >   ofproto/ofproto-dpif-xlate-cache.c |  2 +
> >   ofproto/ofproto-dpif-xlate.c   | 14 ---
> >   ofproto/ofproto-dpif.c | 24 ++-
> >   ofproto/ofproto-provider.h |  2 +
> >   ofproto/ofproto.c  | 65 +++---
> >   ofproto/ofproto.h  |  4 ++
> >   6 files changed, 90 insertions(+), 21 deletions(-)
> >
> > diff --git a/ofproto/ofproto-dpif-xlate-cache.c
> b/ofproto/ofproto-dpif-xlate-cache.c
> > index dcc91cb38..9224ee2e6 100644
> > --- a/ofproto/ofproto-dpif-xlate-cache.c
> > +++ b/ofproto/ofproto-dpif-xlate-cache.c
> > @@ -209,6 +209,7 @@ xlate_cache_clear_entry(struct xc_entry *entry)
> >   {
> >   switch (entry->type) {
> >   case XC_TABLE:
> > +ofproto_unref(&(entry->table.ofproto->up));
> >   break;
> >   case XC_RULE:
> >   ofproto_rule_unref(&entry->rule->up);
> > @@ -231,6 +232,7 @@ xlate_cache_clear_entry(struct xc_entry *entry)
> >   free(entry->learn.ofm);
> >   break;
> >   case XC_NORMAL:
> > +ofproto_unref(&(entry->normal.ofproto->up));
> >   break;
> >   case XC_FIN_TIMEOUT:
> >   /* 'u.fin.rule' is always already held as a XC_RULE, which
> > diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
> > index 6fb59e170..129cdf714 100644
> > --- a/ofproto/ofproto-dpif-xlate.c
> > +++ b/ofproto/ofproto-dpif-xlate.c
> > @@ -3024,12 +3024,14 @@ xlate_normal(struct xlate_ctx *ctx)
> >   struct xc_entry *entry;
> >
> >   /* Save just enough info to update mac learning table later. */
> > -entry = xlate_cache_add_entry(c

[ovs-dev] [PATCH] ofproto/ofproto-dpif: Fix dpif_type for userspace tunnels

[Wan Junjie]

When we create two or more tunnels with the same type, only the first
tunnel will be added by dpif since they share the same datapath port.
Set the dpif_type here will clear the ioctl error logs.

Fixes: 4f19a78 ("netdev-vport: Fix userspace tunnel ioctl(SIOCGIFINDEX) info 
logs.")
Signed-off-by: Wan Junjie 
---
 ofproto/ofproto-dpif.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index 8143dd965..938967d19 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -3963,6 +3963,10 @@ port_add(struct ofproto *ofproto_, struct netdev *netdev)
 simap_put(&ofproto->backer->tnl_backers,
   dp_port_name, odp_to_u32(port_no));
 }
+} else {
+struct dpif *dpif = ofproto->backer->dpif;
+const char *dpif_type_str = dpif_normalize_type(dpif_type(dpif));
+netdev_set_dpif_type(netdev, dpif_type_str);
 }
 
 if (netdev_get_tunnel_config(netdev)) {
-- 
2.33.0

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

[ovs-dev] [PATCH v2] tc: Keep header rewrite actions order

[Roi Dayan via dev]

From: Chris Mi 

Currently, tc merges all header rewrite actions into one tc pedit
action. So the header rewrite actions order is lost. Save each header
rewrite action into one tc pedit action to keep the order. And only
append one tc csum action to the last pedit action of a series.

Signed-off-by: Chris Mi 
Reviewed-by: Roi Dayan 
---

v2:
- rebased to fix conflict.

 lib/netdev-offload-tc.c | 22 
 lib/tc.c| 54 +
 lib/tc.h| 25 +++
 3 files changed, 57 insertions(+), 44 deletions(-)

diff --git a/lib/netdev-offload-tc.c b/lib/netdev-offload-tc.c
index 9845e8d3feae..ab2a678c6923 100644
--- a/lib/netdev-offload-tc.c
+++ b/lib/netdev-offload-tc.c
@@ -481,10 +481,10 @@ netdev_tc_flow_dump_destroy(struct netdev_flow_dump *dump)
 
 static void
 parse_flower_rewrite_to_netlink_action(struct ofpbuf *buf,
-   struct tc_flower *flower)
+   struct tc_action *action)
 {
-char *mask = (char *) &flower->rewrite.mask;
-char *data = (char *) &flower->rewrite.key;
+char *mask = (char *) &action->rewrite.mask;
+char *data = (char *) &action->rewrite.key;
 
 for (int type = 0; type < ARRAY_SIZE(set_flower_map); type++) {
 char *put = NULL;
@@ -877,7 +877,7 @@ parse_tc_flower_to_match(struct tc_flower *flower,
 }
 break;
 case TC_ACT_PEDIT: {
-parse_flower_rewrite_to_netlink_action(buf, flower);
+parse_flower_rewrite_to_netlink_action(buf, action);
 }
 break;
 case TC_ACT_ENCAP: {
@@ -1222,8 +1222,8 @@ parse_put_flow_set_masked_action(struct tc_flower *flower,
 uint64_t set_stub[1024 / 8];
 struct ofpbuf set_buf = OFPBUF_STUB_INITIALIZER(set_stub);
 char *set_data, *set_mask;
-char *key = (char *) &flower->rewrite.key;
-char *mask = (char *) &flower->rewrite.mask;
+char *key = (char *) &action->rewrite.key;
+char *mask = (char *) &action->rewrite.mask;
 const struct nlattr *attr;
 int i, j, type;
 size_t size;
@@ -1265,14 +1265,6 @@ parse_put_flow_set_masked_action(struct tc_flower 
*flower,
 }
 }
 
-if (!is_all_zeros(&flower->rewrite, sizeof flower->rewrite)) {
-if (flower->rewrite.rewrite == false) {
-flower->rewrite.rewrite = true;
-action->type = TC_ACT_PEDIT;
-flower->action_count++;
-}
-}
-
 if (hasmask && !is_all_zeros(set_mask, size)) {
 VLOG_DBG_RL(&rl, "unsupported sub attribute of set action type %d",
 type);
@@ -1281,6 +1273,8 @@ parse_put_flow_set_masked_action(struct tc_flower *flower,
 }
 
 ofpbuf_uninit(&set_buf);
+action->type = TC_ACT_PEDIT;
+flower->action_count++;
 return 0;
 }
 
diff --git a/lib/tc.c b/lib/tc.c
index adb2d3182ad4..b637f4c17ed9 100644
--- a/lib/tc.c
+++ b/lib/tc.c
@@ -1006,14 +1006,14 @@ static const struct nl_policy pedit_policy[] = {
 static int
 nl_parse_act_pedit(struct nlattr *options, struct tc_flower *flower)
 {
-struct tc_action *action;
+struct tc_action *action = &flower->actions[flower->action_count++];
 struct nlattr *pe_attrs[ARRAY_SIZE(pedit_policy)];
 const struct tc_pedit *pe;
 const struct tc_pedit_key *keys;
 const struct nlattr *nla, *keys_ex, *ex_type;
 const void *keys_attr;
-char *rewrite_key = (void *) &flower->rewrite.key;
-char *rewrite_mask = (void *) &flower->rewrite.mask;
+char *rewrite_key = (void *) &action->rewrite.key;
+char *rewrite_mask = (void *) &action->rewrite.mask;
 size_t keys_ex_size, left;
 int type, i = 0, err;
 
@@ -1092,7 +1092,6 @@ nl_parse_act_pedit(struct nlattr *options, struct 
tc_flower *flower)
 i++;
 }
 
-action = &flower->actions[flower->action_count++];
 action->type = TC_ACT_PEDIT;
 
 return 0;
@@ -2399,14 +2398,14 @@ nl_msg_put_act_flags(struct ofpbuf *request) {
  * first_word_mask/last_word_mask - the mask to use for the first/last read
  * (as we read entire words). */
 static void
-calc_offsets(struct tc_flower *flower, struct flower_key_to_pedit *m,
+calc_offsets(struct tc_action *action, struct flower_key_to_pedit *m,
  int *cur_offset, int *cnt, ovs_be32 *last_word_mask,
  ovs_be32 *first_word_mask, ovs_be32 **mask, ovs_be32 **data)
 {
 int start_offset, max_offset, total_size;
 int diff, right_zero_bits, left_zero_bits;
-char *rewrite_key = (void *) &flower->rewrite.key;
-char *rewrite_mask = (void *) &flower->rewrite.mask;
+char *rewrite_key = (void *) &action->rewrite.key;
+char *rewrite_mask = (void *) &action->rewrite.mask;
 
 max_offset = m->offset + m->size;
 start_offset = ROUND_DOWN(m->offset, 4);
@@ -2473,7 +2472,8 @@ csum_update_flag(struct tc_flower *flower,
 
 static int
 nl_msg_put_flower_rewrite_pedits(

Re: [ovs-dev] [PATCH v20 8/8] system-offloads-traffic.at: Add sFlow offload test cases

[0-day Robot]

Bleep bloop.  Greetings Chris Mi, I am a robot and I have tried out your patch.
Thanks for your contribution.

I encountered some error that I wasn't expecting.  See the details below.


Patch skipped due to previous failure.

Please check this out.  If you feel there has been an error, please email 
acon...@redhat.com

Thanks,
0-day Robot
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v20 7/8] netdev-offload-tc: Add offload support for sFlow

[0-day Robot]

Bleep bloop.  Greetings Chris Mi, I am a robot and I have tried out your patch.
Thanks for your contribution.

I encountered some error that I wasn't expecting.  See the details below.


git-am:
error: sha1 information is lacking or useless (lib/netdev-offload-tc.c).
error: could not build fake ancestor
hint: Use 'git am --show-current-patch' to see the failed patch
Patch failed at 0001 netdev-offload-tc: Add offload support for sFlow
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".


Patch skipped due to previous failure.

Please check this out.  If you feel there has been an error, please email 
acon...@redhat.com

Thanks,
0-day Robot
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v20 6/8] ofproto: Introduce API to process sFlow offload packet

[0-day Robot]

Bleep bloop.  Greetings Chris Mi, I am a robot and I have tried out your patch.
Thanks for your contribution.

I encountered some error that I wasn't expecting.  See the details below.


checkpatch:
ERROR: Remove Gerrit Change-Id's before submitting upstream.
8: Change-Id: I40e0add3bff69751d8f957e5eeff58c665dd5d45

Lines checked: 118, Warnings: 0, Errors: 1


Please check this out.  If you feel there has been an error, please email 
acon...@redhat.com

Thanks,
0-day Robot
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v20 5/8] dpif-offload-netlink: Implement dpif-offload-provider API

[0-day Robot]

Bleep bloop.  Greetings Chris Mi, I am a robot and I have tried out your patch.
Thanks for your contribution.

I encountered some error that I wasn't expecting.  See the details below.


checkpatch:
ERROR: Remove Gerrit Change-Id's before submitting upstream.
9: Change-Id: I950c3cc3c7092c3d87602da8928ad4aa2df4f38a

Lines checked: 673, Warnings: 0, Errors: 1


Please check this out.  If you feel there has been an error, please email 
acon...@redhat.com

Thanks,
0-day Robot
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v20 4/8] netdev-offload-tc: Introduce group ID management API

[0-day Robot]

Bleep bloop.  Greetings Chris Mi, I am a robot and I have tried out your patch.
Thanks for your contribution.

I encountered some error that I wasn't expecting.  See the details below.


checkpatch:
ERROR: Remove Gerrit Change-Id's before submitting upstream.
12: Change-Id: Ibb000960434ee428b66719dd90d14facba3cf5f6

Lines checked: 386, Warnings: 0, Errors: 1


Please check this out.  If you feel there has been an error, please email 
acon...@redhat.com

Thanks,
0-day Robot
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v20 3/8] dpif-offload-provider: Introduce dpif-offload-provider layer

[0-day Robot]

Bleep bloop.  Greetings Chris Mi, I am a robot and I have tried out your patch.
Thanks for your contribution.

I encountered some error that I wasn't expecting.  See the details below.


checkpatch:
ERROR: Remove Gerrit Change-Id's before submitting upstream.
13: Change-Id: I880b8b2aebbf6886fd8064409108ee59974ff195

Lines checked: 172, Warnings: 0, Errors: 1


Please check this out.  If you feel there has been an error, please email 
acon...@redhat.com

Thanks,
0-day Robot
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v20 2/8] ovs-kmod-ctl: Load kernel module psample

[0-day Robot]

Bleep bloop.  Greetings Chris Mi, I am a robot and I have tried out your patch.
Thanks for your contribution.

I encountered some error that I wasn't expecting.  See the details below.


checkpatch:
ERROR: Remove Gerrit Change-Id's before submitting upstream.
9: Change-Id: I494b8e5e9ebda64376c6f0bcdb6138d49de5c134

Lines checked: 53, Warnings: 0, Errors: 1


Please check this out.  If you feel there has been an error, please email 
acon...@redhat.com

Thanks,
0-day Robot
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v20 1/8] compat: Add psample and tc sample action defines for older kernels

[0-day Robot]

Bleep bloop.  Greetings Chris Mi, I am a robot and I have tried out your patch.
Thanks for your contribution.

I encountered some error that I wasn't expecting.  See the details below.


checkpatch:
ERROR: Remove Gerrit Change-Id's before submitting upstream.
8: Change-Id: I67822667494230bdc1d1f4b2217d019a65486ad3

Lines checked: 135, Warnings: 0, Errors: 1


Please check this out.  If you feel there has been an error, please email 
acon...@redhat.com

Thanks,
0-day Robot
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [ovs-dev v6] ofproto: add refcount to ofproto to fix ofproto use-after-free

[Adrian Moreno]

Hi Peng,

On 2/13/22 03:14, Peng He wrote:

 From hepeng:
https://patchwork.ozlabs.org/project/openvswitch/patch/20200717015041.82746-1-hepeng.0...@bytedance.com/#2487473

also from guohongzhi :
http://patchwork.ozlabs.org/project/openvswitch/patch/20200306130555.19884-1-guohongz...@huawei.com/

also from a discussion about the mixing use of RCU and refcount in the mail
list with Ilya Maximets, William Tu, Ben Pfaf, and Gaëtan Rivet.

A summary, as quoted from Ilya:

"
RCU for ofproto was introduced for one
and only one reason - to avoid freeing ofproto while rules are still
alive.  This was done in commit f416c8d61601 ("ofproto: RCU postpone
rule destruction.").  The goal was to allow using rules without
refcounting them within a single grace period.  And that forced us
to postpone destruction of the ofproto for a single grace period.
Later commit 39c9459355b6 ("Use classifier versioning.") made it
possible for rules to be alive for more than one grace period, so
the commit made ofproto wait for 2 grace periods by double postponing.
As we can see now, that wasn't enough and we have to wait for more
than 2 grace periods in certain cases.
"

In a short, the ofproto should have a longer life time than rule, if
the rule lasts for more than 2 grace periods, the ofproto should live
longer to ensure rule->ofproto is valid. It's hard to predict how long
a ofproto should live, thus we need to use refcount on ofproto to make
things easy. The controversial part is that we have already used RCU postpone
to delay ofproto destrution, if we have to add refcount, is it simpler to
use just refcount without RCU postpone?

IMO, I think going back to the pure refcount solution is more
complicated than mixing using both.

Gaëtan Rive asks some questions on guohongzhi's v2 patch:

during ofproto_rule_create, should we use ofproto_ref
or ofproto_try_ref? how can we make sure the ofproto is alive?

By using RCU, ofproto has three states:

state 1: alive, with refcount >= 1
state 2: dying, with refcount == 0, however pointer is valid
state 3: died, memory freed, pointer might be dangling.

Without using RCU, there is no state 2, thus, we have to be very careful
every time we see a ofproto pointer. In contrast, with RCU, we can be sure
that it's alive at least in this grace peroid, so we can just check if
it is dying by ofproto_try_ref.

This shows that by mixing use of RCU and refcount we can save a lot of work
worrying if ofproto is dangling.

In short, the RCU part makes sure the ofproto is alive when we use it,
and the refcount part makes sure it lives longer enough.

In this patch, I have merged guohongzhi's patch and mine, and fixes
accoring to the previous comments.

v4->v5:
* fix the commits, remove the ref to wangyunjian's patch and
remove the comments describing the previous ofproto destruction code.
* fix group alloc leak issues.

v5->v6:
* fix ofproto unref leak
* fix comments

Signed-off-by: Peng He 
Signed-off-by: guohongzhi 
Acked-by: Mike Pattrick 
---
  ofproto/ofproto-dpif-xlate-cache.c |  2 +
  ofproto/ofproto-dpif-xlate.c   | 14 ---
  ofproto/ofproto-dpif.c | 24 ++-
  ofproto/ofproto-provider.h |  2 +
  ofproto/ofproto.c  | 65 +++---
  ofproto/ofproto.h  |  4 ++
  6 files changed, 90 insertions(+), 21 deletions(-)

diff --git a/ofproto/ofproto-dpif-xlate-cache.c 
b/ofproto/ofproto-dpif-xlate-cache.c
index dcc91cb38..9224ee2e6 100644
--- a/ofproto/ofproto-dpif-xlate-cache.c
+++ b/ofproto/ofproto-dpif-xlate-cache.c
@@ -209,6 +209,7 @@ xlate_cache_clear_entry(struct xc_entry *entry)
  {
  switch (entry->type) {
  case XC_TABLE:
+ofproto_unref(&(entry->table.ofproto->up));
  break;
  case XC_RULE:
  ofproto_rule_unref(&entry->rule->up);
@@ -231,6 +232,7 @@ xlate_cache_clear_entry(struct xc_entry *entry)
  free(entry->learn.ofm);
  break;
  case XC_NORMAL:
+ofproto_unref(&(entry->normal.ofproto->up));
  break;
  case XC_FIN_TIMEOUT:
  /* 'u.fin.rule' is always already held as a XC_RULE, which
diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index 6fb59e170..129cdf714 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -3024,12 +3024,14 @@ xlate_normal(struct xlate_ctx *ctx)
  struct xc_entry *entry;
  
  /* Save just enough info to update mac learning table later. */

-entry = xlate_cache_add_entry(ctx->xin->xcache, XC_NORMAL);
-entry->normal.ofproto = ctx->xbridge->ofproto;
-entry->normal.in_port = flow->in_port.ofp_port;
-entry->normal.dl_src = flow->dl_src;
-entry->normal.vlan = vlan;
-entry->normal.is_gratuitous_arp = is_grat_arp;
+if (ofproto_try_ref(&ctx->xbridge->ofproto->up)) {
+entry = xlate_cache_add_entry(ctx->xin->xcache, XC_NORMAL);
+entry->normal.ofproto = ctx->xbridge->ofproto;
+   

[ovs-dev] [PATCH v20 8/8] system-offloads-traffic.at: Add sFlow offload test cases

[Chris Mi via dev]

Add two sFlow offload test caes:

  3: sflow offloads with sampling=1 - ping between two ports - offloads enabled 
ok
  4: sflow offloads with sampling=2 - ping between two ports - offloads enabled 
ok

Issue: 2181036
Change-Id: Iab54cbdd18fea3cc99d707cdbde103ecd9a17341
Signed-off-by: Chris Mi 
Acked-by: Eelco Chaudron 
---
 tests/system-offloads-traffic.at | 101 +++
 1 file changed, 101 insertions(+)

diff --git a/tests/system-offloads-traffic.at b/tests/system-offloads-traffic.at
index 80bc1dd5c..19dd2137f 100644
--- a/tests/system-offloads-traffic.at
+++ b/tests/system-offloads-traffic.at
@@ -71,6 +71,107 @@ AT_CHECK([ovs-appctl upcall/show | grep -E "offloaded flows 
: [[1-9]]"], [0], [i
 OVS_TRAFFIC_VSWITCHD_STOP
 AT_CLEANUP
 
+AT_SETUP([sflow offloads with sampling=1 - ping between two ports - offloads 
enabled])
+OVS_TRAFFIC_VSWITCHD_START()
+
+on_exit 'kill `cat test-sflow.pid`'
+AT_CHECK([ovstest test-sflow --log-file --detach --no-chdir --pidfile 
0:127.0.0.1 > sflow.log], [0], [], [ignore])
+AT_CAPTURE_FILE([sflow.log])
+PARSE_LISTENING_PORT([test-sflow.log], [SFLOW_PORT])
+
+AT_CHECK([ovs-vsctl set Open_vSwitch . other_config:hw-offload=true])
+
+AT_CHECK([ovs-appctl -t ovsdb-server exit])
+AT_CHECK([ovs-appctl -t ovs-vswitchd exit])
+AT_CHECK([rm -f ovsdb-server.pid])
+AT_CHECK([rm -f ovs-vswitchd.pid])
+AT_CHECK([ovsdb-server --detach --no-chdir --pidfile --log-file 
--remote=punix:$OVS_RUNDIR/db.sock], [0], [], [stderr])
+AT_CHECK([ovs-vswitchd --detach --no-chdir --pidfile --log-file -vvconn 
-vofproto_dpif -vunixctl], [0], [], [stderr])
+on_exit "kill `cat ovsdb-server.pid`"
+on_exit "kill_ovs_vswitchd `cat ovs-vswitchd.pid`"
+
+AT_CHECK([ovs-ofctl add-flow br0 "actions=normal"])
+
+ADD_NAMESPACES(at_ns0, at_ns1)
+
+ADD_VETH(p0, at_ns0, br0, "10.1.1.1/24")
+ADD_VETH(p1, at_ns1, br0, "10.1.1.2/24")
+AT_CHECK([ovs-appctl dpctl/dump-flows], [0], [ignore])
+
+NS_CHECK_EXEC([at_ns0], [ping -c 10 -w 12 10.1.1.2 | FORMAT_PING], [0], [dnl
+10 packets transmitted, 10 received, 0% packet loss, time 0ms
+])
+AT_CHECK([ovs-vsctl -- --id=@sflow create sflow agent=lo 
target=\"127.0.0.1:$SFLOW_PORT\" header=128 sampling=1 polling=100 -- set 
bridge br0 sflow=@sflow], [0], [ignore])
+NS_CHECK_EXEC([at_ns0], [ping -c 10 -w 12 10.1.1.2 | FORMAT_PING], [0], [dnl
+10 packets transmitted, 10 received, 0% packet loss, time 0ms
+])
+
+m4_define([DUMP_SFLOW], [sed -e 
"s/used:[[0-9]].[[0-9]]*s/used:0.001s/;s/eth(src=[[a-z0-9:]]*,dst=[[a-z0-9:]]*)/eth(macs)/;s/pid=[[0-9]]*/pid=1/;s/output=$P1_IFINDEX/output=1/"])
+P1_IFINDEX=$(cat /sys/class/net/ovs-p1/ifindex)
+AT_CHECK([ovs-appctl dpctl/dump-flows type=tc,offloaded | grep "in_port(2)" | 
grep "eth_type(0x0800)" | grep "actions:userspace" | grep "sFlow" | 
DUMP_SFLOW], [0], [dnl
+recirc_id(0),in_port(2),eth(macs),eth_type(0x0800),ipv4(frag=no), packets:10, 
bytes:840, used:0.001s, 
actions:userspace(pid=1,sFlow(vid=0,pcp=0,output=1),actions),3
+])
+
+m4_define([DUMP_SFLOW], [sed -e 
"s/used:[[0-9]].[[0-9]]*s/used:0.001s/;s/eth(src=[[a-z0-9:]]*,dst=[[a-z0-9:]]*)/eth(macs)/;s/pid=[[0-9]]*/pid=1/;s/output=$P0_IFINDEX/output=1/"])
+P0_IFINDEX=$(cat /sys/class/net/ovs-p0/ifindex)
+AT_CHECK([ovs-appctl dpctl/dump-flows type=tc,offloaded | grep "in_port(3)" | 
grep "eth_type(0x0800)" | grep "actions:userspace" | grep "sFlow" | 
DUMP_SFLOW], [0], [dnl
+recirc_id(0),in_port(3),eth(macs),eth_type(0x0800),ipv4(frag=no), packets:10, 
bytes:840, used:0.001s, 
actions:userspace(pid=1,sFlow(vid=0,pcp=0,output=1),actions),2
+])
+
+OVS_TRAFFIC_VSWITCHD_STOP
+OVS_APP_EXIT_AND_WAIT([test-sflow])
+count=`cat sflow.log | wc -l`
+AT_CHECK( $count -le 22 && $count -ge 18 )
+AT_CLEANUP
+
+AT_SETUP([sflow offloads with sampling=2 - ping between two ports - offloads 
enabled])
+OVS_TRAFFIC_VSWITCHD_START()
+
+on_exit 'kill `cat test-sflow.pid`'
+AT_CHECK([ovstest test-sflow --log-file --detach --no-chdir --pidfile 
0:127.0.0.1 > sflow.log], [0], [], [ignore])
+AT_CAPTURE_FILE([sflow.log])
+PARSE_LISTENING_PORT([test-sflow.log], [SFLOW_PORT])
+
+AT_CHECK([ovs-vsctl set Open_vSwitch . other_config:hw-offload=true])
+
+AT_CHECK([ovs-appctl -t ovsdb-server exit])
+AT_CHECK([ovs-appctl -t ovs-vswitchd exit])
+AT_CHECK([rm -f ovsdb-server.pid])
+AT_CHECK([rm -f ovs-vswitchd.pid])
+AT_CHECK([ovsdb-server --detach --no-chdir --pidfile --log-file 
--remote=punix:$OVS_RUNDIR/db.sock], [0], [], [stderr])
+AT_CHECK([ovs-vswitchd --detach --no-chdir --pidfile --log-file -vvconn 
-vofproto_dpif -vunixctl], [0], [], [stderr])
+on_exit "kill `cat ovsdb-server.pid`"
+on_exit "kill_ovs_vswitchd `cat ovs-vswitchd.pid`"
+
+AT_CHECK([ovs-ofctl add-flow br0 "actions=normal"])
+
+ADD_NAMESPACES(at_ns0, at_ns1)
+
+ADD_VETH(p0, at_ns0, br0, "10.1.1.1/24")
+ADD_VETH(p1, at_ns1, br0, "10.1.1.2/24")
+AT_CHECK([ovs-appctl dpctl/dump-flows], [0], [ignore])
+
+AT_CHECK([ovs-vsctl -- --id=@sflow create sflow agent=lo 
target=\"127.0.0.1:$SFLOW_PORT\" header=128 sampling=2 polling=100

[ovs-dev] [PATCH v20 7/8] netdev-offload-tc: Add offload support for sFlow

[Chris Mi via dev]

Create a unique group ID to map the sFlow info when offloading sFlow
action to TC. When showing the offloaded datapath flows, translate the
group ID from TC sample action to sFlow info using the mapping.

Issue: 2181036
Change-Id: Ic73cb614a51fee1cb3f6e606c6fccaf3de70c437
Signed-off-by: Chris Mi 
Reviewed-by: Eli Britstein 
Acked-by: Eelco Chaudron 
---
 NEWS|   1 +
 lib/dpif-offload-netlink.c  |   6 +
 lib/dpif-offload-provider.h |   2 +
 lib/netdev-offload-tc.c | 228 +---
 lib/tc.c|  61 +-
 lib/tc.h|  15 ++-
 6 files changed, 291 insertions(+), 22 deletions(-)

diff --git a/NEWS b/NEWS
index e1c48f3a1..52ea7c083 100644
--- a/NEWS
+++ b/NEWS
@@ -51,6 +51,7 @@ v2.17.0 - xx xxx 
- Ingress policing on Linux now uses 'matchall' classifier instead of
  'basic', if available.
- Add User Statically-Defined Tracing (USDT) probe framework support.
+   - Add sFlow offload support for kernel (netlink) datapath.
 
 
 v2.16.0 - 16 Aug 2021
diff --git a/lib/dpif-offload-netlink.c b/lib/dpif-offload-netlink.c
index 02aea7e2d..b94818ee1 100644
--- a/lib/dpif-offload-netlink.c
+++ b/lib/dpif-offload-netlink.c
@@ -219,3 +219,9 @@ const struct dpif_offload_class dpif_offload_netlink_class 
= {
 .sflow_recv_wait = dpif_offload_netlink_sflow_recv_wait,
 .sflow_recv = dpif_offload_netlink_sflow_recv,
 };
+
+bool
+dpif_offload_netlink_psample_supported(void)
+{
+return psample_sock != NULL;
+}
diff --git a/lib/dpif-offload-provider.h b/lib/dpif-offload-provider.h
index ac13601b5..1b9faa053 100644
--- a/lib/dpif-offload-provider.h
+++ b/lib/dpif-offload-provider.h
@@ -89,4 +89,6 @@ void dpif_offload_sflow_recv_wait(const struct dpif *dpif);
 int dpif_offload_sflow_recv(const struct dpif *dpif,
 struct dpif_offload_sflow *sflow);
 
+bool dpif_offload_netlink_psample_supported(void);
+
 #endif /* DPIF_OFFLOAD_PROVIDER_H */
diff --git a/lib/netdev-offload-tc.c b/lib/netdev-offload-tc.c
index c5362f899..affaf30d1 100644
--- a/lib/netdev-offload-tc.c
+++ b/lib/netdev-offload-tc.c
@@ -20,6 +20,7 @@
 #include 
 
 #include "dpif.h"
+#include "dpif-offload-provider.h"
 #include "hash.h"
 #include "openvswitch/hmap.h"
 #include "openvswitch/match.h"
@@ -1052,6 +1053,19 @@ parse_tc_flower_to_match(struct tc_flower *flower,
 action = flower->actions;
 for (i = 0; i < flower->action_count; i++, action++) {
 switch (action->type) {
+case TC_ACT_SAMPLE: {
+const struct sgid_node *node;
+
+node = sgid_find(action->sample.group_id);
+if (!node) {
+VLOG_WARN("Can't find sample group ID data for ID: %u",
+  action->sample.group_id);
+return ENOENT;
+}
+nl_msg_put(buf, node->sflow.action,
+   node->sflow.action->nla_len);
+}
+break;
 case TC_ACT_VLAN_POP: {
 nl_msg_put_flag(buf, OVS_ACTION_ATTR_POP_VLAN);
 }
@@ -1784,6 +1798,151 @@ parse_match_ct_state_to_flower(struct tc_flower 
*flower, struct match *match)
 }
 }
 
+static int
+parse_userspace_attributes(const struct nlattr *actions,
+   struct dpif_sflow_attr *sflow_attr)
+{
+static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 20);
+const struct nlattr *nla;
+unsigned int left;
+
+NL_NESTED_FOR_EACH_UNSAFE (nla, left, actions) {
+if (nl_attr_type(nla) == OVS_USERSPACE_ATTR_USERDATA) {
+struct user_action_cookie *cookie;
+
+cookie = CONST_CAST(struct user_action_cookie *, nl_attr_get(nla));
+if (cookie->type == USER_ACTION_COOKIE_SFLOW) {
+sflow_attr->userdata = nla;
+return 0;
+}
+}
+}
+
+VLOG_DBG_RL(&rl, "Can't offload userspace action other than sFlow");
+return EOPNOTSUPP;
+}
+
+static int
+parse_sample_actions_attribute(const struct nlattr *actions,
+   struct dpif_sflow_attr *sflow_attr)
+{
+static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 20);
+const struct nlattr *nla;
+unsigned int left;
+int err = EINVAL;
+
+NL_NESTED_FOR_EACH_UNSAFE (nla, left, actions) {
+if (nl_attr_type(nla) == OVS_ACTION_ATTR_USERSPACE) {
+err = parse_userspace_attributes(nla, sflow_attr);
+} else {
+/* We can't offload other nested actions */
+VLOG_DBG_RL(&rl, "Can only offload OVS_ACTION_ATTR_USERSPACE"
+ " attribute");
+return EINVAL;
+}
+}
+
+if (err) {
+VLOG_ERR_RL(&error_rl, "No OVS_ACTION_ATTR_USERSPACE attribute");
+}
+return err;
+}
+
+static int
+parse_sample_action(struct tc_flower *flower, struct tc_action *tc_action,
+  

[ovs-dev] [PATCH v20 2/8] ovs-kmod-ctl: Load kernel module psample

[Chris Mi via dev]

Load kernel module psample to receive sampled packets from TC.
Before removing kernel module psample, remove act_sample first.

Issue: 2181036
Change-Id: I494b8e5e9ebda64376c6f0bcdb6138d49de5c134
Signed-off-by: Chris Mi 
Reviewed-by: Eli Britstein 
Acked-by: Eelco Chaudron 
---
 utilities/ovs-kmod-ctl.in | 14 ++
 1 file changed, 14 insertions(+)

diff --git a/utilities/ovs-kmod-ctl.in b/utilities/ovs-kmod-ctl.in
index 19f100964..6fa945a83 100644
--- a/utilities/ovs-kmod-ctl.in
+++ b/utilities/ovs-kmod-ctl.in
@@ -28,6 +28,14 @@ for dir in "$sbindir" "$bindir" /sbin /bin /usr/sbin 
/usr/bin; do
 done
 
 insert_mods () {
+# Try loading psample kernel module.
+modinfo psample > /dev/null 2>&1
+if test $? = 0; then
+action "Inserting psample module" modprobe psample
+else
+log_warning_msg "No psample module, can't offload sFlow action"
+fi
+
 # Try loading openvswitch kernel module.
 action "Inserting openvswitch module" modprobe openvswitch
 }
@@ -95,6 +103,12 @@ remove_kmods() {
 if test -e /sys/module/vxlan; then
 action "Forcing removal of vxlan module" rmmod vxlan
 fi
+if test -e /sys/module/act_sample; then
+action "Forcing removal of act_sample module" rmmod act_sample
+fi
+if test -e /sys/module/psample; then
+action "Forcing removal of psample module" rmmod psample
+fi
 }
 
 usage () {
-- 
2.30.2

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

[ovs-dev] [PATCH v20 4/8] netdev-offload-tc: Introduce group ID management API

[Chris Mi via dev]

When offloading sample action to TC, userspace creates a unique ID
to map sFlow action and tunnel info and passes this ID to kernel instead
of the sFlow info. psample will send this ID and sampled packet to
userspace. Using the ID, userspace can recover the sFlow info and send
sampled packet to the right sFlow monitoring host.

Issue: 2181036
Change-Id: Ibb000960434ee428b66719dd90d14facba3cf5f6
Signed-off-by: Chris Mi 
Reviewed-by: Eli Britstein 
Acked-by: Eelco Chaudron 
---
 lib/dpif-offload-provider.h |  17 +++
 lib/netdev-offload-tc.c | 246 +++-
 lib/netdev-offload.h|   2 +
 lib/tc.h|   1 +
 4 files changed, 260 insertions(+), 6 deletions(-)

diff --git a/lib/dpif-offload-provider.h b/lib/dpif-offload-provider.h
index 3b94740df..af49eedb9 100644
--- a/lib/dpif-offload-provider.h
+++ b/lib/dpif-offload-provider.h
@@ -17,9 +17,26 @@
 #ifndef DPIF_OFFLOAD_PROVIDER_H
 #define DPIF_OFFLOAD_PROVIDER_H
 
+#include "netlink-protocol.h"
+#include "openvswitch/packets.h"
+#include "openvswitch/types.h"
+
 struct dpif;
 struct dpif_offload_sflow;
 
+/* When offloading sample action, userspace creates a unique ID to map
+ * sFlow action and tunnel info and passes this ID to datapath instead
+ * of the sFlow info. Datapath will send this ID and sampled packet to
+ * userspace. Using the ID, userspace can recover the sFlow info and send
+ * sampled packet to the right sFlow monitoring host.
+ */
+struct dpif_sflow_attr {
+const struct nlattr *action;/* SFlow action. */
+const struct nlattr *userdata;  /* Struct user_action_cookie. */
+struct flow_tnl *tunnel;/* Tunnel info. */
+ovs_u128 ufid;  /* Flow ufid. */
+};
+
 /* Datapath interface offload structure, to be defined by each implementation
  * of a datapath interface.
  */
diff --git a/lib/netdev-offload-tc.c b/lib/netdev-offload-tc.c
index ab2a678c6..c5362f899 100644
--- a/lib/netdev-offload-tc.c
+++ b/lib/netdev-offload-tc.c
@@ -40,6 +40,7 @@
 #include "unaligned.h"
 #include "util.h"
 #include "dpif-provider.h"
+#include "cmap.h"
 
 VLOG_DEFINE_THIS_MODULE(netdev_offload_tc);
 
@@ -62,6 +63,233 @@ struct chain_node {
 uint32_t chain;
 };
 
+/* This maps a psample group ID to struct dpif_sflow_attr for sFlow */
+struct sgid_node {
+struct ovs_list exp_node OVS_GUARDED;
+struct cmap_node metadata_node;
+struct cmap_node id_node;
+struct ovs_refcount refcount;
+uint32_t hash;
+uint32_t id;
+const struct dpif_sflow_attr sflow;
+};
+
+static struct ovs_rwlock sgid_rwlock = OVS_RWLOCK_INITIALIZER;
+static struct cmap sgid_map = CMAP_INITIALIZER;
+static struct cmap sgid_metadata_map = CMAP_INITIALIZER;
+static struct ovs_list sgid_expiring OVS_GUARDED_BY(sgid_rwlock)
+= OVS_LIST_INITIALIZER(&sgid_expiring);
+static uint32_t next_sample_group_id OVS_GUARDED_BY(sgid_rwlock) = 1;
+
+static void
+sgid_node_free(struct sgid_node *node)
+{
+free(node->sflow.tunnel);
+free(CONST_CAST(void *, node->sflow.action));
+free(CONST_CAST(void *, node->sflow.userdata));
+free(node);
+}
+
+/* Lockless RCU protected lookup.  If node is needed accross RCU quiescent
+ * state, caller should copy the contents. */
+static const struct sgid_node *
+sgid_find(uint32_t id)
+{
+const struct cmap_node *node = cmap_find(&sgid_map, id);
+
+return node ? CONTAINER_OF(node, const struct sgid_node, id_node) : NULL;
+}
+
+const struct dpif_sflow_attr *
+dpif_offload_sflow_attr_find(uint32_t id)
+{
+const struct sgid_node *node;
+
+node = sgid_find(id);
+if (!node) {
+return NULL;
+}
+
+return &node->sflow;
+}
+
+static uint32_t
+dpif_sflow_attr_hash(const struct dpif_sflow_attr *sflow)
+{
+return hash_bytes(&sflow->ufid, sizeof sflow->ufid, 0);
+}
+
+static bool
+dpif_sflow_attr_equal(const struct dpif_sflow_attr *a,
+  const struct dpif_sflow_attr *b)
+{
+if (!ovs_u128_equals(a->ufid, b->ufid)) {
+return false;
+}
+if (!a->action || !b->action) {
+return false;
+}
+if (a->action->nla_len != b->action->nla_len
+|| memcmp(a->action, b->action, a->action->nla_len)) {
+return false;
+}
+if (!a->tunnel && !b->tunnel) {
+return true;
+}
+if (!a->tunnel || !b->tunnel) {
+return false;
+}
+
+return !memcmp(a->tunnel, b->tunnel, sizeof *a->tunnel);
+}
+
+/* Lockless RCU protected lookup.  If node is needed accross RCU quiescent
+ * state, caller should take a reference. */
+static struct sgid_node *
+sgid_find_equal(const struct dpif_sflow_attr *target, uint32_t hash)
+{
+struct sgid_node *node;
+
+CMAP_FOR_EACH_WITH_HASH (node, metadata_node, hash, &sgid_metadata_map) {
+if (dpif_sflow_attr_equal(&node->sflow, target)) {
+return node;
+}
+}
+return NULL;
+}
+
+static struct sgid_node *
+sgid_ref_equal(const struct dpif_sflow_attr *target, uint32_t hash)
+

[ovs-dev] [PATCH v20 3/8] dpif-offload-provider: Introduce dpif-offload-provider layer

[Chris Mi via dev]

Some offload actions require functionality that is not netdev
based, but dpif. For example, sFlow action requires to create
a psample netlink socket to receive the sampled packets from
TC or kernel driver.

Create dpif-offload-provider layer to support such actions.

Issue:  2181036
Change-Id: I880b8b2aebbf6886fd8064409108ee59974ff195
Signed-off-by: Chris Mi 
Reviewed-by: Eli Britstein 
---
 lib/automake.mk |  2 ++
 lib/dpif-offload-provider.h | 52 +
 lib/dpif-offload.c  | 43 ++
 lib/dpif-provider.h |  4 ++-
 4 files changed, 100 insertions(+), 1 deletion(-)
 create mode 100644 lib/dpif-offload-provider.h
 create mode 100644 lib/dpif-offload.c

diff --git a/lib/automake.mk b/lib/automake.mk
index a23cdc4ad..781fba47a 100644
--- a/lib/automake.mk
+++ b/lib/automake.mk
@@ -129,6 +129,8 @@ lib_libopenvswitch_la_SOURCES = \
lib/dpif-netdev-private.h \
lib/dpif-netdev-perf.c \
lib/dpif-netdev-perf.h \
+   lib/dpif-offload.c \
+   lib/dpif-offload-provider.h \
lib/dpif-provider.h \
lib/dpif.c \
lib/dpif.h \
diff --git a/lib/dpif-offload-provider.h b/lib/dpif-offload-provider.h
new file mode 100644
index 0..3b94740df
--- /dev/null
+++ b/lib/dpif-offload-provider.h
@@ -0,0 +1,52 @@
+/*
+ * Copyright (c) 2021, NVIDIA CORPORATION & AFFILIATES. All rights reserved.
+ *
+ * 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.
+ */
+
+#ifndef DPIF_OFFLOAD_PROVIDER_H
+#define DPIF_OFFLOAD_PROVIDER_H
+
+struct dpif;
+struct dpif_offload_sflow;
+
+/* Datapath interface offload structure, to be defined by each implementation
+ * of a datapath interface.
+ */
+struct dpif_offload_class {
+/* Type of dpif offload in this class, e.g. "system", "netdev", etc.
+ *
+ * One of the providers should supply a "system" type, since this is
+ * the type assumed if no type is specified when opening a dpif. */
+const char *type;
+
+/* Called when the dpif offload provider is registered. */
+int (*init)(void);
+
+/* Free all dpif offload resources. */
+void (*destroy)(void);
+
+/* Arranges for the poll loop for an upcall handler to wake up when psample
+ * has a message queued to be received. */
+void (*sflow_recv_wait)(void);
+
+/* Polls for an upcall from psample for an upcall handler.
+ * Return 0 for success. */
+int (*sflow_recv)(struct dpif_offload_sflow *sflow);
+};
+
+void dpif_offload_sflow_recv_wait(const struct dpif *dpif);
+int dpif_offload_sflow_recv(const struct dpif *dpif,
+struct dpif_offload_sflow *sflow);
+
+#endif /* DPIF_OFFLOAD_PROVIDER_H */
diff --git a/lib/dpif-offload.c b/lib/dpif-offload.c
new file mode 100644
index 0..f2bf3e634
--- /dev/null
+++ b/lib/dpif-offload.c
@@ -0,0 +1,43 @@
+/*
+ * Copyright (c) 2021, NVIDIA CORPORATION & AFFILIATES. All rights reserved.
+ *
+ * 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.
+ */
+
+#include 
+#include 
+
+#include "dpif-provider.h"
+
+void
+dpif_offload_sflow_recv_wait(const struct dpif *dpif)
+{
+const struct dpif_offload_class *offload_class = dpif->offload_class;
+
+if (offload_class && offload_class->sflow_recv_wait) {
+offload_class->sflow_recv_wait();
+}
+}
+
+int
+dpif_offload_sflow_recv(const struct dpif *dpif,
+struct dpif_offload_sflow *sflow)
+{
+const struct dpif_offload_class *offload_class = dpif->offload_class;
+
+if (offload_class && offload_class->sflow_recv) {
+return offload_class->sflow_recv(sflow);
+}
+
+return EOPNOTSUPP;
+}
diff --git a/lib/dpif-provider.h b/lib/dpif-provider.h
index 12477a24f..99009722a 100644
--- a/lib/dpif-provider.h
+++ b/lib/dpif-provider.h
@@ -22,8 +22,9 @@
  * exposed over OpenFlow as a single switch.  Datapaths and the collections of
  * ports that they contain may be fixed or dynamic. */
 
-#include "o

[ovs-dev] [PATCH v20 6/8] ofproto: Introduce API to process sFlow offload packet

[Chris Mi via dev]

Process sFlow offload packet in handler thread if handler id is 0.

Issue: 2181036
Change-Id: I40e0add3bff69751d8f957e5eeff58c665dd5d45
Signed-off-by: Chris Mi 
Reviewed-by: Eli Britstein 
Acked-by: Eelco Chaudron 
---
 ofproto/ofproto-dpif-upcall.c | 73 +++
 1 file changed, 73 insertions(+)

diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c
index 57f94df54..6e05466d5 100644
--- a/ofproto/ofproto-dpif-upcall.c
+++ b/ofproto/ofproto-dpif-upcall.c
@@ -22,6 +22,7 @@
 #include "connmgr.h"
 #include "coverage.h"
 #include "cmap.h"
+#include "lib/dpif-offload-provider.h"
 #include "lib/dpif-provider.h"
 #include "dpif.h"
 #include "openvswitch/dynamic-string.h"
@@ -779,6 +780,74 @@ udpif_get_n_flows(struct udpif *udpif)
 return flow_count;
 }
 
+static void
+process_offload_sflow(struct udpif *udpif, struct dpif_offload_sflow *sflow)
+{
+const struct dpif_sflow_attr *attr = sflow->attr;
+const struct user_action_cookie *cookie;
+struct dpif_sflow *dpif_sflow;
+struct ofproto_dpif *ofproto;
+struct upcall upcall;
+struct flow flow;
+
+if (!attr) {
+VLOG_WARN_RL(&rl, "sFlow upcall is missing its attribute");
+return;
+}
+
+cookie = nl_attr_get(attr->userdata);
+if (!cookie) {
+VLOG_WARN_RL(&rl, "sFlow user action cookie is missing");
+return;
+}
+if (nl_attr_get_size(attr->userdata) < sizeof(struct user_action_cookie)) {
+VLOG_WARN_RL(&rl, "sFlow user action cookie is corrupted");
+return;
+}
+ofproto = ofproto_dpif_lookup_by_uuid(&cookie->ofproto_uuid);
+if (!ofproto) {
+VLOG_WARN_RL(&rl, "sFlow upcall can't find ofproto dpif for UUID "
+ UUID_FMT, UUID_ARGS(&cookie->ofproto_uuid));
+return;
+}
+dpif_sflow = ofproto->sflow;
+if (!dpif_sflow) {
+VLOG_WARN_RL(&rl, "sFlow upcall is missing dpif information");
+return;
+}
+
+memset(&flow, 0, sizeof flow);
+if (attr->tunnel) {
+memcpy(&flow.tunnel, attr->tunnel, sizeof flow.tunnel);
+}
+flow.in_port.odp_port = netdev_ifindex_to_odp_port(sflow->iifindex);
+memset(&upcall, 0, sizeof upcall);
+upcall.flow = &flow;
+upcall.cookie = *cookie;
+upcall.packet = &sflow->packet;
+upcall.sflow = dpif_sflow;
+upcall.ufid = &attr->ufid;
+upcall.type = SFLOW_UPCALL;
+process_upcall(udpif, &upcall, NULL, NULL);
+}
+
+static void
+recv_offload_sflow(struct udpif *udpif)
+{
+size_t n_sflow = 0;
+
+while (n_sflow < UPCALL_MAX_BATCH) {
+struct dpif_offload_sflow sflow;
+
+if (dpif_offload_sflow_recv(udpif->dpif, &sflow)) {
+break;
+}
+process_offload_sflow(udpif, &sflow);
+n_sflow++;
+}
+dpif_offload_sflow_recv_wait(udpif->dpif);
+}
+
 /* The upcall handler thread tries to read a batch of UPCALL_MAX_BATCH
  * upcalls from dpif, processes the batch and installs corresponding flows
  * in dpif. */
@@ -795,6 +864,10 @@ udpif_upcall_handler(void *arg)
 dpif_recv_wait(udpif->dpif, handler->handler_id);
 latch_wait(&udpif->exit_latch);
 }
+/* Only handler id 0 thread process sFlow offload packet. */
+if (handler->handler_id == 0) {
+recv_offload_sflow(udpif);
+}
 poll_block();
 }
 
-- 
2.30.2

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

[ovs-dev] [PATCH v20 5/8] dpif-offload-netlink: Implement dpif-offload-provider API

[Chris Mi via dev]

Implement dpif-offload API for netlink datapath. And implement a
dummy dpif-offload API for netdev datapath to make tests pass.

Issue: 2181036
Change-Id: I950c3cc3c7092c3d87602da8928ad4aa2df4f38a
Signed-off-by: Chris Mi 
Reviewed-by: Eli Britstein 
---
 lib/automake.mk |   2 +
 lib/dpif-netdev.c   |   8 +-
 lib/dpif-netlink.c  |   4 +-
 lib/dpif-offload-netdev.c   |  43 +++
 lib/dpif-offload-netlink.c  | 221 
 lib/dpif-offload-provider.h |  25 +++-
 lib/dpif-offload.c  | 157 +
 lib/dpif-provider.h |   6 +-
 lib/dpif.c  |  17 ++-
 9 files changed, 476 insertions(+), 7 deletions(-)
 create mode 100644 lib/dpif-offload-netdev.c
 create mode 100644 lib/dpif-offload-netlink.c

diff --git a/lib/automake.mk b/lib/automake.mk
index 781fba47a..9ed61029a 100644
--- a/lib/automake.mk
+++ b/lib/automake.mk
@@ -130,6 +130,7 @@ lib_libopenvswitch_la_SOURCES = \
lib/dpif-netdev-perf.c \
lib/dpif-netdev-perf.h \
lib/dpif-offload.c \
+   lib/dpif-offload-netdev.c \
lib/dpif-offload-provider.h \
lib/dpif-provider.h \
lib/dpif.c \
@@ -453,6 +454,7 @@ lib_libopenvswitch_la_SOURCES += \
lib/dpif-netlink.h \
lib/dpif-netlink-rtnl.c \
lib/dpif-netlink-rtnl.h \
+   lib/dpif-offload-netlink.c \
lib/if-notifier.c \
lib/netdev-linux.c \
lib/netdev-linux.h \
diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index e28e0b554..5ebd127b9 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -1683,7 +1683,8 @@ create_dpif_netdev(struct dp_netdev *dp)
 ovs_refcount_ref(&dp->ref_cnt);
 
 dpif = xmalloc(sizeof *dpif);
-dpif_init(&dpif->dpif, dp->class, dp->name, netflow_id >> 8, netflow_id);
+dpif_init(&dpif->dpif, dp->class, NULL, dp->name, netflow_id >> 8,
+  netflow_id);
 dpif->dp = dp;
 dpif->last_port_seq = seq_read(dp->port_seq);
 
@@ -9639,6 +9640,10 @@ dpif_dummy_override(const char *type)
 if (error == 0 || error == EAFNOSUPPORT) {
 dpif_dummy_register__(type);
 }
+error = dp_offload_unregister_provider(type);
+if (error == 0 || error == EAFNOSUPPORT) {
+dpif_offload_dummy_register(type);
+}
 }
 
 void
@@ -9659,6 +9664,7 @@ dpif_dummy_register(enum dummy_level level)
 }
 
 dpif_dummy_register__("dummy");
+dpif_offload_dummy_register("dummy");
 
 unixctl_command_register("dpif-dummy/change-port-number",
  "dp port new-number",
diff --git a/lib/dpif-netlink.c b/lib/dpif-netlink.c
index 71e35ccdd..75f85c254 100644
--- a/lib/dpif-netlink.c
+++ b/lib/dpif-netlink.c
@@ -461,8 +461,8 @@ open_dpif(const struct dpif_netlink_dp *dp, struct dpif 
**dpifp)
 dpif->port_notifier = NULL;
 fat_rwlock_init(&dpif->upcall_lock);
 
-dpif_init(&dpif->dpif, &dpif_netlink_class, dp->name,
-  dp->dp_ifindex, dp->dp_ifindex);
+dpif_init(&dpif->dpif, &dpif_netlink_class, &dpif_offload_netlink_class,
+  dp->name, dp->dp_ifindex, dp->dp_ifindex);
 
 dpif->dp_ifindex = dp->dp_ifindex;
 dpif->user_features = dp->user_features;
diff --git a/lib/dpif-offload-netdev.c b/lib/dpif-offload-netdev.c
new file mode 100644
index 0..35dac2cc3
--- /dev/null
+++ b/lib/dpif-offload-netdev.c
@@ -0,0 +1,43 @@
+/*
+ * Copyright (c) 2021, NVIDIA CORPORATION & AFFILIATES. All rights reserved.
+ *
+ * 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.
+ */
+
+#include 
+
+#include "dpif.h"
+#include "dpif-offload-provider.h"
+#include "openvswitch/vlog.h"
+
+VLOG_DEFINE_THIS_MODULE(dpif_offload_netdev);
+
+/* Currently, it is used only for dummy netdev to make tests pass. */
+const struct dpif_offload_class dpif_offload_netdev_class = {
+.type = "netdev",
+.init = NULL,
+.destroy = NULL,
+.sflow_recv_wait = NULL,
+.sflow_recv = NULL,
+};
+
+void
+dpif_offload_dummy_register(const char *type)
+{
+struct dpif_offload_class *class;
+
+class = xmalloc(sizeof *class);
+*class = dpif_offload_netdev_class;
+class->type = xstrdup(type);
+dp_offload_register_provider(class);
+}
diff --git a/lib/dpif-offload-netlink.c b/lib/dpif-offload-netlink.c
new file mode 100644
index 0..02aea7e2d
--- /dev/null
+++ b/lib/dpif-offload-netlink.c
@@ -0,0 +1,221 @@
+/*
+ * Copyright (c) 2021, NVIDIA CORPORATION & AFFILIATES. All rights reserved.
+ *
+ * Licens

[ovs-dev] [PATCH v20 1/8] compat: Add psample and tc sample action defines for older kernels

[Chris Mi via dev]

Update kernel UAPI to support psample and the tc sample action.

Issue: 2181036
Change-Id: I67822667494230bdc1d1f4b2217d019a65486ad3
Signed-off-by: Chris Mi 
Reviewed-by: Eli Britstein 
Acked-by: Eelco Chaudron 
---
 include/linux/automake.mk|  4 ++-
 include/linux/psample.h  | 62 
 include/linux/tc_act/tc_sample.h | 25 +
 3 files changed, 90 insertions(+), 1 deletion(-)
 create mode 100644 include/linux/psample.h
 create mode 100644 include/linux/tc_act/tc_sample.h

diff --git a/include/linux/automake.mk b/include/linux/automake.mk
index 8f063f482..c48d9699a 100644
--- a/include/linux/automake.mk
+++ b/include/linux/automake.mk
@@ -7,4 +7,6 @@ noinst_HEADERS += \
include/linux/tc_act/tc_skbedit.h \
include/linux/tc_act/tc_tunnel_key.h \
include/linux/tc_act/tc_vlan.h \
-   include/linux/tc_act/tc_ct.h
+   include/linux/tc_act/tc_ct.h \
+   include/linux/tc_act/tc_sample.h \
+   include/linux/psample.h
diff --git a/include/linux/psample.h b/include/linux/psample.h
new file mode 100644
index 0..e585db5bf
--- /dev/null
+++ b/include/linux/psample.h
@@ -0,0 +1,62 @@
+/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
+#ifndef __UAPI_PSAMPLE_H
+#define __UAPI_PSAMPLE_H
+
+enum {
+   PSAMPLE_ATTR_IIFINDEX,
+   PSAMPLE_ATTR_OIFINDEX,
+   PSAMPLE_ATTR_ORIGSIZE,
+   PSAMPLE_ATTR_SAMPLE_GROUP,
+   PSAMPLE_ATTR_GROUP_SEQ,
+   PSAMPLE_ATTR_SAMPLE_RATE,
+   PSAMPLE_ATTR_DATA,
+   PSAMPLE_ATTR_GROUP_REFCOUNT,
+   PSAMPLE_ATTR_TUNNEL,
+
+   PSAMPLE_ATTR_PAD,
+   PSAMPLE_ATTR_OUT_TC,/* u16 */
+   PSAMPLE_ATTR_OUT_TC_OCC,/* u64, bytes */
+   PSAMPLE_ATTR_LATENCY,   /* u64, nanoseconds */
+   PSAMPLE_ATTR_TIMESTAMP, /* u64, nanoseconds */
+   PSAMPLE_ATTR_PROTO, /* u16 */
+
+   __PSAMPLE_ATTR_MAX
+};
+
+enum psample_command {
+   PSAMPLE_CMD_SAMPLE,
+   PSAMPLE_CMD_GET_GROUP,
+   PSAMPLE_CMD_NEW_GROUP,
+   PSAMPLE_CMD_DEL_GROUP,
+};
+
+enum psample_tunnel_key_attr {
+   PSAMPLE_TUNNEL_KEY_ATTR_ID, /* be64 Tunnel ID */
+   PSAMPLE_TUNNEL_KEY_ATTR_IPV4_SRC,   /* be32 src IP address. */
+   PSAMPLE_TUNNEL_KEY_ATTR_IPV4_DST,   /* be32 dst IP address. */
+   PSAMPLE_TUNNEL_KEY_ATTR_TOS,/* u8 Tunnel IP ToS. */
+   PSAMPLE_TUNNEL_KEY_ATTR_TTL,/* u8 Tunnel IP TTL. */
+   PSAMPLE_TUNNEL_KEY_ATTR_DONT_FRAGMENT,  /* No argument, set DF. */
+   PSAMPLE_TUNNEL_KEY_ATTR_CSUM,   /* No argument. CSUM 
packet. */
+   PSAMPLE_TUNNEL_KEY_ATTR_OAM,/* No argument. OAM frame.  
*/
+   PSAMPLE_TUNNEL_KEY_ATTR_GENEVE_OPTS,/* Array of Geneve options. 
*/
+   PSAMPLE_TUNNEL_KEY_ATTR_TP_SRC, /* be16 src Transport Port. 
*/
+   PSAMPLE_TUNNEL_KEY_ATTR_TP_DST, /* be16 dst Transport Port. 
*/
+   PSAMPLE_TUNNEL_KEY_ATTR_VXLAN_OPTS, /* Nested VXLAN opts* */
+   PSAMPLE_TUNNEL_KEY_ATTR_IPV6_SRC,   /* struct in6_addr src IPv6 
address. */
+   PSAMPLE_TUNNEL_KEY_ATTR_IPV6_DST,   /* struct in6_addr dst IPv6 
address. */
+   PSAMPLE_TUNNEL_KEY_ATTR_PAD,
+   PSAMPLE_TUNNEL_KEY_ATTR_ERSPAN_OPTS,/* struct erspan_metadata */
+   PSAMPLE_TUNNEL_KEY_ATTR_IPV4_INFO_BRIDGE,   /* No argument. 
IPV4_INFO_BRIDGE mode.*/
+   __PSAMPLE_TUNNEL_KEY_ATTR_MAX
+};
+
+/* Can be overridden at runtime by module option */
+#define PSAMPLE_ATTR_MAX (__PSAMPLE_ATTR_MAX - 1)
+
+#define PSAMPLE_NL_MCGRP_CONFIG_NAME "config"
+#define PSAMPLE_NL_MCGRP_SAMPLE_NAME "packets"
+#define PSAMPLE_GENL_NAME "psample"
+#define PSAMPLE_GENL_VERSION 1
+
+#endif
diff --git a/include/linux/tc_act/tc_sample.h b/include/linux/tc_act/tc_sample.h
new file mode 100644
index 0..fee1bcc20
--- /dev/null
+++ b/include/linux/tc_act/tc_sample.h
@@ -0,0 +1,25 @@
+/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
+#ifndef __LINUX_TC_SAMPLE_H
+#define __LINUX_TC_SAMPLE_H
+
+#include 
+#include 
+#include 
+
+struct tc_sample {
+   tc_gen;
+};
+
+enum {
+   TCA_SAMPLE_UNSPEC,
+   TCA_SAMPLE_TM,
+   TCA_SAMPLE_PARMS,
+   TCA_SAMPLE_RATE,
+   TCA_SAMPLE_TRUNC_SIZE,
+   TCA_SAMPLE_PSAMPLE_GROUP,
+   TCA_SAMPLE_PAD,
+   __TCA_SAMPLE_MAX
+};
+#define TCA_SAMPLE_MAX (__TCA_SAMPLE_MAX - 1)
+
+#endif
-- 
2.30.2

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

[ovs-dev] [PATCH v20 0/8] Add offload support for sFlow

[Chris Mi via dev]

This patch set adds offload support for sFlow.

Psample is a genetlink channel for packet sampling. TC action act_sample
uses psample to send sampled packets to userspace.

When offloading sample action to TC, userspace creates a unique ID to
map sFlow action and tunnel info and passes this ID to kernel instead
of the sFlow info. psample will send this ID and sampled packet to
userspace. Using the ID, userspace can recover the sFlow info and send
sampled packet to the right sFlow monitoring host.

v2-v1:
- Fix robot errors.
v3-v2:
- Remove Gerrit Change-Id.
- Add patch #9 to fix older kernels build issue.
- Add travis test result.
v4-v3:
- Fix offload issue when sampling rate is 1.
v5-v4:
- Move polling thread from ofproto to netdev-offload-tc.
v6-v5:
- Rebase.
- Add GitHub Actions test result.
v7-v6:
- Remove Gerrit Change-Id.
- Fix "ERROR: Inappropriate spacing around cast"
v8-v7
- Address Eelco Chaudron's comment for patch #11.
v9-v8
- Remove sflow_len from struct dpif_sflow_attr.
- Log a debug message for other userspace actions.
v10-v9
- Address Eelco Chaudron's comments on v9.
v11-v10
- Fix a bracing error.
v12-v11
- Add duplicate sample group id check.
v13-v12
- Remove the psample poll thread from netdev-offload-tc and reuse
  ofproto handler thread according to Ilya's new desgin.
- Add dpif-offload-provider layer according to Eli's suggestion.
v14-v13
- Fix a robot error.
v15-v14
- Address Eelco Chaudron's comments on v14.
v16-v15
- Address Eelco Chaudron's comments on v15.
- Add two test cases.
v17-v16
- Address Eelco Chaudron's comments on v16.
- Move struct dpif_offload_api from struct dpif_class to struct dpif.
v18-v17
- Rename dpif_offload_api to dpif_offload_class.
- Add init and destroy callbacks in dpif_offload_class. They are called
  when registering dpif_offload_class.
v19-18
- Fix a bug that psample_sock is destroyed when last bridge is deleted.
v20-19
- Move buf_stub to struct dpif_offload_sflow avoid garbage values when
  ofproto proceses the sampled packet.

Chris Mi (8):
  compat: Add psample and tc sample action defines for older kernels
  ovs-kmod-ctl: Load kernel module psample
  dpif-offload-provider: Introduce dpif-offload-provider layer
  netdev-offload-tc: Introduce group ID management API
  dpif-offload-netlink: Implement dpif-offload-provider API
  ofproto: Introduce API to process sFlow offload packet
  netdev-offload-tc: Add offload support for sFlow
  system-offloads-traffic.at: Add sFlow offload test cases

 NEWS |   1 +
 include/linux/automake.mk|   4 +-
 include/linux/psample.h  |  62 
 include/linux/tc_act/tc_sample.h |  25 ++
 lib/automake.mk  |   4 +
 lib/dpif-netdev.c|   8 +-
 lib/dpif-netlink.c   |   4 +-
 lib/dpif-offload-netdev.c|  43 +++
 lib/dpif-offload-netlink.c   | 227 +++
 lib/dpif-offload-provider.h  |  94 +++
 lib/dpif-offload.c   | 200 +
 lib/dpif-provider.h  |  10 +-
 lib/dpif.c   |  17 +-
 lib/netdev-offload-tc.c  | 466 +--
 lib/netdev-offload.h |   2 +
 lib/tc.c |  61 +++-
 lib/tc.h |  16 +-
 ofproto/ofproto-dpif-upcall.c|  73 +
 tests/system-offloads-traffic.at | 101 +++
 utilities/ovs-kmod-ctl.in|  14 +
 20 files changed, 1400 insertions(+), 32 deletions(-)
 create mode 100644 include/linux/psample.h
 create mode 100644 include/linux/tc_act/tc_sample.h
 create mode 100644 lib/dpif-offload-netdev.c
 create mode 100644 lib/dpif-offload-netlink.c
 create mode 100644 lib/dpif-offload-provider.h
 create mode 100644 lib/dpif-offload.c

-- 
2.30.2

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