Re: [ovs-dev] [Patch ovn 1/2] actions.c/h: Enable specifying zone for ct_commit.
On Wed, Mar 6, 2024 at 7:26 AM Martin Kalcok
wrote:
> Hi Ales,
> Thank you for review and helpful comments. I'll update commit subjects for
> this series in v2.
>
> On Wed, Mar 6, 2024 at 7:45 AM Ales Musil wrote:
>
>>
>>
>> On Mon, Mar 4, 2024 at 11:56 AM Martin Kalcok <
>> martin.kal...@canonical.com> wrote:
>>
>>> Action `ct_commit` currently does not allow specifying zone into
>>> which connection is committed. For example, in LR datapath, the
>>> `ct_commit`
>>> will always use the DNAT zone.
>>>
>>> This change adds option to use `ct_commit(snat)` or `ct_commit(dnat)` to
>>> explicitly specify the zone into which the connection will be committed.
>>>
>>> Original behavior of `ct_commit` without the arguments remains unchanged.
>>>
>>> Signed-off-by: Martin Kalcok
>>>
>>
>> Hi Martin,
>>
>> thank you for the followup, I have a few comments down below.
>> One small nit for the commit subject, we usually specify only the module
>> name without .c/.h e.g. action, ovn-controller, northd etc.
>> AFAIR ct_commit_v1 is deprecated, but I guess it's fine to repurpose it
>> for this. Maybe we could remove the old behavior and leave just this?
>> Before posting v2 we should discuss this with others.
>> Adding Dumitru and Numan to this discussion.
>>
>
> Ack, I'll defer to your recommendation as to where this should be
> implemented. If I read it correctly, `parse_ct_commit_v1_arg` still has
> code to parse mark/label options but it's never used because if `ct_commit`
> action is followed by "{", the `parse_CT_COMMIT` will send it to
> `parse_nested_action` instead. If that's the case and the
> `parse_CT_COMMIT_V1` is unused, I'd be happy to remove it/repurpose it to
> something like `parse_CT_COMMIT_ZONE` that handles only
> `ct_commit({snat,dnat})` actions.
>
Instead of modifying ct_commit, I'd suggest adding 2 new actions -
ct_commit_snat and ct_commit_dnat.
This would not have any ambiguity on the backward compatibility.
Thanks
Numan
>
>>
>>> ---
>>> include/ovn/actions.h | 9 +
>>> lib/actions.c | 20 +++-
>>> ovn-sb.xml| 9 +
>>> 3 files changed, 37 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/include/ovn/actions.h b/include/ovn/actions.h
>>> index 49fb96fc6..ce9597cf5 100644
>>> --- a/include/ovn/actions.h
>>> +++ b/include/ovn/actions.h
>>> @@ -259,11 +259,20 @@ struct ovnact_ct_next {
>>> uint8_t ltable;/* Logical table ID of next table. */
>>> };
>>>
>>> +/* Conntrack zone to be used for commiting CT entries by the action.
>>> + * DEFAULT uses default zone for given datapath. */
>>> +enum ovnact_ct_zone {
>>> +OVNACT_CT_ZONE_DEFAULT,
>>> +OVNACT_CT_ZONE_SNAT,
>>> +OVNACT_CT_ZONE_DNAT,
>>> +};
>>> +
>>>
>>
>> There is no need for this enum, see details below.
>>
>>
>>> /* OVNACT_CT_COMMIT_V1. */
>>> struct ovnact_ct_commit_v1 {
>>> struct ovnact ovnact;
>>> uint32_t ct_mark, ct_mark_mask;
>>> ovs_be128 ct_label, ct_label_mask;
>>> +enum ovnact_ct_zone zone;
>>>
>> };
>>>
>>> /* Type of NAT used for the particular action.
>>> diff --git a/lib/actions.c b/lib/actions.c
>>> index a45874dfb..319e65b6f 100644
>>> --- a/lib/actions.c
>>> +++ b/lib/actions.c
>>> @@ -707,6 +707,7 @@ static void
>>> parse_ct_commit_v1_arg(struct action_context *ctx,
>>> struct ovnact_ct_commit_v1 *cc)
>>> {
>>> +cc->zone = OVNACT_CT_ZONE_DEFAULT;
>>> if (lexer_match_id(ctx->lexer, "ct_mark")) {
>>> if (!lexer_force_match(ctx->lexer, LEX_T_EQUALS)) {
>>> return;
>>> @@ -737,6 +738,10 @@ parse_ct_commit_v1_arg(struct action_context *ctx,
>>> return;
>>> }
>>> lexer_get(ctx->lexer);
>>> +} else if (lexer_match_id(ctx->lexer, "snat")) {
>>> +cc->zone = OVNACT_CT_ZONE_SNAT;
>>> +} else if (lexer_match_id(ctx->lexer, "dnat")) {
>>> +cc->zone = OVNACT_CT_ZONE_DNAT;
>>> } else {
>>> lexer_syntax_error(ctx->lexer, NULL);
>>> }
>>> @@ -814,7 +819,20 @@ encode_CT_COMMIT_V1(const struct
>>> ovnact_ct_commit_v1 *cc,
>>> struct ofpact_conntrack *ct = ofpact_put_CT(ofpacts);
>>> ct->flags = NX_CT_F_COMMIT;
>>> ct->recirc_table = NX_CT_RECIRC_NONE;
>>> -ct->zone_src.field = mf_from_id(MFF_LOG_CT_ZONE);
>>> +switch (cc->zone) {
>>> +case OVNACT_CT_ZONE_SNAT:
>>> +ct->zone_src.field = mf_from_id(MFF_LOG_SNAT_ZONE);
>>> +break;
>>> +
>>> +case OVNACT_CT_ZONE_DNAT:
>>> +ct->zone_src.field = mf_from_id(MFF_LOG_DNAT_ZONE);
>>> +break;
>>> +
>>> +case OVNACT_CT_ZONE_DEFAULT:
>>> +default:
>>> +ct->zone_src.field = mf_from_id(MFF_LOG_CT_ZONE);
>>> +break;
>>> +}
>>>
>>
>> This would actually break potential use in the switch pipeline when
>> someone would specify ct_commit(snat)/ct_commit(dnat).
>> The switch pipeline uses MFF_LOG_CT_ZONE only. See ct_commit_nat for
>> example [0]. There is only indication
[ovs-dev] [PATCH v3] ovsdb: Don't iterate over rows on empty mutation.
Previously when an empty mutation was used to count the number of rows
in a table, OVSDB would iterate over all rows twice. First to perform an
RBAC check, and then to perform the no-operation.
This change adds a short circuit to mutate operations with no conditions
and an empty mutation set, returning immediately. One notable change in
functionality is not performing the RBAC check in this condition, as no
mutation actually takes place.
Reported-by: Terry Wilson
Reported-at: https://issues.redhat.com/browse/FDP-359
Signed-off-by: Mike Pattrick
---
v2: Added additional non-rbac tests, and support for conditional
counting without the rbac check
v3: Changed a struct to a size_t.
---
ovsdb/execution.c| 23 +-
ovsdb/mutation.h | 6 +
tests/ovsdb-execution.at | 51
tests/ovsdb-rbac.at | 23 ++
4 files changed, 102 insertions(+), 1 deletion(-)
diff --git a/ovsdb/execution.c b/ovsdb/execution.c
index 8c20c3b54..f4cc9e802 100644
--- a/ovsdb/execution.c
+++ b/ovsdb/execution.c
@@ -585,6 +585,16 @@ mutate_row_cb(const struct ovsdb_row *row, void *mr_)
return *mr->error == NULL;
}
+static bool
+count_row_cb(const struct ovsdb_row *row OVS_UNUSED, void *rc)
+{
+size_t *row_count = rc;
+
+(*row_count)++;
+
+return true;
+}
+
static struct ovsdb_error *
ovsdb_execute_mutate(struct ovsdb_execution *x, struct ovsdb_parser *parser,
struct json *result)
@@ -609,7 +619,18 @@ ovsdb_execute_mutate(struct ovsdb_execution *x, struct
ovsdb_parser *parser,
error = ovsdb_condition_from_json(table->schema, where, x->symtab,
);
}
-if (!error) {
+if (!error && ovsdb_mutation_set_empty()) {
+/* Special case with no mutations, just return the row count. */
+if (ovsdb_condition_empty()) {
+json_object_put(result, "count",
+json_integer_create(hmap_count(>rows)));
+} else {
+size_t row_count = 0;
+ovsdb_query(table, , count_row_cb, _count);
+json_object_put(result, "count",
+json_integer_create(row_count));
+}
+} else if (!error) {
mr.n_matches = 0;
mr.txn = x->txn;
mr.mutations =
diff --git a/ovsdb/mutation.h b/ovsdb/mutation.h
index 7566ef199..05d4a262a 100644
--- a/ovsdb/mutation.h
+++ b/ovsdb/mutation.h
@@ -69,4 +69,10 @@ void ovsdb_mutation_set_destroy(struct ovsdb_mutation_set *);
struct ovsdb_error *ovsdb_mutation_set_execute(
struct ovsdb_row *, const struct ovsdb_mutation_set *)
OVS_WARN_UNUSED_RESULT;
+static inline bool ovsdb_mutation_set_empty(
+const struct ovsdb_mutation_set *ms)
+{
+return ms->n_mutations == 0;
+}
+
#endif /* ovsdb/mutation.h */
diff --git a/tests/ovsdb-execution.at b/tests/ovsdb-execution.at
index fd1c7a239..1ffa2b738 100644
--- a/tests/ovsdb-execution.at
+++ b/tests/ovsdb-execution.at
@@ -1201,4 +1201,55 @@ OVSDB_CHECK_EXECUTION([garbage collection],
[{"rows":[]}]
]])])
+OVSDB_CHECK_EXECUTION([insert rows, count with mutation],
+ [ordinal_schema],
+ "ordinals",
+ {"op": "insert",
+ "table": "ordinals",
+ "row": {"number": 0, "name": "zero"},
+ "uuid-name": "first"}]]],
+ [[["ordinals",
+ {"op": "insert",
+ "table": "ordinals",
+ "row": {"number": 1, "name": "one"},
+ "uuid-name": "first"}]]],
+ [[["ordinals",
+ {"op": "mutate",
+ "table": "ordinals",
+ "where": [["name", "==", "zero"]],
+ "mutations": []}]]],
+ [[["ordinals",
+ {"op": "mutate",
+ "table": "ordinals",
+ "where": [["name", "==", "one"]],
+ "mutations": []}]]],
+ [[["ordinals",
+ {"op": "insert",
+ "table": "ordinals",
+ "row": {"number": 2, "name": "one"},
+ "uuid-name": "first"}]]],
+ [[["ordinals",
+ {"op": "mutate",
+ "table": "ordinals",
+ "where": [["name", "==", "one"]],
+ "mutations": []}]]],
+ [[["ordinals",
+ {"op": "delete",
+ "table": "ordinals",
+ "where": [["name", "==", "zero"]]}]]],
+ [[["ordinals",
+ {"op": "mutate",
+ "table": "ordinals",
+ "where": [],
+ "mutations": []},
+ [[[{"uuid":["uuid","<0>"]}]
+[{"uuid":["uuid","<1>"]}]
+[{"count":1}]
+[{"count":1}]
+[{"uuid":["uuid","<2>"]}]
+[{"count":2}]
+[{"count":1}]
+[{"count":2}]
+]])
+
EXECUTION_EXAMPLES
diff --git a/tests/ovsdb-rbac.at b/tests/ovsdb-rbac.at
index 3172e4bf5..c1e5a9134 100644
--- a/tests/ovsdb-rbac.at
+++ b/tests/ovsdb-rbac.at
@@ -355,6 +355,29 @@ AT_CHECK([uuidfilt stdout], [0], [[[{"details":"RBAC rules
for client \"client-2
], [ignore])
# Test 14:
+# Count the rows in other_colors. This should pass even though the RBAC
+# authorization would fail because "client-2" does not match the
+# "creator" column for this row. Because the RBAC check is bypassed
Re: [ovs-dev] [RFC] bridge: Retry tunnel port addition for conflict.
On 2/27/24 20:14, Han Zhou wrote: > For kernel datapath, when a new tunnel port is created in the same > transaction in which an old tunnel port with the same tunnel > configuration is deleted, the new tunnel port creation will fail and > left in an error state. This can be easily reproduced in OVN by > triggering a chassis deletion and addition with the same encap in the > same SB DB transaction, such as: > > ovn-sbctl chassis-add aa geneve 1.2.3.4 > ovn-sbctl chassis-del aa -- chassis-add bb 1.2.3.4 > > ovs-vsctl show | grep error > error: "could not add network device ovn-bb-0 to ofproto (File exists)" > > Related logs in OVS: > — > 2024-02-23T05:41:49.978Z|405933|bridge|INFO|bridge br-int: deleted interface > ovn-aa-0 on port 113 > 2024-02-23T05:41:49.989Z|405935|tunnel|WARN|ovn-bb-0: attempting to add > tunnel port with same config as port 'ovn-aa-0' (::->1.2.3.4, key=flow, > legacy_l2, dp port=9) > 2024-02-23T05:41:49.989Z|405936|ofproto|WARN|br-int: could not add port > ovn-bb-0 (File exists) > 2024-02-23T05:41:49.989Z|405937|bridge|WARN|could not add network device > ovn-bb-0 to ofproto (File exists) > — Hi, Han. Thanks for the patch! > > Depending on when there are other OVSDB changes, it may take a long time > for the device to be added successfully, triggered by the next OVS > iteration. > > (note: native tunnel ports do not have this problem) I don't think this is correct. The code path is common for both system and native tunnels. I can reproduce the issues in a sandbox with: $ make -j8 sandbox SANDBOXFLAGS="\-\-dummy='system'" [tutorial]$ ovs-vsctl add-port br0 tunnel_port \ -- set Interface tunnel_port \ type=geneve options:remote_ip=flow options:key=123 [tutorial]$ ovs-vsctl del-port tunnel_port \ -- add-port br0 tunnel_port2 \ -- set Interface tunnel_port2 \ type=geneve options:remote_ip=flow options:key=123 ovs-vsctl: Error detected while setting up 'tunnel_port2': could not add network device tunnel_port2 to ofproto (File exists). See ovs-vswitchd log for details. The same should work in a testsuite as well, i.e. we should be able to create a test for this scenario. Note: The --dummy=system prevents OVS from replacing tunnel ports with dummy ones. > > The problem is how the tunnel port deletion and creation are handled. In > bridge_reconfigure(), port deletion is handled before creation, to avoid > such resource conflict. However, for kernel tunnel ports, the real clean > up is performed at the end of the bridge_reconfigure() in the: > bridge_run__()->ofproto_run()->...->ofproto_dpif:port_destruct() > > We cannot call bridge_run__() at an earlier point before all > reconfigurations are done, so this patch tries a generic approach to > just re-run the bridge_reconfigure() when there are any port creations > encountered "File exists" error, which indicates a possible resource > conflict may have happened due to a later deleted port, and retry may > succeed. > > Signed-off-by: Han Zhou > --- > This is RFC because I am not sure if there is a better way to solve the > problem > more specifically by executing the port_destruct for the old port before > trying > to create the new port. The fix may be more complex though. I don't think re-trying is a good approach in general. We should likely just destroy the tnl_port structure right away, similarly to how we clean up stp, lldp and bundles in ofproto_port_unregister(). Maybe we can add a new callback similar to bundle_remove() and call tnl_port_del() from it? (I didn't try, so I'm not 100% sure this will not cause any issues.) What do you think? Best regards, Ilya Maximets. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH branch-2.17] conntrack: Fix flush not flushing all elements.
Bleep bloop. Greetings Simon Horman, 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: WARNING: Unexpected sign-offs from developers who are not authors or co-authors or committers: Simon Horman Lines checked: 145, Warnings: 1, Errors: 0 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 OVN] Add support to make fdb table local to the chassis.
> On 18-Dec-2023, at 8:53 PM, Dumitru Ceara wrote:
>
> On 12/18/23 16:17, Naveen Yerramneni wrote:
>>
>>
>>> On 18-Dec-2023, at 7:26 PM, Dumitru Ceara wrote:
>>>
>>> On 11/30/23 16:32, Dumitru Ceara wrote:
On 11/30/23 15:54, Naveen Yerramneni wrote:
>
>
>> On 30-Nov-2023, at 6:06 PM, Dumitru Ceara wrote:
>>
>> On 11/30/23 09:45, Naveen Yerramneni wrote:
>>>
>>>
On 29-Nov-2023, at 2:24 PM, Dumitru Ceara wrote:
On 11/29/23 07:45, naveen.yerramneni wrote:
> This functionality can be enabled at the logical switch level:
> - "other_config:fdb_local" can be used to enable/disable this
> functionality, it is disabled by default.
> - "other_config:fdb_local_idle_timeout" sepcifies idle timeout
> for locally learned fdb flows, default timeout is 300 secs.
>
> If enabled, below lflow is added for each port that has unknown addr
> set.
> - table=2 (ls_in_lookup_fdb), priority=100, match=(inport ==
> ),
> action=(commit_fdb_local(timeout=); next;
>
> New OVN action: "commit_fdb_local". This sets following OVS action.
> -
> learn(table=71,idle_timeout=,delete_learned,OXM_OF_METADATA[],
>
> NXM_OF_ETH_DST[]=NXM_OF_ETH_SRC[],load:NXM_NX_REG14[]->NXM_NX_REG15[])
>
> This is useful when OVN is managing VLAN network that has multiple
> ports
> set with unknown addr and localnet_learn_fdb is enabled. With this
> config,
> if there is east-west traffic flowing between VMs part of same VLAN
> deployed on different hypervisors then, MAC addrs of the source and
> destination VMs keeps flapping between VM port and localnet port in
> Southbound FDB table. Enabling fdb_local config makes fdb table local
> to
> the chassis and avoids MAC flapping.
>
> Signed-off-by: Naveen Yerramneni
> ---
Hi Naveen,
Thanks a lot for the patch!
Just a note, we already have a fix for the east-west traffic that
causes
FDB flapping when localnet is used:
https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_ovn-2Dorg_ovn_commit_2acf91e9628e9481c48e4a6cec8ad5159fdd6d2e=DwICaQ=s883GpUCOChKOHiocYtGcg=2PQjSDR7A28z1kXE1ptSm6X36oL_nCq1XxeEt7FkLmA=kPuq992rikXYk63APGxlIpfqY3lPpreN9f4ha9pZKpodnVgE9KfjEUNozpPUFzUu=LP9_zs2Rj34vMx20ntbu-A3taXqKMJNVH2TLQyOXCh0=
https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_ovn-2Dorg_ovn_commit_f3a14907fe2b1ecdcfddfbed595cd097b6efbe14=DwICaQ=s883GpUCOChKOHiocYtGcg=2PQjSDR7A28z1kXE1ptSm6X36oL_nCq1XxeEt7FkLmA=kPuq992rikXYk63APGxlIpfqY3lPpreN9f4ha9pZKpodnVgE9KfjEUNozpPUFzUu=gsUGtjyf9gSOr1LkcCH0O6MB1_tjXi9fuTgwEFgbRx8=
In general, however, I think it's a very good idea to move the FDB away
from the Southbound and make it local to each hypervisor. That reduces
load on the Southbound among other things.
>>>
>>> Hi Dumitru,
>>>
>>> Thanks for informing about the patches.
>>> Yes, local FDB reduces load on southbound.
>>>
>>>
> include/ovn/actions.h | 7 +++
> lib/actions.c | 94
> northd/northd.c | 26 ++
> ovn-nb.xml| 14 ++
> tests/ovn.at | 108 ++
> utilities/ovn-trace.c | 2 +
> 6 files changed, 251 insertions(+)
>
> diff --git a/include/ovn/actions.h b/include/ovn/actions.h
> index 49cfe0624..85ac92cd3 100644
> --- a/include/ovn/actions.h
> +++ b/include/ovn/actions.h
> @@ -127,6 +127,7 @@ struct collector_set_ids;
> OVNACT(CHK_LB_AFF,ovnact_result) \
> OVNACT(SAMPLE,ovnact_sample) \
> OVNACT(MAC_CACHE_USE, ovnact_null)\
> +OVNACT(COMMIT_FDB_LOCAL, ovnact_commit_fdb_local) \
>
> /* enum ovnact_type, with a member OVNACT_ for each action. */
> enum OVS_PACKED_ENUM ovnact_type {
> @@ -514,6 +515,12 @@ struct ovnact_commit_lb_aff {
> uint16_t timeout;
> };
>
> +/* OVNACT_COMMIT_FBD_LOCAL. */
> +struct ovnact_commit_fdb_local{
> +struct ovnact ovnact;
> +uint16_t timeout; /* fdb_local flow timeout */
> +};
> +
> /* Internal use by the helpers below. */
> void ovnact_init(struct ovnact *, enum ovnact_type, size_t len);
> void *ovnact_put(struct ofpbuf *, enum ovnact_type, size_t len);
> diff --git a/lib/actions.c b/lib/actions.c
> index a73fe1a1e..f5aa78db1 100644
> ---
Re: [ovs-dev] [PATCH branch-3.0] conntrack: Fix flush not flushing all elements.
Bleep bloop. Greetings Simon Horman, 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: WARNING: Unexpected sign-offs from developers who are not authors or co-authors or committers: Simon Horman Lines checked: 142, Warnings: 1, Errors: 0 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 ovn] northd: Don't skip the unSNAT stage for traffic towards VIPs.
On 3/5/24 15:56, Numan Siddique wrote:
> On Mon, Feb 26, 2024 at 7:59 AM Dumitru Ceara wrote:
>>
>> Otherwise, in case there's also a SNAT rule that uses the VIP as
>> external IP, we break sessions initiated from behind the VIP.
>>
>> This partially reverts 832893bdbb42 ("ovn-northd: Skip unsnat flows for
>> load balancer vips in router ingress pipeline"). That's OK because
>> commit 384a7c6237da ("northd: Refactor Logical Flows for routers with
>> DNAT/Load Balancers") addressed the original issue in a better way:
>>
>> In the reply direction, the order of traversal of the tables
>> "lr_in_defrag", "lr_in_unsnat" and "lr_in_dnat" adds incorrect
>> datapath flows that check ct_state in the wrong conntrack zone.
>> This is illustrated below where reply trafic enters the physical host
>> port (6) and traverses DNAT zone (14), SNAT zone (default), back to the
>> DNAT zone and then on to Logical Switch Port zone (22). The third
>> flow is incorrectly checking the state from the SNAT zone instead
>> of the DNAT zone.
>>
>> We also add a system test to ensure traffic initiated from behind a VIP
>> + SNAT is not broken.
>>
>> Another nice side effect is that the northd I-P is slightly simplified
>> because we don't need to track NAT external IPs anymore.
>>
>> Fixes: 832893bdbb42 ("ovn-northd: Skip unsnat flows for load balancer vips
>> in router ingress pipeline")
>> Reported-at: https://issues.redhat.com/browse/FDP-291
>> Signed-off-by: Dumitru Ceara
>
>
> Thanks for the fix. It also simplified the lr-nat-stateful code.
>
> Acked-by: Numan Siddique
>
Thanks, Numan!
Applied to main and backported to all branches down to 22.03.
Regards,
Dumitru
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH branch-2.17] conntrack: Fix flush not flushing all elements.
From: Xavier Simonart
On netdev datapath, when a ct element was cleaned, the cmap
could be shrinked, potentially causing some elements to be skipped
in the flush iteration.
Fixes: 967bb5c5cd90 ("conntrack: Add rcu support.")
Signed-off-by: Xavier Simonart
Acked-by: Mike Pattrick
Signed-off-by: Simon Horman
---
Changes in backport
* Resolved minor conflict in conntrack.c
* The test has been updated to avoid using ovs-ofctl compose-packet
--bare which does not exist until OVS v3.3. Instead frames are
constructed using printf.
* Updated test so a full tuple is passed to dpctl/flush-conntrack,
which means it needs to be wrapped in a for loop to cover all
entries that should be flushed.
---
lib/conntrack.c | 15 +++---
lib/conntrack.h | 2 +-
tests/system-traffic.at | 63 +
3 files changed, 68 insertions(+), 12 deletions(-)
diff --git a/lib/conntrack.c b/lib/conntrack.c
index a85e9ba8860b..af31f385d1be 100644
--- a/lib/conntrack.c
+++ b/lib/conntrack.c
@@ -2538,26 +2538,19 @@ conntrack_dump_start(struct conntrack *ct, struct
conntrack_dump *dump,
dump->ct = ct;
*ptot_bkts = 1; /* Need to clean up the callers. */
+dump->cursor = cmap_cursor_start(>conns);
return 0;
}
int
conntrack_dump_next(struct conntrack_dump *dump, struct ct_dpif_entry *entry)
{
-struct conntrack *ct = dump->ct;
long long now = time_msec();
-for (;;) {
-struct cmap_node *cm_node = cmap_next_position(>conns,
- >cm_pos);
-if (!cm_node) {
-break;
-}
-struct conn_key_node *keyn;
-struct conn *conn;
-
-INIT_CONTAINER(keyn, cm_node, cm_node);
+struct conn_key_node *keyn;
+struct conn *conn;
+CMAP_CURSOR_FOR_EACH_CONTINUE (keyn, cm_node, >cursor) {
if (keyn->dir != CT_DIR_FWD) {
continue;
}
diff --git a/lib/conntrack.h b/lib/conntrack.h
index 9553b188a410..7876e8d4387f 100644
--- a/lib/conntrack.h
+++ b/lib/conntrack.h
@@ -100,7 +100,7 @@ void conntrack_clear(struct dp_packet *packet);
struct conntrack_dump {
struct conntrack *ct;
unsigned bucket;
-struct cmap_position cm_pos;
+struct cmap_cursor cursor;
bool filter_zone;
uint16_t zone;
};
diff --git a/tests/system-traffic.at b/tests/system-traffic.at
index 099e9728128e..36f044416366 100644
--- a/tests/system-traffic.at
+++ b/tests/system-traffic.at
@@ -7369,6 +7369,69 @@ AT_CHECK([ovs-pcap client.pcap | grep
20102000], [0], [dnl
OVS_TRAFFIC_VSWITCHD_STOP
AT_CLEANUP
+AT_SETUP([conntrack - Flush many conntrack entries by port])
+CHECK_CONNTRACK()
+OVS_TRAFFIC_VSWITCHD_START()
+
+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_DATA([flows.txt], [dnl
+priority=100,in_port=1,udp,action=ct(zone=1,commit),2
+])
+
+AT_CHECK([ovs-ofctl --bundle add-flows br0 flows.txt])
+
+dnl 20 packets from port 1 and 1 packet from port 2.
+flow_l3="\
+eth_src=50:54:00:00:00:09,eth_dst=50:54:00:00:00:0a,dl_type=0x0800,\
+nw_src=10.1.1.1,nw_dst=10.1.1.2,nw_proto=17,nw_ttl=64,nw_frag=no"
+
+head="5054000a505400090800455c4011648d0a0101010a010102"
+len=72
+base_csum=1366
+tail="000102030405060708090a0b0c0d0e0f101112131415161718191a1b1c1d1e1f\
+ 202122232425262728292a2b2c2d2e2f303132333435363738393a3b3c3d3e3f"
+
+dst_port=1
+for src_port in $(seq 1 20); do
+csum=$((base_csum - src_port - dst_port))
+frame=$(printf "%s%04x%04x%04x%04x%s" $head 1 $src_port $len $csum $tail)
+AT_CHECK([ovs-ofctl -O OpenFlow13 packet-out br0 "in_port=1 packet=$frame
actions=resubmit(,0)"])
+done
+
+src_port=2
+dst_port=1
+csum=$((base_csum - src_port - dst_port))
+frame=$(printf "%s%04x%04x%04x%04x%s" $head $src_port $dst_port $len $csum
$tail)
+AT_CHECK([ovs-ofctl -O OpenFlow13 packet-out br0 "in_port=1 packet=$frame
actions=resubmit(,0)"])
+
+: > conntrack
+
+for i in $(seq 1 20); do
+echo
"udp,orig=(src=10.1.1.1,dst=10.1.1.2,sport=1,dport=${i}),reply=(src=10.1.1.2,dst=10.1.1.1,sport=${i},dport=1),zone=1"
>> conntrack
+done
+echo
"udp,orig=(src=10.1.1.1,dst=10.1.1.2,sport=2,dport=1),reply=(src=10.1.1.2,dst=10.1.1.1,sport=1,dport=2),zone=1"
>> conntrack
+
+sort conntrack > expout
+
+AT_CHECK([ovs-appctl dpctl/dump-conntrack zone=1 | grep -F "src=10.1.1.1," |
sort ], [0], [expout])
+
+dnl Check that flushing conntrack by port 1 flush all ct for port 1 but keeps
ct for port 2.
+for i in $(seq 1 20); do
+AT_CHECK([ovs-appctl dpctl/flush-conntrack zone=1
"ct_nw_src=10.1.1.1,ct_nw_dst=10.1.1.2,ct_nw_proto=17,ct_tp_src=1,ct_tp_dst=$i"])
+done
+AT_CHECK([ovs-appctl dpctl/dump-conntrack zone=1 | grep -F "src=10.1.1.1," |
sort ], [0], [dnl
+udp,orig=(src=10.1.1.1,dst=10.1.1.2,sport=2,dport=1),reply=(src=10.1.1.2,dst=10.1.1.1,sport=1,dport=2),zone=1
+])
+
+OVS_TRAFFIC_VSWITCHD_STOP
+AT_CLEANUP
[ovs-dev] [PATCH branch-3.0] conntrack: Fix flush not flushing all elements.
From: Xavier Simonart
On netdev datapath, when a ct element was cleaned, the cmap
could be shrinked, potentially causing some elements to be skipped
in the flush iteration.
Fixes: 967bb5c5cd90 ("conntrack: Add rcu support.")
Signed-off-by: Xavier Simonart
Acked-by: Mike Pattrick
Signed-off-by: Simon Horman
---
The test has been updated to avoid using ovs-ofctl compose-packet --bare
which does not exist until OVS v3.3. Instead frames are constructed
using printf.
---
lib/conntrack.c | 14 +++---
lib/conntrack.h | 2 +-
tests/system-traffic.at | 61 +
3 files changed, 66 insertions(+), 11 deletions(-)
diff --git a/lib/conntrack.c b/lib/conntrack.c
index cea3e451d3c7..aaa94b297b5e 100644
--- a/lib/conntrack.c
+++ b/lib/conntrack.c
@@ -2530,25 +2530,19 @@ conntrack_dump_start(struct conntrack *ct, struct
conntrack_dump *dump,
dump->ct = ct;
*ptot_bkts = 1; /* Need to clean up the callers. */
+dump->cursor = cmap_cursor_start(>conns);
return 0;
}
int
conntrack_dump_next(struct conntrack_dump *dump, struct ct_dpif_entry *entry)
{
-struct conntrack *ct = dump->ct;
long long now = time_msec();
-for (;;) {
-struct cmap_node *cm_node = cmap_next_position(>conns,
- >cm_pos);
-if (!cm_node) {
-break;
-}
-struct conn_key_node *keyn;
-struct conn *conn;
+struct conn_key_node *keyn;
+struct conn *conn;
-INIT_CONTAINER(keyn, cm_node, cm_node);
+CMAP_CURSOR_FOR_EACH_CONTINUE (keyn, cm_node, >cursor) {
if (keyn->dir != CT_DIR_FWD) {
continue;
}
diff --git a/lib/conntrack.h b/lib/conntrack.h
index b064abc9fa43..bed23a9515a2 100644
--- a/lib/conntrack.h
+++ b/lib/conntrack.h
@@ -100,7 +100,7 @@ void conntrack_clear(struct dp_packet *packet);
struct conntrack_dump {
struct conntrack *ct;
unsigned bucket;
-struct cmap_position cm_pos;
+struct cmap_cursor cursor;
bool filter_zone;
uint16_t zone;
};
diff --git a/tests/system-traffic.at b/tests/system-traffic.at
index a06afed98cc1..6eb9f603f1e8 100644
--- a/tests/system-traffic.at
+++ b/tests/system-traffic.at
@@ -7324,6 +7324,67 @@ AT_CHECK([ovs-pcap client.pcap | grep
20102000], [0], [dnl
OVS_TRAFFIC_VSWITCHD_STOP
AT_CLEANUP
+AT_SETUP([conntrack - Flush many conntrack entries by port])
+CHECK_CONNTRACK()
+OVS_TRAFFIC_VSWITCHD_START()
+
+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_DATA([flows.txt], [dnl
+priority=100,in_port=1,udp,action=ct(zone=1,commit),2
+])
+
+AT_CHECK([ovs-ofctl --bundle add-flows br0 flows.txt])
+
+dnl 20 packets from port 1 and 1 packet from port 2.
+flow_l3="\
+eth_src=50:54:00:00:00:09,eth_dst=50:54:00:00:00:0a,dl_type=0x0800,\
+nw_src=10.1.1.1,nw_dst=10.1.1.2,nw_proto=17,nw_ttl=64,nw_frag=no"
+
+head="5054000a505400090800455c4011648d0a0101010a010102"
+len=72
+base_csum=1366
+tail="000102030405060708090a0b0c0d0e0f101112131415161718191a1b1c1d1e1f\
+ 202122232425262728292a2b2c2d2e2f303132333435363738393a3b3c3d3e3f"
+
+dst_port=1
+for src_port in $(seq 1 20); do
+csum=$((base_csum - src_port - dst_port))
+frame=$(printf "%s%04x%04x%04x%04x%s" $head 1 $src_port $len $csum $tail)
+AT_CHECK([ovs-ofctl -O OpenFlow13 packet-out br0 "in_port=1 packet=$frame
actions=resubmit(,0)"])
+done
+
+src_port=2
+dst_port=1
+csum=$((base_csum - src_port - dst_port))
+frame=$(printf "%s%04x%04x%04x%04x%s" $head $src_port $dst_port $len $csum
$tail)
+AT_CHECK([ovs-ofctl -O OpenFlow13 packet-out br0 "in_port=1 packet=$frame
actions=resubmit(,0)"])
+
+: > conntrack
+
+for i in $(seq 1 20); do
+echo
"udp,orig=(src=10.1.1.1,dst=10.1.1.2,sport=1,dport=${i}),reply=(src=10.1.1.2,dst=10.1.1.1,sport=${i},dport=1),zone=1"
>> conntrack
+done
+echo
"udp,orig=(src=10.1.1.1,dst=10.1.1.2,sport=2,dport=1),reply=(src=10.1.1.2,dst=10.1.1.1,sport=1,dport=2),zone=1"
>> conntrack
+
+sort conntrack > expout
+
+AT_CHECK([ovs-appctl dpctl/dump-conntrack zone=1 | grep -F "src=10.1.1.1," |
sort ], [0], [expout])
+
+dnl Check that flushing conntrack by port 1 flush all ct for port 1 but keeps
ct for port 2.
+for i in $(seq 1 20); do
+AT_CHECK([ovs-appctl dpctl/flush-conntrack zone=1
"ct_nw_src=10.1.1.1,ct_nw_dst=10.1.1.2,ct_nw_proto=17,ct_tp_src=1,ct_tp_dst=$i"])
+done
+AT_CHECK([ovs-appctl dpctl/dump-conntrack zone=1 | grep -F "src=10.1.1.1," |
sort ], [0], [dnl
+udp,orig=(src=10.1.1.1,dst=10.1.1.2,sport=2,dport=1),reply=(src=10.1.1.2,dst=10.1.1.1,sport=1,dport=2),zone=1
+])
+
+OVS_TRAFFIC_VSWITCHD_STOP
+AT_CLEANUP
+
AT_BANNER([IGMP])
AT_SETUP([IGMP - flood under normal action])
--
2.43.0
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH branch-3.1] conntrack: Fix flush not flushing all elements.
+ Xavier Simonart
Mike Pattrick
On Wed, Mar 06, 2024 at 06:21:36PM +, 'Simon Horman' wrote:
> From: Xavier Simonart
>
> On netdev datapath, when a ct element was cleaned, the cmap
> could be shrinked, potentially causing some elements to be skipped
> in the flush iteration.
>
> Fixes: 967bb5c5cd90 ("conntrack: Add rcu support.")
> Signed-off-by: Xavier Simonart
> Acked-by: Mike Pattrick
> Signed-off-by: Simon Horman
> ---
> The test has been updated to avoid using ovs-ofctl compose-packet --bare
> which does not exist until OVS v3.3. Instead frames are constructed
> using printf.
Adding missing CCs.
It seems that b4 is not working well for me today :(
> ---
> lib/conntrack.c | 14
> lib/conntrack.h | 2 +-
> tests/system-traffic.at | 59
> +
> 3 files changed, 64 insertions(+), 11 deletions(-)
>
> diff --git a/lib/conntrack.c b/lib/conntrack.c
> index 2470c16895c2..38b788747461 100644
> --- a/lib/conntrack.c
> +++ b/lib/conntrack.c
> @@ -2534,25 +2534,19 @@ conntrack_dump_start(struct conntrack *ct, struct
> conntrack_dump *dump,
>
> dump->ct = ct;
> *ptot_bkts = 1; /* Need to clean up the callers. */
> +dump->cursor = cmap_cursor_start(>conns);
> return 0;
> }
>
> int
> conntrack_dump_next(struct conntrack_dump *dump, struct ct_dpif_entry *entry)
> {
> -struct conntrack *ct = dump->ct;
> long long now = time_msec();
>
> -for (;;) {
> -struct cmap_node *cm_node = cmap_next_position(>conns,
> - >cm_pos);
> -if (!cm_node) {
> -break;
> -}
> -struct conn_key_node *keyn;
> -struct conn *conn;
> +struct conn_key_node *keyn;
> +struct conn *conn;
>
> -INIT_CONTAINER(keyn, cm_node, cm_node);
> +CMAP_CURSOR_FOR_EACH_CONTINUE (keyn, cm_node, >cursor) {
> if (keyn->dir != CT_DIR_FWD) {
> continue;
> }
> diff --git a/lib/conntrack.h b/lib/conntrack.h
> index b064abc9fa43..bed23a9515a2 100644
> --- a/lib/conntrack.h
> +++ b/lib/conntrack.h
> @@ -100,7 +100,7 @@ void conntrack_clear(struct dp_packet *packet);
> struct conntrack_dump {
> struct conntrack *ct;
> unsigned bucket;
> -struct cmap_position cm_pos;
> +struct cmap_cursor cursor;
> bool filter_zone;
> uint16_t zone;
> };
> diff --git a/tests/system-traffic.at b/tests/system-traffic.at
> index 5d12fd41b4f7..c99c637eb1fd 100644
> --- a/tests/system-traffic.at
> +++ b/tests/system-traffic.at
> @@ -7488,6 +7488,65 @@ AT_CHECK([ovs-pcap client.pcap | grep
> 20102000], [0], [dnl
> OVS_TRAFFIC_VSWITCHD_STOP
> AT_CLEANUP
>
> +AT_SETUP([conntrack - Flush many conntrack entries by port])
> +CHECK_CONNTRACK()
> +OVS_TRAFFIC_VSWITCHD_START()
> +
> +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_DATA([flows.txt], [dnl
> +priority=100,in_port=1,udp,action=ct(zone=1,commit),2
> +])
> +
> +AT_CHECK([ovs-ofctl --bundle add-flows br0 flows.txt])
> +
> +dnl 20 packets from port 1 and 1 packet from port 2.
> +flow_l3="\
> +eth_src=50:54:00:00:00:09,eth_dst=50:54:00:00:00:0a,dl_type=0x0800,\
> +nw_src=10.1.1.1,nw_dst=10.1.1.2,nw_proto=17,nw_ttl=64,nw_frag=no"
> +
> +head="5054000a505400090800455c4011648d0a0101010a010102"
> +len=72
> +base_csum=1366
> +tail="000102030405060708090a0b0c0d0e0f101112131415161718191a1b1c1d1e1f\
> + 202122232425262728292a2b2c2d2e2f303132333435363738393a3b3c3d3e3f"
> +
> +dst_port=1
> +for src_port in $(seq 1 20); do
> +csum=$((base_csum - src_port - dst_port))
> +frame=$(printf "%s%04x%04x%04x%04x%s" $head 1 $src_port $len $csum $tail)
> +AT_CHECK([ovs-ofctl -O OpenFlow13 packet-out br0 "in_port=1
> packet=$frame actions=resubmit(,0)"])
> +done
> +
> +src_port=2
> +dst_port=1
> +csum=$((base_csum - src_port - dst_port))
> +frame=$(printf "%s%04x%04x%04x%04x%s" $head $src_port $dst_port $len $csum
> $tail)
> +AT_CHECK([ovs-ofctl -O OpenFlow13 packet-out br0 "in_port=1 packet=$frame
> actions=resubmit(,0)"])
> +
> +: > conntrack
> +
> +for i in $(seq 1 20); do
> +echo
> "udp,orig=(src=10.1.1.1,dst=10.1.1.2,sport=1,dport=${i}),reply=(src=10.1.1.2,dst=10.1.1.1,sport=${i},dport=1),zone=1"
> >> conntrack
> +done
> +echo
> "udp,orig=(src=10.1.1.1,dst=10.1.1.2,sport=2,dport=1),reply=(src=10.1.1.2,dst=10.1.1.1,sport=1,dport=2),zone=1"
> >> conntrack
> +
> +sort conntrack > expout
> +
> +AT_CHECK([ovs-appctl dpctl/dump-conntrack zone=1 | grep -F "src=10.1.1.1," |
> sort ], [0], [expout])
> +
> +dnl Check that flushing conntrack by port 1 flush all ct for port 1 but
> keeps ct for port 2.
> +AT_CHECK([ovs-appctl dpctl/flush-conntrack zone=1
> 'ct_nw_proto=17,ct_tp_src=1'])
> +AT_CHECK([ovs-appctl dpctl/dump-conntrack zone=1 | grep -F "src=10.1.1.1," |
> sort
Re: [ovs-dev] [PATCH v8 3/4] dpif: Probe support for OVS_ACTION_ATTR_DROP.
On Wed, Mar 06, 2024 at 12:34:07PM -0500, Eric Garver wrote:
> Kernel support has been added for this action. As such, we need to probe
> the datapath for support.
>
> Signed-off-by: Eric Garver
> ---
> include/linux/openvswitch.h | 2 +-
> lib/dpif.c | 6 ---
> lib/dpif.h | 1 -
> ofproto/ofproto-dpif.c | 89 +++--
> ofproto/ofproto-dpif.h | 4 +-
> 5 files changed, 89 insertions(+), 13 deletions(-)
>
> diff --git a/include/linux/openvswitch.h b/include/linux/openvswitch.h
> index a265e05ad253..fce6456f47c8 100644
> --- a/include/linux/openvswitch.h
> +++ b/include/linux/openvswitch.h
> @@ -1086,11 +1086,11 @@ enum ovs_action_attr {
> OVS_ACTION_ATTR_CHECK_PKT_LEN, /* Nested OVS_CHECK_PKT_LEN_ATTR_*. */
> OVS_ACTION_ATTR_ADD_MPLS, /* struct ovs_action_add_mpls. */
> OVS_ACTION_ATTR_DEC_TTL, /* Nested OVS_DEC_TTL_ATTR_*. */
> + OVS_ACTION_ATTR_DROP, /* u32 xlate_error. */
>
> #ifndef __KERNEL__
> OVS_ACTION_ATTR_TUNNEL_PUSH, /* struct ovs_action_push_tnl*/
> OVS_ACTION_ATTR_TUNNEL_POP,/* u32 port number. */
> - OVS_ACTION_ATTR_DROP, /* u32 xlate_error. */
> OVS_ACTION_ATTR_LB_OUTPUT, /* u32 bond-id. */
> #endif
> __OVS_ACTION_ATTR_MAX,/* Nothing past this will be accepted
> diff --git a/lib/dpif.c b/lib/dpif.c
> index 0f480bec48d0..28fa40879655 100644
> --- a/lib/dpif.c
> +++ b/lib/dpif.c
> @@ -1934,12 +1934,6 @@ dpif_supports_tnl_push_pop(const struct dpif *dpif)
> return dpif_is_netdev(dpif);
> }
>
> -bool
> -dpif_supports_explicit_drop_action(const struct dpif *dpif)
> -{
> -return dpif_is_netdev(dpif);
> -}
> -
> bool
> dpif_supports_lb_output_action(const struct dpif *dpif)
> {
> diff --git a/lib/dpif.h b/lib/dpif.h
> index 0f2dc2ef3c55..8dc5da45b3ef 100644
> --- a/lib/dpif.h
> +++ b/lib/dpif.h
> @@ -940,7 +940,6 @@ int dpif_get_pmds_for_port(const struct dpif * dpif,
> odp_port_t port_no,
>
> char *dpif_get_dp_version(const struct dpif *);
> bool dpif_supports_tnl_push_pop(const struct dpif *);
> -bool dpif_supports_explicit_drop_action(const struct dpif *);
> bool dpif_synced_dp_layers(struct dpif *);
>
> /* Log functions. */
> diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
> index b1978e3f2fd5..da83d6196b13 100644
> --- a/ofproto/ofproto-dpif.c
> +++ b/ofproto/ofproto-dpif.c
> @@ -72,6 +72,8 @@
> #include "util.h"
> #include "uuid.h"
> #include "vlan-bitmap.h"
> +#include "dpif-netdev.h"
> +#include "netdev-offload.h"
>
> VLOG_DEFINE_THIS_MODULE(ofproto_dpif);
>
> @@ -221,6 +223,8 @@ static void ct_zone_config_init(struct dpif_backer
> *backer);
> static void ct_zone_config_uninit(struct dpif_backer *backer);
> static void ct_zone_timeout_policy_sweep(struct dpif_backer *backer);
> static void ct_zone_limits_commit(struct dpif_backer *backer);
> +static bool check_drop_action(struct dpif_backer *backer);
> +static bool recheck_support_explicit_drop_action(struct dpif_backer *backer);
>
> static inline struct ofproto_dpif *
> ofproto_dpif_cast(const struct ofproto *ofproto)
> @@ -391,6 +395,10 @@ type_run(const char *type)
> udpif_set_threads(backer->udpif, n_handlers, n_revalidators);
> }
>
> +if (recheck_support_explicit_drop_action(backer)) {
> +backer->need_revalidate = REV_RECONFIGURE;
> +}
> +
> if (backer->need_revalidate) {
> struct ofproto_dpif *ofproto;
> struct simap_node *node;
> @@ -807,6 +815,7 @@ open_dpif_backer(const char *type, struct dpif_backer
> **backerp)
>
> shash_add(_dpif_backers, type, backer);
>
> +atomic_init(>rt_support.explicit_drop_action, false);
> check_support(backer);
> atomic_count_init(>tnl_count, 0);
>
> @@ -855,7 +864,10 @@ ovs_native_tunneling_is_on(struct ofproto_dpif *ofproto)
> bool
> ovs_explicit_drop_action_supported(struct ofproto_dpif *ofproto)
> {
> -return ofproto->backer->rt_support.explicit_drop_action;
> +bool value;
> +atomic_read_relaxed(>backer->rt_support.explicit_drop_action,
> +);
> +return value;
> }
>
> bool
> @@ -1379,6 +1391,43 @@ check_ct_timeout_policy(struct dpif_backer *backer)
> return !error;
> }
>
> +/* Tests whether backer's datapath supports the OVS_ACTION_ATTR_DROP action.
> */
> +static bool
> +check_drop_action(struct dpif_backer *backer)
> +{
> +struct odputil_keybuf keybuf;
> +uint8_t actbuf[NL_A_U32_SIZE];
> +struct ofpbuf actions;
> +struct ofpbuf key;
> +struct flow flow;
> +bool supported;
> +bool hw_offload;
> +
> +struct odp_flow_key_parms odp_parms = {
> +.flow = ,
> +.probe = true,
> +};
> +
> +memset(, 0, sizeof flow);
> +ofpbuf_use_stack(, , sizeof keybuf);
> +odp_flow_key_from_flow(_parms, );
> +
> +ofpbuf_use_stack(, , sizeof actbuf);
> +nl_msg_put_u32(, OVS_ACTION_ATTR_DROP,
Re: [ovs-dev] [PATCH branch-3.1] conntrack: Fix flush not flushing all elements.
Bleep bloop. Greetings 'Simon Horman', 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: WARNING: Unexpected sign-offs from developers who are not authors or co-authors or committers: Simon Horman Lines checked: 140, Warnings: 1, Errors: 0 Please check this out. If you feel there has been an error, please email acon...@redhat.com Thanks, 0-day Robot ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH branch-3.1] conntrack: Fix flush not flushing all elements.
From: Xavier Simonart
On netdev datapath, when a ct element was cleaned, the cmap
could be shrinked, potentially causing some elements to be skipped
in the flush iteration.
Fixes: 967bb5c5cd90 ("conntrack: Add rcu support.")
Signed-off-by: Xavier Simonart
Acked-by: Mike Pattrick
Signed-off-by: Simon Horman
---
The test has been updated to avoid using ovs-ofctl compose-packet --bare
which does not exist until OVS v3.3. Instead frames are constructed
using printf.
---
lib/conntrack.c | 14
lib/conntrack.h | 2 +-
tests/system-traffic.at | 59 +
3 files changed, 64 insertions(+), 11 deletions(-)
diff --git a/lib/conntrack.c b/lib/conntrack.c
index 2470c16895c2..38b788747461 100644
--- a/lib/conntrack.c
+++ b/lib/conntrack.c
@@ -2534,25 +2534,19 @@ conntrack_dump_start(struct conntrack *ct, struct
conntrack_dump *dump,
dump->ct = ct;
*ptot_bkts = 1; /* Need to clean up the callers. */
+dump->cursor = cmap_cursor_start(>conns);
return 0;
}
int
conntrack_dump_next(struct conntrack_dump *dump, struct ct_dpif_entry *entry)
{
-struct conntrack *ct = dump->ct;
long long now = time_msec();
-for (;;) {
-struct cmap_node *cm_node = cmap_next_position(>conns,
- >cm_pos);
-if (!cm_node) {
-break;
-}
-struct conn_key_node *keyn;
-struct conn *conn;
+struct conn_key_node *keyn;
+struct conn *conn;
-INIT_CONTAINER(keyn, cm_node, cm_node);
+CMAP_CURSOR_FOR_EACH_CONTINUE (keyn, cm_node, >cursor) {
if (keyn->dir != CT_DIR_FWD) {
continue;
}
diff --git a/lib/conntrack.h b/lib/conntrack.h
index b064abc9fa43..bed23a9515a2 100644
--- a/lib/conntrack.h
+++ b/lib/conntrack.h
@@ -100,7 +100,7 @@ void conntrack_clear(struct dp_packet *packet);
struct conntrack_dump {
struct conntrack *ct;
unsigned bucket;
-struct cmap_position cm_pos;
+struct cmap_cursor cursor;
bool filter_zone;
uint16_t zone;
};
diff --git a/tests/system-traffic.at b/tests/system-traffic.at
index 5d12fd41b4f7..c99c637eb1fd 100644
--- a/tests/system-traffic.at
+++ b/tests/system-traffic.at
@@ -7488,6 +7488,65 @@ AT_CHECK([ovs-pcap client.pcap | grep
20102000], [0], [dnl
OVS_TRAFFIC_VSWITCHD_STOP
AT_CLEANUP
+AT_SETUP([conntrack - Flush many conntrack entries by port])
+CHECK_CONNTRACK()
+OVS_TRAFFIC_VSWITCHD_START()
+
+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_DATA([flows.txt], [dnl
+priority=100,in_port=1,udp,action=ct(zone=1,commit),2
+])
+
+AT_CHECK([ovs-ofctl --bundle add-flows br0 flows.txt])
+
+dnl 20 packets from port 1 and 1 packet from port 2.
+flow_l3="\
+eth_src=50:54:00:00:00:09,eth_dst=50:54:00:00:00:0a,dl_type=0x0800,\
+nw_src=10.1.1.1,nw_dst=10.1.1.2,nw_proto=17,nw_ttl=64,nw_frag=no"
+
+head="5054000a505400090800455c4011648d0a0101010a010102"
+len=72
+base_csum=1366
+tail="000102030405060708090a0b0c0d0e0f101112131415161718191a1b1c1d1e1f\
+ 202122232425262728292a2b2c2d2e2f303132333435363738393a3b3c3d3e3f"
+
+dst_port=1
+for src_port in $(seq 1 20); do
+csum=$((base_csum - src_port - dst_port))
+frame=$(printf "%s%04x%04x%04x%04x%s" $head 1 $src_port $len $csum $tail)
+AT_CHECK([ovs-ofctl -O OpenFlow13 packet-out br0 "in_port=1 packet=$frame
actions=resubmit(,0)"])
+done
+
+src_port=2
+dst_port=1
+csum=$((base_csum - src_port - dst_port))
+frame=$(printf "%s%04x%04x%04x%04x%s" $head $src_port $dst_port $len $csum
$tail)
+AT_CHECK([ovs-ofctl -O OpenFlow13 packet-out br0 "in_port=1 packet=$frame
actions=resubmit(,0)"])
+
+: > conntrack
+
+for i in $(seq 1 20); do
+echo
"udp,orig=(src=10.1.1.1,dst=10.1.1.2,sport=1,dport=${i}),reply=(src=10.1.1.2,dst=10.1.1.1,sport=${i},dport=1),zone=1"
>> conntrack
+done
+echo
"udp,orig=(src=10.1.1.1,dst=10.1.1.2,sport=2,dport=1),reply=(src=10.1.1.2,dst=10.1.1.1,sport=1,dport=2),zone=1"
>> conntrack
+
+sort conntrack > expout
+
+AT_CHECK([ovs-appctl dpctl/dump-conntrack zone=1 | grep -F "src=10.1.1.1," |
sort ], [0], [expout])
+
+dnl Check that flushing conntrack by port 1 flush all ct for port 1 but keeps
ct for port 2.
+AT_CHECK([ovs-appctl dpctl/flush-conntrack zone=1
'ct_nw_proto=17,ct_tp_src=1'])
+AT_CHECK([ovs-appctl dpctl/dump-conntrack zone=1 | grep -F "src=10.1.1.1," |
sort ], [0], [dnl
+udp,orig=(src=10.1.1.1,dst=10.1.1.2,sport=2,dport=1),reply=(src=10.1.1.2,dst=10.1.1.1,sport=1,dport=2),zone=1
+])
+
+OVS_TRAFFIC_VSWITCHD_STOP
+AT_CLEANUP
+
AT_BANNER([IGMP])
AT_SETUP([IGMP - flood under normal action])
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH branch-3.2 2/2] conntrack: Fix flush not flushing all elements.
Bleep bloop. Greetings 'Simon Horman', 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: WARNING: Unexpected sign-offs from developers who are not authors or co-authors or committers: Simon Horman Lines checked: 141, Warnings: 1, Errors: 0 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 branch-3.2 0/2] conntrack: Fix flush not flushing all elements.
+ Aaron Conole Frode Nordahl Mike Pattrick Paolo Valerio Xavier Simonart On Wed, Mar 06, 2024 at 06:11:08PM +, 'Simon Horman' wrote: > Backport to branch-3.20 of: s/branch-3.20/branch-3.2/ > - conntrack: Fix flush not flushing all elements. > https://github.com/openvswitch/ovs/commit/6c082a8310d5 > > Including dependency which is not present in branch-3.2. Sorry, something when slightly wrong with the metadata for this post. Adding missing CCs and fixing obviously incorrect branch name. Series can be found here: - https://patchwork.ozlabs.org/project/openvswitch/list/?series=398064=%2A=both > > --- > Peng He (1): > conntrack: Remove nat_conn introducing key directionality. > > Xavier Simonart (1): > conntrack: Fix flush not flushing all elements. > > lib/conntrack-private.h | 19 +-- > lib/conntrack-tp.c | 6 +- > lib/conntrack.c | 374 > +++- > lib/conntrack.h | 2 +- > tests/system-traffic.at | 59 > 5 files changed, 225 insertions(+), 235 deletions(-) > > base-commit: b5f2ed2a21714c45ba6a636d2468c1d0b293613b > > ___ > dev mailing list > d...@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH branch-3.2 1/2] conntrack: Remove nat_conn introducing key directionality.
Bleep bloop. Greetings 'Simon Horman', 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: WARNING: Unexpected sign-offs from developers who are not authors or co-authors or committers: Aaron Conole , Simon Horman Lines checked: 910, Warnings: 1, Errors: 0 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 v3] conntrack: Remove nat_conn introducing key directionality.
Simon Horman writes: > + Xavier > > On Thu, Aug 31, 2023 at 02:52:59PM -0400, Aaron Conole wrote: >> Ilya Maximets writes: >> >> > On 8/31/23 09:15, Frode Nordahl wrote: >> >> On Wed, Aug 30, 2023 at 9:30 PM Paolo Valerio wrote: >> >>> >> >>> From: hepeng >> >>> >> >>> The patch avoids the extra allocation for nat_conn. >> >>> Currently, when doing NAT, the userspace conntrack will use an extra >> >>> conn for the two directions in a flow. However, each conn has actually >> >>> the two keys for both orig and rev directions. This patch introduces a >> >>> key_node[CT_DIRS] member as per Aaron's suggestion in the conn which >> >>> consists of a key, direction, and a cmap_node for hash lookup so >> >>> addressing the feedback received by the original patch [0]. >> >>> >> >>> [0] >> >>> https://patchwork.ozlabs.org/project/openvswitch/patch/20201129033255.64647-2-hepeng.0...@bytedance.com/ >> >>> >> >>> Signed-off-by: Peng He >> >>> Co-authored-by: Paolo Valerio >> >>> Signed-off-by: Paolo Valerio >> >> >> >> Thanks alot for working on this, should we perhaps reference the >> >> original bug report, i.e: >> >> Reported-by: Michael Plato >> >> Reported-at: >> >> https://mail.openvswitch.org/pipermail/ovs-discuss/2022-September/052065.html >> > >> > Can be added while applying, I think. It also may be worth adding >> > a sentence about fixing the assertion to the commit message. >> >> Done. >> >> >> >> >> We have a reproducer for the issue and we no longer see it occurring >> >> with this patch. >> >> Tested-by: Frode Nordahl >> > >> > Thanks! >> >> Thanks everyone! I've applied and backported down to branch-3.0, and >> will work on the backport to branch-2.17. > > Hi Aaron, > > while working on [1] I notice that this patch did not seem to be > backported to branch-3.2. I will plan on doing so as part of > my backports of [1]. > > [1] [ovs-dev,v3] conntrack: Fix flush not flushing all elements. > > https://patchwork.ozlabs.org/project/openvswitch/patch/20240304152159.1710977-1-xsimo...@redhat.com/ Strange - I would have thought I had applied it. Glad to see this get resolved. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH branch-3.2 1/2] conntrack: Remove nat_conn introducing key directionality.
From: Peng He
The patch avoids the extra allocation for nat_conn.
Currently, when doing NAT, the userspace conntrack will use an extra
conn for the two directions in a flow. However, each conn has actually
the two keys for both orig and rev directions. This patch introduces a
key_node[CT_DIRS] member as per Aaron's suggestion in the conn which
consists of a key, direction, and a cmap_node for hash lookup so
addressing the feedback received by the original patch [0].
With this adjustment, we also remove the assertion that connections in
the table are DEFAULT while updating connection state and/or removing
connections.
[0]
https://patchwork.ozlabs.org/project/openvswitch/patch/20201129033255.64647-2-hepeng.0...@bytedance.com/
Reported-by: Michael Plato
Reported-at:
https://mail.openvswitch.org/pipermail/ovs-discuss/2022-September/052065.html
Signed-off-by: Peng He
Co-authored-by: Paolo Valerio
Signed-off-by: Paolo Valerio
Tested-by: Frode Nordahl
Acked-by: Ilya Maximets
Acked-by: Aaron Conole
Signed-off-by: Aaron Conole
Signed-off-by: Simon Horman
---
This patch is included in this series as a dependency as although it is
present in branches for older releases it is not present in branch-3.2.
I assume this is an oversight [1].
[1]
https://patchwork.ozlabs.org/project/openvswitch/patch/169342379158.466383.12493978742760368032.stgit@rawp/#3277395
---
lib/conntrack-private.h | 19 +--
lib/conntrack-tp.c | 6 +-
lib/conntrack.c | 366
3 files changed, 164 insertions(+), 227 deletions(-)
diff --git a/lib/conntrack-private.h b/lib/conntrack-private.h
index bb326868e9dd..3fd5fccd3eb3 100644
--- a/lib/conntrack-private.h
+++ b/lib/conntrack-private.h
@@ -49,6 +49,12 @@ struct ct_endpoint {
* hashing in ct_endpoint_hash_add(). */
BUILD_ASSERT_DECL(sizeof(struct ct_endpoint) == sizeof(union ct_addr) + 4);
+enum key_dir {
+CT_DIR_FWD = 0,
+CT_DIR_REV,
+CT_DIRS,
+};
+
/* Changes to this structure need to be reflected in conn_key_hash()
* and conn_key_cmp(). */
struct conn_key {
@@ -112,20 +118,18 @@ enum ct_timeout {
#define N_EXP_LISTS 100
-enum OVS_PACKED_ENUM ct_conn_type {
-CT_CONN_TYPE_DEFAULT,
-CT_CONN_TYPE_UN_NAT,
+struct conn_key_node {
+enum key_dir dir;
+struct conn_key key;
+struct cmap_node cm_node;
};
struct conn {
/* Immutable data. */
-struct conn_key key;
-struct conn_key rev_key;
+struct conn_key_node key_node[CT_DIRS];
struct conn_key parent_key; /* Only used for orig_tuple support. */
-struct cmap_node cm_node;
uint16_t nat_action;
char *alg;
-struct conn *nat_conn; /* The NAT 'conn' context, if there is one. */
atomic_flag reclaimed; /* False during the lifetime of the connection,
* True as soon as a thread has started freeing
* its memory. */
@@ -150,7 +154,6 @@ struct conn {
/* Immutable data. */
bool alg_related; /* True if alg data connection. */
-enum ct_conn_type conn_type;
uint32_t tp_id; /* Timeout policy ID. */
};
diff --git a/lib/conntrack-tp.c b/lib/conntrack-tp.c
index 89cb2704a6c7..2149fdc73a7c 100644
--- a/lib/conntrack-tp.c
+++ b/lib/conntrack-tp.c
@@ -253,7 +253,8 @@ conn_update_expiration(struct conntrack *ct, struct conn
*conn,
}
VLOG_DBG_RL(, "Update timeout %s zone=%u with policy id=%d "
"val=%u sec.",
-ct_timeout_str[tm], conn->key.zone, conn->tp_id, val);
+ct_timeout_str[tm], conn->key_node[CT_DIR_FWD].key.zone,
+conn->tp_id, val);
atomic_store_relaxed(>expiration, now + val * 1000);
}
@@ -273,7 +274,8 @@ conn_init_expiration(struct conntrack *ct, struct conn
*conn,
}
VLOG_DBG_RL(, "Init timeout %s zone=%u with policy id=%d val=%u sec.",
-ct_timeout_str[tm], conn->key.zone, conn->tp_id, val);
+ct_timeout_str[tm], conn->key_node[CT_DIR_FWD].key.zone,
+conn->tp_id, val);
conn->expiration = now + val * 1000;
}
diff --git a/lib/conntrack.c b/lib/conntrack.c
index 5f1176d333f9..47a443fba4db 100644
--- a/lib/conntrack.c
+++ b/lib/conntrack.c
@@ -103,7 +103,7 @@ static enum ct_update_res conn_update(struct conntrack *ct,
struct conn *conn,
struct conn_lookup_ctx *ctx,
long long now);
static long long int conn_expiration(const struct conn *);
-static bool conn_expired(struct conn *, long long now);
+static bool conn_expired(const struct conn *, long long now);
static void conn_expire_push_front(struct conntrack *ct, struct conn *conn);
static void set_mark(struct dp_packet *, struct conn *,
uint32_t val, uint32_t mask);
@@ -113,8 +113,7 @@ static void set_label(struct dp_packet *, struct conn *,
static void *clean_thread_main(void *f_);
static bool
[ovs-dev] [PATCH branch-3.2 0/2] conntrack: Fix flush not flushing all elements.
Backport to branch-3.20 of: - conntrack: Fix flush not flushing all elements. https://github.com/openvswitch/ovs/commit/6c082a8310d5 Including dependency which is not present in branch-3.2. --- Peng He (1): conntrack: Remove nat_conn introducing key directionality. Xavier Simonart (1): conntrack: Fix flush not flushing all elements. lib/conntrack-private.h | 19 +-- lib/conntrack-tp.c | 6 +- lib/conntrack.c | 374 +++- lib/conntrack.h | 2 +- tests/system-traffic.at | 59 5 files changed, 225 insertions(+), 235 deletions(-) base-commit: b5f2ed2a21714c45ba6a636d2468c1d0b293613b ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH branch-3.2 2/2] conntrack: Fix flush not flushing all elements.
From: Xavier Simonart
On netdev datapath, when a ct element was cleaned, the cmap
could be shrinked, potentially causing some elements to be skipped
in the flush iteration.
Fixes: 967bb5c5cd90 ("conntrack: Add rcu support.")
Signed-off-by: Xavier Simonart
Acked-by: Mike Pattrick
Signed-off-by: Simon Horman
---
The test has been updated to avoid using ovs-ofctl compose-packet --bare
which does not exist until OVS v3.3. Instead frames are constructed
using printf.
---
lib/conntrack.c | 14
lib/conntrack.h | 2 +-
tests/system-traffic.at | 59 +
3 files changed, 64 insertions(+), 11 deletions(-)
diff --git a/lib/conntrack.c b/lib/conntrack.c
index 47a443fba4db..592bbaa3e149 100644
--- a/lib/conntrack.c
+++ b/lib/conntrack.c
@@ -2632,25 +2632,19 @@ conntrack_dump_start(struct conntrack *ct, struct
conntrack_dump *dump,
dump->ct = ct;
*ptot_bkts = 1; /* Need to clean up the callers. */
+dump->cursor = cmap_cursor_start(>conns);
return 0;
}
int
conntrack_dump_next(struct conntrack_dump *dump, struct ct_dpif_entry *entry)
{
-struct conntrack *ct = dump->ct;
long long now = time_msec();
-for (;;) {
-struct cmap_node *cm_node = cmap_next_position(>conns,
- >cm_pos);
-if (!cm_node) {
-break;
-}
-struct conn_key_node *keyn;
-struct conn *conn;
+struct conn_key_node *keyn;
+struct conn *conn;
-INIT_CONTAINER(keyn, cm_node, cm_node);
+CMAP_CURSOR_FOR_EACH_CONTINUE (keyn, cm_node, >cursor) {
if (keyn->dir != CT_DIR_FWD) {
continue;
}
diff --git a/lib/conntrack.h b/lib/conntrack.h
index 57d5159b61b8..ecf539b736c2 100644
--- a/lib/conntrack.h
+++ b/lib/conntrack.h
@@ -101,8 +101,8 @@ struct conntrack_dump {
struct conntrack *ct;
unsigned bucket;
union {
-struct cmap_position cm_pos;
struct hmap_position hmap_pos;
+struct cmap_cursor cursor;
};
bool filter_zone;
uint16_t zone;
diff --git a/tests/system-traffic.at b/tests/system-traffic.at
index 3cdc27951449..23404a279972 100644
--- a/tests/system-traffic.at
+++ b/tests/system-traffic.at
@@ -7913,6 +7913,65 @@ AT_CHECK([ovs-pcap client.pcap | grep
20102000], [0], [dnl
OVS_TRAFFIC_VSWITCHD_STOP
AT_CLEANUP
+AT_SETUP([conntrack - Flush many conntrack entries by port])
+CHECK_CONNTRACK()
+OVS_TRAFFIC_VSWITCHD_START()
+
+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_DATA([flows.txt], [dnl
+priority=100,in_port=1,udp,action=ct(zone=1,commit),2
+])
+
+AT_CHECK([ovs-ofctl --bundle add-flows br0 flows.txt])
+
+dnl 20 packets from port 1 and 1 packet from port 2.
+flow_l3="\
+eth_src=50:54:00:00:00:09,eth_dst=50:54:00:00:00:0a,dl_type=0x0800,\
+nw_src=10.1.1.1,nw_dst=10.1.1.2,nw_proto=17,nw_ttl=64,nw_frag=no"
+
+head="5054000a505400090800455c4011648d0a0101010a010102"
+len=72
+base_csum=1366
+tail="000102030405060708090a0b0c0d0e0f101112131415161718191a1b1c1d1e1f\
+ 202122232425262728292a2b2c2d2e2f303132333435363738393a3b3c3d3e3f"
+
+dst_port=1
+for src_port in $(seq 1 20); do
+csum=$((base_csum - src_port - dst_port))
+frame=$(printf "%s%04x%04x%04x%04x%s" $head 1 $src_port $len $csum $tail)
+AT_CHECK([ovs-ofctl -O OpenFlow13 packet-out br0 "in_port=1 packet=$frame
actions=resubmit(,0)"])
+done
+
+src_port=2
+dst_port=1
+csum=$((base_csum - src_port - dst_port))
+frame=$(printf "%s%04x%04x%04x%04x%s" $head $src_port $dst_port $len $csum
$tail)
+AT_CHECK([ovs-ofctl -O OpenFlow13 packet-out br0 "in_port=1 packet=$frame
actions=resubmit(,0)"])
+
+: > conntrack
+
+for i in $(seq 1 20); do
+echo
"udp,orig=(src=10.1.1.1,dst=10.1.1.2,sport=1,dport=${i}),reply=(src=10.1.1.2,dst=10.1.1.1,sport=${i},dport=1),zone=1"
>> conntrack
+done
+echo
"udp,orig=(src=10.1.1.1,dst=10.1.1.2,sport=2,dport=1),reply=(src=10.1.1.2,dst=10.1.1.1,sport=1,dport=2),zone=1"
>> conntrack
+
+sort conntrack > expout
+
+AT_CHECK([ovs-appctl dpctl/dump-conntrack zone=1 | grep -F "src=10.1.1.1," |
sort ], [0], [expout])
+
+dnl Check that flushing conntrack by port 1 flush all ct for port 1 but keeps
ct for port 2.
+AT_CHECK([ovs-appctl dpctl/flush-conntrack zone=1
'ct_nw_proto=17,ct_tp_src=1'])
+AT_CHECK([ovs-appctl dpctl/dump-conntrack zone=1 | grep -F "src=10.1.1.1," |
sort ], [0], [dnl
+udp,orig=(src=10.1.1.1,dst=10.1.1.2,sport=2,dport=1),reply=(src=10.1.1.2,dst=10.1.1.1,sport=1,dport=2),zone=1
+])
+
+OVS_TRAFFIC_VSWITCHD_STOP
+AT_CLEANUP
+
AT_BANNER([IGMP])
AT_SETUP([IGMP - flood under normal action])
--
2.43.0
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v3] conntrack: Fix flush not flushing all elements.
On Mon, Mar 04, 2024 at 11:18:22AM -0500, Mike Pattrick wrote:
> On Mon, Mar 4, 2024 at 10:22 AM Xavier Simonart wrote:
> >
> > On netdev datapath, when a ct element was cleaned, the cmap
> > could be shrinked, potentially causing some elements to be skipped
> > in the flush iteration.
> >
> > Fixes: 967bb5c5cd90 ("conntrack: Add rcu support.")
> > Signed-off-by: Xavier Simonart
>
> Acked-by: Mike Pattrick
Thanks Xavier and Mike,
I have applied this patch-set;
- conntrack: Fix flush not flushing all elements.
https://github.com/openvswitch/ovs/commit/6c082a8310d5
And a backport to branch-3.3:
- conntrack: Fix flush not flushing all elements.
https://github.com/openvswitch/ovs/commit/0f1af687cc7e
I have also prepared backports back to branch-2.17 which
I will post separately for review.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v3] conntrack: Remove nat_conn introducing key directionality.
+ Xavier On Thu, Aug 31, 2023 at 02:52:59PM -0400, Aaron Conole wrote: > Ilya Maximets writes: > > > On 8/31/23 09:15, Frode Nordahl wrote: > >> On Wed, Aug 30, 2023 at 9:30 PM Paolo Valerio wrote: > >>> > >>> From: hepeng > >>> > >>> The patch avoids the extra allocation for nat_conn. > >>> Currently, when doing NAT, the userspace conntrack will use an extra > >>> conn for the two directions in a flow. However, each conn has actually > >>> the two keys for both orig and rev directions. This patch introduces a > >>> key_node[CT_DIRS] member as per Aaron's suggestion in the conn which > >>> consists of a key, direction, and a cmap_node for hash lookup so > >>> addressing the feedback received by the original patch [0]. > >>> > >>> [0] > >>> https://patchwork.ozlabs.org/project/openvswitch/patch/20201129033255.64647-2-hepeng.0...@bytedance.com/ > >>> > >>> Signed-off-by: Peng He > >>> Co-authored-by: Paolo Valerio > >>> Signed-off-by: Paolo Valerio > >> > >> Thanks alot for working on this, should we perhaps reference the > >> original bug report, i.e: > >> Reported-by: Michael Plato > >> Reported-at: > >> https://mail.openvswitch.org/pipermail/ovs-discuss/2022-September/052065.html > > > > Can be added while applying, I think. It also may be worth adding > > a sentence about fixing the assertion to the commit message. > > Done. > > >> > >> We have a reproducer for the issue and we no longer see it occurring > >> with this patch. > >> Tested-by: Frode Nordahl > > > > Thanks! > > Thanks everyone! I've applied and backported down to branch-3.0, and > will work on the backport to branch-2.17. Hi Aaron, while working on [1] I notice that this patch did not seem to be backported to branch-3.2. I will plan on doing so as part of my backports of [1]. [1] [ovs-dev,v3] conntrack: Fix flush not flushing all elements. https://patchwork.ozlabs.org/project/openvswitch/patch/20240304152159.1710977-1-xsimo...@redhat.com/ ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH v8 4/4] tests: system-traffic: Add coverage for drop action.
Exercise the drop action in the datapath. This specific tests triggers an xlate_error. For the kernel datapath skb drop reasons can then be seen while this test runs. # perf trace -e skb:kfree_skb --filter="reason >= 0x3" 0.000 ping/1275884 skb:kfree_skb(skbaddr: 0x8acd76546000, \ location: 0xc0ee3634, protocol: 2048, reason: 196611) Signed-off-by: Eric Garver --- tests/system-common-macros.at | 4 tests/system-traffic.at | 31 +++ 2 files changed, 35 insertions(+) diff --git a/tests/system-common-macros.at b/tests/system-common-macros.at index 01ebe364ee7c..564bc163945e 100644 --- a/tests/system-common-macros.at +++ b/tests/system-common-macros.at @@ -374,3 +374,7 @@ m4_define([OVS_CHECK_CT_CLEAR], # OVS_CHECK_GITHUB_ACTION m4_define([OVS_CHECK_GITHUB_ACTION], [AT_SKIP_IF([test "$GITHUB_ACTIONS" = "true"])]) + +# OVS_CHECK_DROP_ACTION() +m4_define([OVS_CHECK_DROP_ACTION], +[AT_SKIP_IF([! grep -q "Datapath supports drop action" ovs-vswitchd.log])]) diff --git a/tests/system-traffic.at b/tests/system-traffic.at index 07d09b912e0b..d7673ecac8c9 100644 --- a/tests/system-traffic.at +++ b/tests/system-traffic.at @@ -2231,6 +2231,37 @@ masks-cache:size:256 OVS_TRAFFIC_VSWITCHD_STOP AT_CLEANUP +AT_SETUP([datapath - drop action]) +OVS_TRAFFIC_VSWITCHD_START() +OVS_CHECK_DROP_ACTION() +AT_KEYWORDS(drop_action) + +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") + +dnl Exceed the max number of resubmits. +(echo "dl_type=0x806, actions=normal" +for i in `seq 1 64`; do + j=`expr $i + 1` + echo "in_port=$i, actions=resubmit:$j, resubmit:$j, local" + done + echo "in_port=65, actions=local" +) > flows.txt +AT_CHECK([ovs-ofctl add-flows br0 flows.txt]) + +dnl Generate some traffic. +NS_CHECK_EXEC([at_ns0], [ping -q -c 10 -i 0.1 -w 2 10.1.1.2], [1], [ignore]) + +OVS_WAIT_UNTIL_EQUAL([ovs-appctl dpctl/dump-flows | grep "eth_type(0x0800)" | dnl + sed 's/,packet_type(ns=[[0-9]]*,id=[[0-9]]*),/,/;s/,eth(),/,/;' | dnl + strip_recirc | strip_stats | strip_used | sort], [dnl +recirc_id(),in_port(2),eth_type(0x0800),ipv4(frag=no), packets:0, bytes:0, used:0.0s, actions:drop]) + +OVS_VSWITCHD_STOP(["/|WARN|/d"]) +AT_CLEANUP + AT_SETUP([datapath - simulated flow action update]) OVS_TRAFFIC_VSWITCHD_START() -- 2.43.0 ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH v8 2/4] dpif: Support atomic_bool field type.
The next commit will convert a dp feature from bool to atomic_bool. As
such we have to add support to the macros and functions. We must pass by
reference instead of pass by value because all the atomic operations
require a reference.
Signed-off-by: Eric Garver
---
ofproto/ofproto-dpif.c | 52 --
1 file changed, 45 insertions(+), 7 deletions(-)
diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index f59d69c4d1e8..b1978e3f2fd5 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -717,6 +717,8 @@ close_dpif_backer(struct dpif_backer *backer, bool del)
}
static void check_support(struct dpif_backer *backer);
+static void copy_support(struct dpif_backer_support *dst,
+ struct dpif_backer_support *src);
static int
open_dpif_backer(const char *type, struct dpif_backer **backerp)
@@ -837,7 +839,7 @@ open_dpif_backer(const char *type, struct dpif_backer
**backerp)
* 'boottime_support' can be checked to prevent 'support' to be changed
* beyond the datapath capabilities. In case 'support' is changed by
* the user, 'boottime_support' can be used to restore it. */
-backer->bt_support = backer->rt_support;
+copy_support(>bt_support, >rt_support);
return error;
}
@@ -1611,6 +1613,22 @@ CHECK_FEATURE__(ct_orig_tuple6, ct_orig_tuple6,
ct_nw_proto, 1, ETH_TYPE_IPV6)
#undef CHECK_FEATURE
#undef CHECK_FEATURE__
+static void
+copy_support(struct dpif_backer_support *dst, struct dpif_backer_support *src)
+{
+#define DPIF_SUPPORT_FIELD(TYPE, NAME, TITLE) \
+if (!strcmp(#TYPE, "atomic_bool")) { \
+bool value; \
+atomic_read_relaxed(>NAME, ); \
+atomic_store_relaxed(>NAME, value); \
+} else { \
+dst->NAME = src->NAME; \
+}
+
+DPIF_SUPPORT_FIELDS
+#undef DPIF_SUPPORT_FIELD
+}
+
static void
check_support(struct dpif_backer *backer)
{
@@ -6249,20 +6267,30 @@ ofproto_unixctl_dpif_dump_dps(struct unixctl_conn
*conn, int argc OVS_UNUSED,
}
static void
-show_dp_feature_bool(struct ds *ds, const char *feature, bool b)
+show_dp_feature_bool(struct ds *ds, const char *feature, const bool *b)
{
-ds_put_format(ds, "%s: %s\n", feature, b ? "Yes" : "No");
+ds_put_format(ds, "%s: %s\n", feature, *b ? "Yes" : "No");
}
static void
-show_dp_feature_size_t(struct ds *ds, const char *feature, size_t s)
+show_dp_feature_atomic_bool(struct ds *ds, const char *feature,
+const atomic_bool *b)
{
-ds_put_format(ds, "%s: %"PRIuSIZE"\n", feature, s);
+bool value;
+atomic_read_relaxed(b, );
+ds_put_format(ds, "%s: %s\n", feature, value ? "Yes" : "No");
+}
+
+static void
+show_dp_feature_size_t(struct ds *ds, const char *feature, const size_t *s)
+{
+ds_put_format(ds, "%s: %"PRIuSIZE"\n", feature, *s);
}
enum dpif_support_field_type {
DPIF_SUPPORT_FIELD_bool,
DPIF_SUPPORT_FIELD_size_t,
+DPIF_SUPPORT_FIELD_atomic_bool,
};
struct dpif_support_field {
@@ -6279,12 +6307,12 @@ static void
dpif_show_support(const struct dpif_backer_support *support, struct ds *ds)
{
#define DPIF_SUPPORT_FIELD(TYPE, NAME, TITLE) \
-show_dp_feature_##TYPE (ds, TITLE, support->NAME);
+show_dp_feature_##TYPE (ds, TITLE, >NAME);
DPIF_SUPPORT_FIELDS
#undef DPIF_SUPPORT_FIELD
#define ODP_SUPPORT_FIELD(TYPE, NAME, TITLE) \
-show_dp_feature_##TYPE (ds, TITLE, support->odp.NAME );
+show_dp_feature_##TYPE (ds, TITLE, >odp.NAME );
ODP_SUPPORT_FIELDS
#undef ODP_SUPPORT_FIELD
}
@@ -6303,6 +6331,16 @@ display_support_field(const char *name,
b ? "true" : "false");
break;
}
+case DPIF_SUPPORT_FIELD_atomic_bool: {
+bool v;
+bool b;
+atomic_read_relaxed((atomic_bool *) field->rt_ptr, );
+atomic_read_relaxed((atomic_bool *) field->bt_ptr, );
+ds_put_format(ds, "%s (%s) : [run time]:%s, [boot time]:%s\n", name,
+ field->title, v ? "true" : "false",
+ b ? "true" : "false");
+break;
+}
case DPIF_SUPPORT_FIELD_size_t:
ds_put_format(ds, "%s (%s) : [run time]:%"PRIuSIZE
", [boot time]:%"PRIuSIZE"\n", name,
--
2.43.0
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH v8 0/4] dpif: probe support for OVS_ACTION_ATTR_DROP
Probe the datapath implementation for support of OVS_ACTION_ATTR_DROP. Also add a new test case. v8: - new patch to support atomic_bool dpif field types - re-add re-probe of support - use atomic_bool type for explicit_drop_action v7: - remove re-probe of support as Ilya is working on a generic solution v6: - improve log if hw-offload enabled - re-probe support if hw-offload set true at runtime v5: - combine patches 3 and 4 - add description to combined patch 3 v4: - avoid passing action to datapath if TC offload in use v3: - alter test such that it's reliable, different xlate_error - better commit message in patch 2 - reordered _DEC_TTL value in switch statements - add format_dec_ttl_action() v2: - new patch (1) to fix build (switch cases) - fixed check-system-userspace Eric Garver (4): dpif: Stub out unimplemented action OVS_ACTION_ATTR_DEC_TTL. dpif: Support atomic_bool field type. dpif: Probe support for OVS_ACTION_ATTR_DROP. tests: system-traffic: Add coverage for drop action. include/linux/openvswitch.h | 3 +- lib/dpif-netdev.c | 1 + lib/dpif.c| 7 +- lib/dpif.h| 1 - lib/odp-execute.c | 2 + lib/odp-util.c| 23 ++ ofproto/ofproto-dpif-ipfix.c | 1 + ofproto/ofproto-dpif-sflow.c | 1 + ofproto/ofproto-dpif.c| 141 +++--- ofproto/ofproto-dpif.h| 4 +- tests/system-common-macros.at | 4 + tests/system-traffic.at | 31 12 files changed, 199 insertions(+), 20 deletions(-) -- 2.43.0 ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH v8 1/4] dpif: Stub out unimplemented action OVS_ACTION_ATTR_DEC_TTL.
This is prep for adding a different OVS_ACTION_ATTR_ enum value. This
action, OVS_ACTION_ATTR_DEC_TTL, is not actually implemented. However,
to make -Werror happy we must add a case to all existing switches.
Signed-off-by: Eric Garver
---
include/linux/openvswitch.h | 1 +
lib/dpif-netdev.c| 1 +
lib/dpif.c | 1 +
lib/odp-execute.c| 2 ++
lib/odp-util.c | 23 +++
ofproto/ofproto-dpif-ipfix.c | 1 +
ofproto/ofproto-dpif-sflow.c | 1 +
7 files changed, 30 insertions(+)
diff --git a/include/linux/openvswitch.h b/include/linux/openvswitch.h
index e305c331516b..a265e05ad253 100644
--- a/include/linux/openvswitch.h
+++ b/include/linux/openvswitch.h
@@ -1085,6 +1085,7 @@ enum ovs_action_attr {
OVS_ACTION_ATTR_CLONE,/* Nested OVS_CLONE_ATTR_*. */
OVS_ACTION_ATTR_CHECK_PKT_LEN, /* Nested OVS_CHECK_PKT_LEN_ATTR_*. */
OVS_ACTION_ATTR_ADD_MPLS, /* struct ovs_action_add_mpls. */
+ OVS_ACTION_ATTR_DEC_TTL, /* Nested OVS_DEC_TTL_ATTR_*. */
#ifndef __KERNEL__
OVS_ACTION_ATTR_TUNNEL_PUSH, /* struct ovs_action_push_tnl*/
diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index e6c53937d8b9..89b0d1d6b4aa 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -9489,6 +9489,7 @@ dp_execute_cb(void *aux_, struct dp_packet_batch
*packets_,
case OVS_ACTION_ATTR_CHECK_PKT_LEN:
case OVS_ACTION_ATTR_DROP:
case OVS_ACTION_ATTR_ADD_MPLS:
+case OVS_ACTION_ATTR_DEC_TTL:
case __OVS_ACTION_ATTR_MAX:
OVS_NOT_REACHED();
}
diff --git a/lib/dpif.c b/lib/dpif.c
index d07241f1e7cd..0f480bec48d0 100644
--- a/lib/dpif.c
+++ b/lib/dpif.c
@@ -1289,6 +1289,7 @@ dpif_execute_helper_cb(void *aux_, struct dp_packet_batch
*packets_,
case OVS_ACTION_ATTR_CHECK_PKT_LEN:
case OVS_ACTION_ATTR_DROP:
case OVS_ACTION_ATTR_ADD_MPLS:
+case OVS_ACTION_ATTR_DEC_TTL:
case __OVS_ACTION_ATTR_MAX:
OVS_NOT_REACHED();
}
diff --git a/lib/odp-execute.c b/lib/odp-execute.c
index eb03b57c42ec..081e4d43268a 100644
--- a/lib/odp-execute.c
+++ b/lib/odp-execute.c
@@ -837,6 +837,7 @@ requires_datapath_assistance(const struct nlattr *a)
case OVS_ACTION_ATTR_CT_CLEAR:
case OVS_ACTION_ATTR_CHECK_PKT_LEN:
case OVS_ACTION_ATTR_ADD_MPLS:
+case OVS_ACTION_ATTR_DEC_TTL:
case OVS_ACTION_ATTR_DROP:
return false;
@@ -1227,6 +1228,7 @@ odp_execute_actions(void *dp, struct dp_packet_batch
*batch, bool steal,
case OVS_ACTION_ATTR_RECIRC:
case OVS_ACTION_ATTR_CT:
case OVS_ACTION_ATTR_UNSPEC:
+case OVS_ACTION_ATTR_DEC_TTL:
case __OVS_ACTION_ATTR_MAX:
/* The following actions are handled by the scalar implementation. */
case OVS_ACTION_ATTR_POP_VLAN:
diff --git a/lib/odp-util.c b/lib/odp-util.c
index 9306c9b4d47a..f4c492f2ae6f 100644
--- a/lib/odp-util.c
+++ b/lib/odp-util.c
@@ -143,6 +143,7 @@ odp_action_len(uint16_t type)
case OVS_ACTION_ATTR_POP_NSH: return 0;
case OVS_ACTION_ATTR_CHECK_PKT_LEN: return ATTR_LEN_VARIABLE;
case OVS_ACTION_ATTR_ADD_MPLS: return sizeof(struct ovs_action_add_mpls);
+case OVS_ACTION_ATTR_DEC_TTL: return ATTR_LEN_VARIABLE;
case OVS_ACTION_ATTR_DROP: return sizeof(uint32_t);
case OVS_ACTION_ATTR_UNSPEC:
@@ -1130,6 +1131,25 @@ format_odp_check_pkt_len_action(struct ds *ds, const
struct nlattr *attr,
ds_put_cstr(ds, "))");
}
+static void
+format_dec_ttl_action(struct ds *ds, const struct nlattr *attr,
+ const struct hmap *portno_names)
+{
+const struct nlattr *a;
+unsigned int left;
+
+ds_put_cstr(ds,"dec_ttl(le_1(");
+NL_ATTR_FOR_EACH (a, left,
+ nl_attr_get(attr), nl_attr_get_size(attr)) {
+if (nl_attr_type(a) == OVS_ACTION_ATTR_DEC_TTL) {
+ format_odp_actions(ds, nl_attr_get(a),
+ nl_attr_get_size(a), portno_names);
+ break;
+}
+}
+ds_put_format(ds, "))");
+}
+
static void
format_odp_action(struct ds *ds, const struct nlattr *a,
const struct hmap *portno_names)
@@ -1283,6 +1303,9 @@ format_odp_action(struct ds *ds, const struct nlattr *a,
ntohs(mpls->mpls_ethertype));
break;
}
+case OVS_ACTION_ATTR_DEC_TTL:
+format_dec_ttl_action(ds, a, portno_names);
+break;
case OVS_ACTION_ATTR_DROP:
ds_put_cstr(ds, "drop");
break;
diff --git a/ofproto/ofproto-dpif-ipfix.c b/ofproto/ofproto-dpif-ipfix.c
index e6c2968f7e90..cd65dae7e18a 100644
--- a/ofproto/ofproto-dpif-ipfix.c
+++ b/ofproto/ofproto-dpif-ipfix.c
@@ -3135,6 +3135,7 @@ dpif_ipfix_read_actions(const struct flow *flow,
case OVS_ACTION_ATTR_UNSPEC:
case OVS_ACTION_ATTR_DROP:
case OVS_ACTION_ATTR_ADD_MPLS:
+case OVS_ACTION_ATTR_DEC_TTL:
case __OVS_ACTION_ATTR_MAX:
default:
[ovs-dev] [PATCH v8 3/4] dpif: Probe support for OVS_ACTION_ATTR_DROP.
Kernel support has been added for this action. As such, we need to probe
the datapath for support.
Signed-off-by: Eric Garver
---
include/linux/openvswitch.h | 2 +-
lib/dpif.c | 6 ---
lib/dpif.h | 1 -
ofproto/ofproto-dpif.c | 89 +++--
ofproto/ofproto-dpif.h | 4 +-
5 files changed, 89 insertions(+), 13 deletions(-)
diff --git a/include/linux/openvswitch.h b/include/linux/openvswitch.h
index a265e05ad253..fce6456f47c8 100644
--- a/include/linux/openvswitch.h
+++ b/include/linux/openvswitch.h
@@ -1086,11 +1086,11 @@ enum ovs_action_attr {
OVS_ACTION_ATTR_CHECK_PKT_LEN, /* Nested OVS_CHECK_PKT_LEN_ATTR_*. */
OVS_ACTION_ATTR_ADD_MPLS, /* struct ovs_action_add_mpls. */
OVS_ACTION_ATTR_DEC_TTL, /* Nested OVS_DEC_TTL_ATTR_*. */
+ OVS_ACTION_ATTR_DROP, /* u32 xlate_error. */
#ifndef __KERNEL__
OVS_ACTION_ATTR_TUNNEL_PUSH, /* struct ovs_action_push_tnl*/
OVS_ACTION_ATTR_TUNNEL_POP,/* u32 port number. */
- OVS_ACTION_ATTR_DROP, /* u32 xlate_error. */
OVS_ACTION_ATTR_LB_OUTPUT, /* u32 bond-id. */
#endif
__OVS_ACTION_ATTR_MAX,/* Nothing past this will be accepted
diff --git a/lib/dpif.c b/lib/dpif.c
index 0f480bec48d0..28fa40879655 100644
--- a/lib/dpif.c
+++ b/lib/dpif.c
@@ -1934,12 +1934,6 @@ dpif_supports_tnl_push_pop(const struct dpif *dpif)
return dpif_is_netdev(dpif);
}
-bool
-dpif_supports_explicit_drop_action(const struct dpif *dpif)
-{
-return dpif_is_netdev(dpif);
-}
-
bool
dpif_supports_lb_output_action(const struct dpif *dpif)
{
diff --git a/lib/dpif.h b/lib/dpif.h
index 0f2dc2ef3c55..8dc5da45b3ef 100644
--- a/lib/dpif.h
+++ b/lib/dpif.h
@@ -940,7 +940,6 @@ int dpif_get_pmds_for_port(const struct dpif * dpif,
odp_port_t port_no,
char *dpif_get_dp_version(const struct dpif *);
bool dpif_supports_tnl_push_pop(const struct dpif *);
-bool dpif_supports_explicit_drop_action(const struct dpif *);
bool dpif_synced_dp_layers(struct dpif *);
/* Log functions. */
diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index b1978e3f2fd5..da83d6196b13 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -72,6 +72,8 @@
#include "util.h"
#include "uuid.h"
#include "vlan-bitmap.h"
+#include "dpif-netdev.h"
+#include "netdev-offload.h"
VLOG_DEFINE_THIS_MODULE(ofproto_dpif);
@@ -221,6 +223,8 @@ static void ct_zone_config_init(struct dpif_backer *backer);
static void ct_zone_config_uninit(struct dpif_backer *backer);
static void ct_zone_timeout_policy_sweep(struct dpif_backer *backer);
static void ct_zone_limits_commit(struct dpif_backer *backer);
+static bool check_drop_action(struct dpif_backer *backer);
+static bool recheck_support_explicit_drop_action(struct dpif_backer *backer);
static inline struct ofproto_dpif *
ofproto_dpif_cast(const struct ofproto *ofproto)
@@ -391,6 +395,10 @@ type_run(const char *type)
udpif_set_threads(backer->udpif, n_handlers, n_revalidators);
}
+if (recheck_support_explicit_drop_action(backer)) {
+backer->need_revalidate = REV_RECONFIGURE;
+}
+
if (backer->need_revalidate) {
struct ofproto_dpif *ofproto;
struct simap_node *node;
@@ -807,6 +815,7 @@ open_dpif_backer(const char *type, struct dpif_backer
**backerp)
shash_add(_dpif_backers, type, backer);
+atomic_init(>rt_support.explicit_drop_action, false);
check_support(backer);
atomic_count_init(>tnl_count, 0);
@@ -855,7 +864,10 @@ ovs_native_tunneling_is_on(struct ofproto_dpif *ofproto)
bool
ovs_explicit_drop_action_supported(struct ofproto_dpif *ofproto)
{
-return ofproto->backer->rt_support.explicit_drop_action;
+bool value;
+atomic_read_relaxed(>backer->rt_support.explicit_drop_action,
+);
+return value;
}
bool
@@ -1379,6 +1391,43 @@ check_ct_timeout_policy(struct dpif_backer *backer)
return !error;
}
+/* Tests whether backer's datapath supports the OVS_ACTION_ATTR_DROP action. */
+static bool
+check_drop_action(struct dpif_backer *backer)
+{
+struct odputil_keybuf keybuf;
+uint8_t actbuf[NL_A_U32_SIZE];
+struct ofpbuf actions;
+struct ofpbuf key;
+struct flow flow;
+bool supported;
+bool hw_offload;
+
+struct odp_flow_key_parms odp_parms = {
+.flow = ,
+.probe = true,
+};
+
+memset(, 0, sizeof flow);
+ofpbuf_use_stack(, , sizeof keybuf);
+odp_flow_key_from_flow(_parms, );
+
+ofpbuf_use_stack(, , sizeof actbuf);
+nl_msg_put_u32(, OVS_ACTION_ATTR_DROP, XLATE_OK);
+
+supported = dpif_probe_feature(backer->dpif, "drop", , , NULL);
+
+/* TC does not support offloading this action. */
+hw_offload = netdev_is_flow_api_enabled() && !dpif_is_netdev(backer->dpif);
+
+VLOG_INFO("%s: Datapath %s drop action%s", dpif_name(backer->dpif),
+ (supported) ?
Re: [ovs-dev] [PATCH v6 2/2] netlink-conntrack: Optimize flushing ct zone.
On Mon, Mar 4, 2024 at 3:22 AM Felix Huettner via dev wrote: > > Previously the kernel did not provide a netlink interface to flush/list > only conntrack entries matching a specific zone. With [1] and [2] it is now > possible to flush and list conntrack entries filtered by zone. Older > kernels not yet supporting this feature will ignore the filter. > For the list request that means just returning all entries (which we can > then filter in userspace as before). > For the flush request that means deleting all conntrack entries. > > The implementation is now identical to the windows one, so we combine > them. > > These significantly improves the performance of flushing conntrack zones > when the conntrack table is large. Since flushing a conntrack zone is > normally triggered via an openflow command it blocks the main ovs thread > and thereby also blocks new flows from being applied. Using this new > feature we can reduce the flushing time for zones by around 93%. > > In combination with OVN the creation of a Logical_Router (which causes > the flushing of a ct zone) could block other operations, e.g. the > failover of Logical_Routers (as they cause new flows to be created). > This is visible from a user perspective as a ovn-controller that is idle > (as it waits for vswitchd) and vswitchd reporting: > "blocked 1000 ms waiting for main to quiesce" (potentially with ever > increasing times). > > The following performance tests where run in a qemu vm with 500.000 > conntrack entries distributed evenly over 500 ct zones using `ovstest > test-netlink-conntrack flush zone=`. > > | flush zone with 1000 entries | flush zone with no entry | > +-+--+-+--| > | with the patch| without | with the patch| without | > +--+--+--+--+--+--| > | v6.8-rc4 | v6.7.1 | v6.8-rc4 | v6.8-rc4 | v6.7.1 | v6.8-rc4 | > +-+--+--+--+--+--+--| > | Min | 0.260 | 3.946 | 3.497 | 0.228 | 3.462 | 3.212 | > | Median | 0.319 | 4.237 | 4.349 | 0.298 | 4.460 | 4.010 | > | 90%ile | 0.335 | 4.367 | 4.522 | 0.325 | 4.662 | 4.572 | > | 99%ile | 0.348 | 4.495 | 4.773 | 0.340 | 4.931 | 6.003 | > | Max | 0.362 | 4.543 | 5.054 | 0.348 | 5.390 | 6.396 | > | Mean| 0.320 | 4.236 | 4.331 | 0.296 | 4.430 | 4.071 | > | Total | 80.02 | 1058| 1082| 73.93 | 1107| 1017| > > [1]: > https://github.com/torvalds/linux/commit/eff3c558bb7e61c41b53e4c8130e514a5a4df9ba > [2]: > https://github.com/torvalds/linux/commit/fa173a1b4e3fd1ab5451cbc57de6fc624c824b0a > > Co-Authored-By: Luca Czesla > Signed-off-by: Luca Czesla > Co-Authored-By: Max Lamprecht > Signed-off-by: Max Lamprecht > Signed-off-by: Felix Huettner > --- Acked-by: Mike Pattrick ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [Patch ovn 2/2] northd.c: Fix direct access to SNAT network on DR.
On Wed, Mar 6, 2024 at 1:50 PM Martin Kalcok wrote: > > Thanks for reviewing this as well, Ales. > > On Wed, Mar 6, 2024 at 8:08 AM Ales Musil wrote: > >> >> >> On Wed, Mar 6, 2024 at 8:07 AM Ales Musil wrote: >> >>> >>> >>> On Mon, Mar 4, 2024 at 11:56 AM Martin Kalcok < >>> martin.kal...@canonical.com> wrote: >>> This change fixes bug that breaks ability of machines from external networks to communicate with machines in SNATed networks (specifically when using a Distributed router). Currently when a machine (S1) on an external network tries to talk over TCP with a machine (A1) in a network that has enabled SNAT, the connection is established successfully. However after the three-way handshake, any packets that come from the A1 machine will have their source address translated by the Distributed router, breaking the communication. Existing rule in `build_lrouter_out_snat_flow` that decides which packets should be SNATed already tries to avoid SNATing packets in reply direction with `(!ct.trk || !ct.rpl)`. However, previous stages in the distributed LR egress pipeline do not initiate the CT state. Additionally we need to commit new connections that originate from external networks into CT, so that the packets in the reply direction can be properly identified. Rationale: In my original RFC [0], there were questions about the motivation for fixing this issue. I'll try to summarize why I think this is a bug that should be fixed. 1. Current implementation for Distributed router already tries to avoid SNATing packets in the reply direction, it's just missing initialized CT states to make proper decisions. 2. This same scenario works with Gateway Router. I tested with following setup: foo -- R1 -- join - R3 -- alice | bar --R2 R1 is a Distributed router with SNAT for foo. R2 is a Gateway router with SNAT for bar. R3 is a Gateway router with no SNAT. Using 'alice1' as a client I was able to talk over TCP with 'bar1' but connection with 'foo1' failed. 3. Regarding security and "leaking" of internal IPs. Reading through RFC 4787 [1], 5382 [2] and their update in 7857 [3], the specifications do not seem to mandate that SNAT implementations must filter incoming traffic destined directly to the internal network. Sections like "5. Filtering Behavior" in 4787 and "4.3 Externally Initiated Connections" in 5382 describe only behavior for traffic destined to external IP/Port associated with NAT on the device that performs NAT. Besides, with the current implementation, it's already possible to scan the internal network with pings and TCP syn scanning. If an additional security is needed, ACLs can be used to filter out incomming traffic from external networks. 4. We do have customers/clouds that depend on this functionality. This is a scenario that used to work in Openstack with ML2/OVS and migrating those clouds to ML2/OVN would break it. [0] https://mail.openvswitch.org/pipermail/ovs-dev/2024-February/411670.html [1]https://datatracker.ietf.org/doc/html/rfc4787 [2]https://datatracker.ietf.org/doc/html/rfc5382 [3]https://datatracker.ietf.org/doc/html/rfc7857 Signed-off-by: Martin Kalcok --- >>> >>> Hi Martin, >>> >>> I have a few comments down below. >>> >>> northd/northd.c | 82 - northd/ovn-northd.8.xml | 29 +++ tests/ovn-northd.at | 15 +++- tests/system-ovn.at | 69 ++ 4 files changed, 185 insertions(+), 10 deletions(-) diff --git a/northd/northd.c b/northd/northd.c index 2c3560ce2..4b79b357c 100644 --- a/northd/northd.c +++ b/northd/northd.c @@ -14437,21 +14437,28 @@ build_lrouter_out_is_dnat_local(struct lflow_table *lflows, } static void -build_lrouter_out_snat_match(struct lflow_table *lflows, - const struct ovn_datapath *od, - const struct nbrec_nat *nat, struct ds *match, - bool distributed_nat, int cidr_bits, bool is_v6, - struct ovn_port *l3dgw_port, - struct lflow_ref *lflow_ref) +build_lrouter_out_snat_direction_match__(struct lflow_table *lflows, + const struct ovn_datapath *od, + const struct nbrec_nat *nat, + struct ds *match, +
Re: [ovs-dev] [Patch ovn 2/2] northd.c: Fix direct access to SNAT network on DR.
Thanks for reviewing this as well, Ales. On Wed, Mar 6, 2024 at 8:08 AM Ales Musil wrote: > > > On Wed, Mar 6, 2024 at 8:07 AM Ales Musil wrote: > >> >> >> On Mon, Mar 4, 2024 at 11:56 AM Martin Kalcok < >> martin.kal...@canonical.com> wrote: >> >>> This change fixes bug that breaks ability of machines from external >>> networks to communicate with machines in SNATed networks (specifically >>> when using a Distributed router). >>> >>> Currently when a machine (S1) on an external network tries to talk >>> over TCP with a machine (A1) in a network that has enabled SNAT, the >>> connection is established successfully. However after the three-way >>> handshake, any packets that come from the A1 machine will have their >>> source address translated by the Distributed router, breaking the >>> communication. >>> >>> Existing rule in `build_lrouter_out_snat_flow` that decides which >>> packets should be SNATed already tries to avoid SNATing packets in >>> reply direction with `(!ct.trk || !ct.rpl)`. However, previous stages >>> in the distributed LR egress pipeline do not initiate the CT state. >>> >>> Additionally we need to commit new connections that originate from >>> external networks into CT, so that the packets in the reply direction >>> can be properly identified. >>> >>> Rationale: >>> >>> In my original RFC [0], there were questions about the motivation for >>> fixing this issue. I'll try to summarize why I think this is a bug >>> that should be fixed. >>> >>> 1. Current implementation for Distributed router already tries to >>>avoid SNATing packets in the reply direction, it's just missing >>>initialized CT states to make proper decisions. >>> >>> 2. This same scenario works with Gateway Router. I tested with >>>following setup: >>> >>> foo -- R1 -- join - R3 -- alice >>> | >>> bar --R2 >>> >>> R1 is a Distributed router with SNAT for foo. R2 is a Gateway >>> router with SNAT for bar. R3 is a Gateway router with no SNAT. >>> Using 'alice1' as a client I was able to talk over TCP with >>> 'bar1' but connection with 'foo1' failed. >>> >>> 3. Regarding security and "leaking" of internal IPs. Reading through >>>RFC 4787 [1], 5382 [2] and their update in 7857 [3], the >>>specifications do not seem to mandate that SNAT implementations >>>must filter incoming traffic destined directly to the internal >>>network. Sections like "5. Filtering Behavior" in 4787 and >>>"4.3 Externally Initiated Connections" in 5382 describe only >>>behavior for traffic destined to external IP/Port associated >>>with NAT on the device that performs NAT. >>> >>>Besides, with the current implementation, it's already possible >>>to scan the internal network with pings and TCP syn scanning. >>> >>>If an additional security is needed, ACLs can be used to filter >>>out incomming traffic from external networks. >>> >>> 4. We do have customers/clouds that depend on this functionality. >>>This is a scenario that used to work in Openstack with ML2/OVS >>>and migrating those clouds to ML2/OVN would break it. >>> >>> [0] >>> https://mail.openvswitch.org/pipermail/ovs-dev/2024-February/411670.html >>> [1]https://datatracker.ietf.org/doc/html/rfc4787 >>> [2]https://datatracker.ietf.org/doc/html/rfc5382 >>> [3]https://datatracker.ietf.org/doc/html/rfc7857 >>> >>> Signed-off-by: Martin Kalcok >>> --- >>> >> >> Hi Martin, >> >> I have a few comments down below. >> >> >>> northd/northd.c | 82 - >>> northd/ovn-northd.8.xml | 29 +++ >>> tests/ovn-northd.at | 15 +++- >>> tests/system-ovn.at | 69 ++ >>> 4 files changed, 185 insertions(+), 10 deletions(-) >>> >>> diff --git a/northd/northd.c b/northd/northd.c >>> index 2c3560ce2..4b79b357c 100644 >>> --- a/northd/northd.c >>> +++ b/northd/northd.c >>> @@ -14437,21 +14437,28 @@ build_lrouter_out_is_dnat_local(struct >>> lflow_table *lflows, >>> } >>> >>> static void >>> -build_lrouter_out_snat_match(struct lflow_table *lflows, >>> - const struct ovn_datapath *od, >>> - const struct nbrec_nat *nat, struct ds >>> *match, >>> - bool distributed_nat, int cidr_bits, bool >>> is_v6, >>> - struct ovn_port *l3dgw_port, >>> - struct lflow_ref *lflow_ref) >>> +build_lrouter_out_snat_direction_match__(struct lflow_table *lflows, >>> + const struct ovn_datapath *od, >>> + const struct nbrec_nat *nat, >>> + struct ds *match, >>> + bool distributed_nat, int >>> cidr_bits, >>> + bool is_v6, >>> + struct ovn_port
Re: [ovs-dev] [Patch ovn 1/2] actions.c/h: Enable specifying zone for ct_commit.
Hi Ales,
Thank you for review and helpful comments. I'll update commit subjects for
this series in v2.
On Wed, Mar 6, 2024 at 7:45 AM Ales Musil wrote:
>
>
> On Mon, Mar 4, 2024 at 11:56 AM Martin Kalcok
> wrote:
>
>> Action `ct_commit` currently does not allow specifying zone into
>> which connection is committed. For example, in LR datapath, the
>> `ct_commit`
>> will always use the DNAT zone.
>>
>> This change adds option to use `ct_commit(snat)` or `ct_commit(dnat)` to
>> explicitly specify the zone into which the connection will be committed.
>>
>> Original behavior of `ct_commit` without the arguments remains unchanged.
>>
>> Signed-off-by: Martin Kalcok
>>
>
> Hi Martin,
>
> thank you for the followup, I have a few comments down below.
> One small nit for the commit subject, we usually specify only the module
> name without .c/.h e.g. action, ovn-controller, northd etc.
> AFAIR ct_commit_v1 is deprecated, but I guess it's fine to repurpose it
> for this. Maybe we could remove the old behavior and leave just this?
> Before posting v2 we should discuss this with others.
> Adding Dumitru and Numan to this discussion.
>
Ack, I'll defer to your recommendation as to where this should be
implemented. If I read it correctly, `parse_ct_commit_v1_arg` still has
code to parse mark/label options but it's never used because if `ct_commit`
action is followed by "{", the `parse_CT_COMMIT` will send it to
`parse_nested_action` instead. If that's the case and the
`parse_CT_COMMIT_V1` is unused, I'd be happy to remove it/repurpose it to
something like `parse_CT_COMMIT_ZONE` that handles only
`ct_commit({snat,dnat})` actions.
>
>> ---
>> include/ovn/actions.h | 9 +
>> lib/actions.c | 20 +++-
>> ovn-sb.xml| 9 +
>> 3 files changed, 37 insertions(+), 1 deletion(-)
>>
>> diff --git a/include/ovn/actions.h b/include/ovn/actions.h
>> index 49fb96fc6..ce9597cf5 100644
>> --- a/include/ovn/actions.h
>> +++ b/include/ovn/actions.h
>> @@ -259,11 +259,20 @@ struct ovnact_ct_next {
>> uint8_t ltable;/* Logical table ID of next table. */
>> };
>>
>> +/* Conntrack zone to be used for commiting CT entries by the action.
>> + * DEFAULT uses default zone for given datapath. */
>> +enum ovnact_ct_zone {
>> +OVNACT_CT_ZONE_DEFAULT,
>> +OVNACT_CT_ZONE_SNAT,
>> +OVNACT_CT_ZONE_DNAT,
>> +};
>> +
>>
>
> There is no need for this enum, see details below.
>
>
>> /* OVNACT_CT_COMMIT_V1. */
>> struct ovnact_ct_commit_v1 {
>> struct ovnact ovnact;
>> uint32_t ct_mark, ct_mark_mask;
>> ovs_be128 ct_label, ct_label_mask;
>> +enum ovnact_ct_zone zone;
>>
> };
>>
>> /* Type of NAT used for the particular action.
>> diff --git a/lib/actions.c b/lib/actions.c
>> index a45874dfb..319e65b6f 100644
>> --- a/lib/actions.c
>> +++ b/lib/actions.c
>> @@ -707,6 +707,7 @@ static void
>> parse_ct_commit_v1_arg(struct action_context *ctx,
>> struct ovnact_ct_commit_v1 *cc)
>> {
>> +cc->zone = OVNACT_CT_ZONE_DEFAULT;
>> if (lexer_match_id(ctx->lexer, "ct_mark")) {
>> if (!lexer_force_match(ctx->lexer, LEX_T_EQUALS)) {
>> return;
>> @@ -737,6 +738,10 @@ parse_ct_commit_v1_arg(struct action_context *ctx,
>> return;
>> }
>> lexer_get(ctx->lexer);
>> +} else if (lexer_match_id(ctx->lexer, "snat")) {
>> +cc->zone = OVNACT_CT_ZONE_SNAT;
>> +} else if (lexer_match_id(ctx->lexer, "dnat")) {
>> +cc->zone = OVNACT_CT_ZONE_DNAT;
>> } else {
>> lexer_syntax_error(ctx->lexer, NULL);
>> }
>> @@ -814,7 +819,20 @@ encode_CT_COMMIT_V1(const struct ovnact_ct_commit_v1
>> *cc,
>> struct ofpact_conntrack *ct = ofpact_put_CT(ofpacts);
>> ct->flags = NX_CT_F_COMMIT;
>> ct->recirc_table = NX_CT_RECIRC_NONE;
>> -ct->zone_src.field = mf_from_id(MFF_LOG_CT_ZONE);
>> +switch (cc->zone) {
>> +case OVNACT_CT_ZONE_SNAT:
>> +ct->zone_src.field = mf_from_id(MFF_LOG_SNAT_ZONE);
>> +break;
>> +
>> +case OVNACT_CT_ZONE_DNAT:
>> +ct->zone_src.field = mf_from_id(MFF_LOG_DNAT_ZONE);
>> +break;
>> +
>> +case OVNACT_CT_ZONE_DEFAULT:
>> +default:
>> +ct->zone_src.field = mf_from_id(MFF_LOG_CT_ZONE);
>> +break;
>> +}
>>
>
> This would actually break potential use in the switch pipeline when
> someone would specify ct_commit(snat)/ct_commit(dnat).
> The switch pipeline uses MFF_LOG_CT_ZONE only. See ct_commit_nat for
> example [0]. There is only indication if it's snat or dnat and check for
> switch/router pipeline.
>
I see. I did not quite understood that the MFF_LOG_CT_ZONE is there for the
switch, thanks, I'll copy the behavior from `encode_CT_COMMIT_NAT` and I'll
use `bool dnat_zone` instead of the enum.
> ct->zone_src.ofs = 0;
>> ct->zone_src.n_bits = 16;
>>
>> diff --git a/ovn-sb.xml b/ovn-sb.xml
>> index ac4e585f2..66cb9747d 100644
>> ---
Re: [ovs-dev] [PATCH v2 0/5] DEI: Address some instances of master and slave
On Fri, Mar 01, 2024 at 02:50:29PM +, Simon Horman wrote:
> Recently OVS adopted a policy of using the inclusive naming word list v1
> [1, 2].
>
> This patch-set addresses some uses of the terms master and slave
> by using alternate terms.
>
> These changes are not intended to have any runtime effect.
>
> [1] df5e5cf ("Documentation: Add section on inclusive language.")
> [2] https://inclusivenaming.org/word-lists/
Thanks Eelco and Kevin for your review,
I have applied this series.
- utilities: Use localhost as sample hostname.
https://github.com/openvswitch/ovs/commit/e0aa15f897f1
- netdev-linux: Rename local variables as primary_*.
https://github.com/openvswitch/ovs/commit/b3ebc34a065e
- netdev-linux: Rename struct nedev_linux field as is_lag_primary.
https://github.com/openvswitch/ovs/commit/f92b30a0ff88
- Documentation: Update to refer to main repository.
https://github.com/openvswitch/ovs/commit/0c255bf763cc
- vswitch.xml: Use member wording for bonds.
https://github.com/openvswitch/ovs/commit/29e09c80916c
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v2 3/5] netdev-linux: Rename sturct nedev_linux field as is_lag_primary
On Fri, Mar 01, 2024 at 05:41:44PM +0100, Eelco Chaudron wrote:
>
>
> On 1 Mar 2024, at 15:50, Simon Horman wrote:
>
> > Recently OVS adopted a policy of using the inclusive naming word list v1
> > [1, 2].
> >
> > This patch partially addresses the use of the term master in the
> > context of LAG devices by using the term primary instead: the
> > is_lag_master field of struct netdev_linux is renamed is_lag_primary.
> >
> > A related comment is also updated.
> >
> > No functional change intended.
> >
> > [1] df5e5cf ("Documentation: Add section on inclusive language.")
> > [2] https://inclusivenaming.org/word-lists/
> >
> > Signed-off-by: Simon Horman
>
> Thanks for fixing these. Except for the missing dot in the subject, the
> changes look good to me. Assuming you will fix this when applying;
Thanks Eelco, will do.
> Acked-by: Eelco Chaudron
>
> //Eelco
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v2 0/5] DEI: Address some instances of master and slave
On Tue, Mar 05, 2024 at 04:08:19PM +, Kevin Traynor wrote:
> On 01/03/2024 14:50, Simon Horman wrote:
> > Recently OVS adopted a policy of using the inclusive naming word list v1
> > [1, 2].
> >
> > This patch-set addresses some uses of the terms master and slave
> > by using alternate terms.
> >
> > These changes are not intended to have any runtime effect.
> >
> > [1] df5e5cf ("Documentation: Add section on inclusive language.")
> > [2] https://inclusivenaming.org/word-lists/
> >
> > ---
> > Simon Horman (5):
> > vswitch.xml: Use member wording for bonds.
> > Documentation: Update to refer to main repository.
> > netdev-linux: Rename sturct nedev_linux field as is_lag_primary
> > netdev-linux: Rename local variables as primary_*.
> > utilities: Use localhost as sample hostname.
> >
> > .../internals/committer-grant-revocation.rst | 2 +-
> > Documentation/intro/install/dpdk.rst | 2 +-
> > lib/netdev-linux-private.h | 2 +-
> > lib/netdev-linux.c | 35
> > +++---
> > utilities/usdt-scripts/kernel_delay.rst| 10 +++
> > vswitchd/vswitch.xml | 4 +--
> > 6 files changed, 28 insertions(+), 27 deletions(-)
> >
> > base-commit: cc0e7951818a48fbbd11664e1ad0a509a180b558
> >
> > ___
> > dev mailing list
> > d...@openvswitch.org
> > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
> >
>
> Series LGTM (nit: typo sturct in one of the titles). Thanks Simon.
Thanks,
spelling of struct corrected and trailing '.' added to title of same patch.
> Acked-by: Kevin Traynor
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
