Re: [ovs-dev] [PATCH v5 2/2] openflow: Add extension to flush CT by generic match
On 12/16/22 13:30, Ales Musil wrote:
> Add extension that allows to flush connections from CT
> by specifying fields that the connections should be
> matched against. This allows to match only some fields
> of the connection e.g. source address for orig direrction.
*direction
>
> Reported-at: https://bugzilla.redhat.com/2120546
> Signed-off-by: Ales Musil
> ---
> v5: Add missing usage and man for ovs-ofctl command.
> v4: Allow ovs-ofctl flush/conntrack without any zone/tuple.
> v3: Rebase on top of master.
> v2: Rebase on top of master.
> Use suggestion from Ilya.
> ---
> NEWS | 6 ++
> include/openflow/nicira-ext.h | 30 +++
> include/openvswitch/ofp-msgs.h | 4 +
> include/openvswitch/ofp-util.h | 4 +
> lib/ofp-bundle.c | 1 +
> lib/ofp-ct-util.c | 146 +
> lib/ofp-ct-util.h | 9 ++
> lib/ofp-print.c| 20 +
> lib/ofp-util.c | 25 ++
> lib/rconn.c| 1 +
> ofproto/ofproto-dpif.c | 8 +-
> ofproto/ofproto-provider.h | 7 +-
> ofproto/ofproto.c | 30 ++-
> tests/ofp-print.at | 93 +
> tests/ovs-ofctl.at | 26 ++
> tests/system-traffic.at| 116 ++
> utilities/ovs-ofctl.8.in | 20 +
> utilities/ovs-ofctl.c | 40 +
> 18 files changed, 528 insertions(+), 58 deletions(-)
>
> diff --git a/NEWS b/NEWS
> index ff8904b02..ac73d6293 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -16,6 +16,12 @@ Post-v3.0.0
> by specifying 'max-rate' or '[r]stp-path-cost' accordingly.
> - ovs-dpctl and related ovs-appctl commands:
> * "flush-conntrack" is capable of handling partial 5-tuple.
> + - OpenFlow:
> + * New OpenFlow extension NXT_CT_FLUSH to flush connections matching
> + the specified fields.
> + - ovs-ofctl:
> + * New command "flush-conntrack" that accepts zone and 5-tuple or partial
> + 5-tuple.
ovs-ofctl commands supposed to mimic actual OF command names,
so, this should be 'ovs-ofctl ct-flush'.
>
>
> v3.0.0 - 15 Aug 2022
> diff --git a/include/openflow/nicira-ext.h b/include/openflow/nicira-ext.h
> index b68804991..32ce56d31 100644
> --- a/include/openflow/nicira-ext.h
> +++ b/include/openflow/nicira-ext.h
> @@ -1064,4 +1064,34 @@ struct nx_zone_id {
> };
> OFP_ASSERT(sizeof(struct nx_zone_id) == 8);
>
> +/* CT flush available TLVs. */
> +enum nx_ct_flush_tlv_type {
> +/* Outer types. */
> +NXT_CT_ORIG_DIRECTION,/* CT orig direction outer type. */
> +NXT_CT_REPLY_DIRECTION, /* CT reply direction outer type. */
Do we need both? See my comments on the patch 1/2.
> +
> +/* Nested types. */
> +NXT_CT_SRC, /* CT source IPv6 or mapped IPv4 address. */
> +NXT_CT_DST, /* CT destination IPv6 or mapped IPv4 address.
> */
> +NXT_CT_SRC_PORT, /* CT source port. */
> +NXT_CT_DST_PORT, /* CT destination port. */
> +NXT_CT_ICMP_ID, /* CT ICMP id. */
> +NXT_CT_ICMP_TYPE, /* CT ICMP type. */
> +NXT_CT_ICMP_CODE, /* CT ICMP code. */
I'm not sure about nested types being part of the same enum.
If we will need to add one new nested type, we will not be
able to add it into the same block here, because we'll
have to preserve a value of NXT_CT_ZONE_ID.
> +
> +/* Primitive types. */
> +NXT_CT_ZONE_ID, /* CT zone id. */
> +};
> +
> +/* NXT_CT_FLUSH.
> + *
> + * Flushes the connection tracking specified by 5-tuple.
> + * The struct should be followed by TLVs specifying the matching parameters.
> */
> +struct nx_ct_flush {
> +uint8_t ip_proto; /* IP protocol. */
> +uint8_t family;/* L3 address family. */
AFAICT, you're expecting users to put a numerical value
of AF_INET or AF_INET6 in this field. However, these
values are non-standard. And they are different on
different systems. For example, winsdk defines AF_INET6
as 23, while it's 10 on linux. Other systems use different
values as well. So, it cannot be used as part of OpenFlow.
You need a separate enum for them. Alternatively, we could
drop that field from the protocol, because it can be
determined by looking at supplied ip addresses, right?
> +uint8_t zero[6]; /* Must be zero. */
Name it 'pad' instead.
Please, add the 'Followed by' comment here.
> +};
> +OFP_ASSERT(sizeof(struct nx_ct_flush) == 8);
> +
> #endif /* openflow/nicira-ext.h */
> diff --git a/include/openvswitch/ofp-msgs.h b/include/openvswitch/ofp-msgs.h
> index 921a937e5..659b0a3e7 100644
> --- a/include/openvswitch/ofp-msgs.h
> +++ b/include/openvswitch/ofp-msgs.h
> @@ -526,6 +526,9 @@ enum ofpraw {
>
> /* NXST 1.0+ (4): struct nx_ipfix_stats_reply[]. */
> OFPRAW_NXST_IPFIX_FLOW_REPLY,
> +
> +/* NXT 1.0+ (32): struct nx_ct_flush, uint8_t[8][].
Re: [ovs-dev] [PATCH v5 2/2] openflow: Add extension to flush CT by generic match
Ales Musil writes: > Add extension that allows to flush connections from CT > by specifying fields that the connections should be > matched against. This allows to match only some fields > of the connection e.g. source address for orig direrction. > > Reported-at: https://bugzilla.redhat.com/2120546 > Signed-off-by: Ales Musil > --- > v5: Add missing usage and man for ovs-ofctl command. > v4: Allow ovs-ofctl flush/conntrack without any zone/tuple. > v3: Rebase on top of master. > v2: Rebase on top of master. > Use suggestion from Ilya. > --- Thanks Ales. LGTM, Acked-by: Paolo Valerio ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH v5 2/2] openflow: Add extension to flush CT by generic match
Add extension that allows to flush connections from CT
by specifying fields that the connections should be
matched against. This allows to match only some fields
of the connection e.g. source address for orig direrction.
Reported-at: https://bugzilla.redhat.com/2120546
Signed-off-by: Ales Musil
---
v5: Add missing usage and man for ovs-ofctl command.
v4: Allow ovs-ofctl flush/conntrack without any zone/tuple.
v3: Rebase on top of master.
v2: Rebase on top of master.
Use suggestion from Ilya.
---
NEWS | 6 ++
include/openflow/nicira-ext.h | 30 +++
include/openvswitch/ofp-msgs.h | 4 +
include/openvswitch/ofp-util.h | 4 +
lib/ofp-bundle.c | 1 +
lib/ofp-ct-util.c | 146 +
lib/ofp-ct-util.h | 9 ++
lib/ofp-print.c| 20 +
lib/ofp-util.c | 25 ++
lib/rconn.c| 1 +
ofproto/ofproto-dpif.c | 8 +-
ofproto/ofproto-provider.h | 7 +-
ofproto/ofproto.c | 30 ++-
tests/ofp-print.at | 93 +
tests/ovs-ofctl.at | 26 ++
tests/system-traffic.at| 116 ++
utilities/ovs-ofctl.8.in | 20 +
utilities/ovs-ofctl.c | 40 +
18 files changed, 528 insertions(+), 58 deletions(-)
diff --git a/NEWS b/NEWS
index ff8904b02..ac73d6293 100644
--- a/NEWS
+++ b/NEWS
@@ -16,6 +16,12 @@ Post-v3.0.0
by specifying 'max-rate' or '[r]stp-path-cost' accordingly.
- ovs-dpctl and related ovs-appctl commands:
* "flush-conntrack" is capable of handling partial 5-tuple.
+ - OpenFlow:
+ * New OpenFlow extension NXT_CT_FLUSH to flush connections matching
+ the specified fields.
+ - ovs-ofctl:
+ * New command "flush-conntrack" that accepts zone and 5-tuple or partial
+ 5-tuple.
v3.0.0 - 15 Aug 2022
diff --git a/include/openflow/nicira-ext.h b/include/openflow/nicira-ext.h
index b68804991..32ce56d31 100644
--- a/include/openflow/nicira-ext.h
+++ b/include/openflow/nicira-ext.h
@@ -1064,4 +1064,34 @@ struct nx_zone_id {
};
OFP_ASSERT(sizeof(struct nx_zone_id) == 8);
+/* CT flush available TLVs. */
+enum nx_ct_flush_tlv_type {
+/* Outer types. */
+NXT_CT_ORIG_DIRECTION,/* CT orig direction outer type. */
+NXT_CT_REPLY_DIRECTION, /* CT reply direction outer type. */
+
+/* Nested types. */
+NXT_CT_SRC, /* CT source IPv6 or mapped IPv4 address. */
+NXT_CT_DST, /* CT destination IPv6 or mapped IPv4 address. */
+NXT_CT_SRC_PORT, /* CT source port. */
+NXT_CT_DST_PORT, /* CT destination port. */
+NXT_CT_ICMP_ID, /* CT ICMP id. */
+NXT_CT_ICMP_TYPE, /* CT ICMP type. */
+NXT_CT_ICMP_CODE, /* CT ICMP code. */
+
+/* Primitive types. */
+NXT_CT_ZONE_ID, /* CT zone id. */
+};
+
+/* NXT_CT_FLUSH.
+ *
+ * Flushes the connection tracking specified by 5-tuple.
+ * The struct should be followed by TLVs specifying the matching parameters. */
+struct nx_ct_flush {
+uint8_t ip_proto; /* IP protocol. */
+uint8_t family;/* L3 address family. */
+uint8_t zero[6]; /* Must be zero. */
+};
+OFP_ASSERT(sizeof(struct nx_ct_flush) == 8);
+
#endif /* openflow/nicira-ext.h */
diff --git a/include/openvswitch/ofp-msgs.h b/include/openvswitch/ofp-msgs.h
index 921a937e5..659b0a3e7 100644
--- a/include/openvswitch/ofp-msgs.h
+++ b/include/openvswitch/ofp-msgs.h
@@ -526,6 +526,9 @@ enum ofpraw {
/* NXST 1.0+ (4): struct nx_ipfix_stats_reply[]. */
OFPRAW_NXST_IPFIX_FLOW_REPLY,
+
+/* NXT 1.0+ (32): struct nx_ct_flush, uint8_t[8][]. */
+OFPRAW_NXT_CT_FLUSH,
};
/* Decoding messages into OFPRAW_* values. */
@@ -772,6 +775,7 @@ enum ofptype {
OFPTYPE_IPFIX_FLOW_STATS_REQUEST, /* OFPRAW_NXST_IPFIX_FLOW_REQUEST */
OFPTYPE_IPFIX_FLOW_STATS_REPLY, /* OFPRAW_NXST_IPFIX_FLOW_REPLY */
OFPTYPE_CT_FLUSH_ZONE,/* OFPRAW_NXT_CT_FLUSH_ZONE. */
+OFPTYPE_CT_FLUSH, /* OFPRAW_NXT_CT_FLUSH. */
/* Flow monitor extension. */
OFPTYPE_FLOW_MONITOR_CANCEL, /* OFPRAW_NXT_FLOW_MONITOR_CANCEL.
diff --git a/include/openvswitch/ofp-util.h b/include/openvswitch/ofp-util.h
index 84937ae26..e10d90b9f 100644
--- a/include/openvswitch/ofp-util.h
+++ b/include/openvswitch/ofp-util.h
@@ -65,6 +65,10 @@ struct ofpbuf *ofputil_encode_echo_reply(const struct
ofp_header *);
struct ofpbuf *ofputil_encode_barrier_request(enum ofp_version);
+struct ofpbuf *ofputil_ct_match_encode(const struct ofputil_ct_match *match,
+ uint16_t *zone_id,
+ enum ofp_version version);
+
#ifdef __cplusplus
}
#endif
diff --git a/lib/ofp-bundle.c b/lib/ofp-bundle.c
index 0161c2bc6..941a8370e 100644
--- a/lib/ofp-bundle.c
+++ b/lib/ofp-bundle.c
