[ovs-dev,RFC,v2,1/8] match: Add match_set_ct_zone_masked helper
diff mbox series

Message ID 1562250507-20335-2-git-send-email-paulb@mellanox.com
State New
Headers show
Series
  • Introduce connection tracking tc offload
Related show

Commit Message

Paul Blakey July 4, 2019, 2:28 p.m. UTC
Sets zone in match.

Signed-off-by: Paul Blakey <paulb@mellanox.com>
---
 include/openvswitch/match.h |  1 +
 lib/match.c                 | 10 ++++++++--
 2 files changed, 9 insertions(+), 2 deletions(-)

Comments

Simon Horman July 26, 2019, 9:18 a.m. UTC | #1
On Thu, Jul 04, 2019 at 05:28:20PM +0300, Paul Blakey wrote:
> Sets zone in match.

I think it would be good if a longer changelog was provided.
Which included some "why", which is not present in the code,
as well as "what", which is present in the code.

This comment applies to the entire series.

Also, I believe this description of this patch is misleading as
match_set_ct_zone seems to already do what is described in the changelog.

As for the code change made by this patch, that looks fine to me.

> 
> Signed-off-by: Paul Blakey <paulb@mellanox.com>
> ---
>  include/openvswitch/match.h |  1 +
>  lib/match.c                 | 10 ++++++++--
>  2 files changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/include/openvswitch/match.h b/include/openvswitch/match.h
> index 05ecee7..219de00 100644
> --- a/include/openvswitch/match.h
> +++ b/include/openvswitch/match.h
> @@ -127,6 +127,7 @@ void match_set_pkt_mark_masked(struct match *, uint32_t pkt_mark, uint32_t mask)
>  void match_set_ct_state(struct match *, uint32_t ct_state);
>  void match_set_ct_state_masked(struct match *, uint32_t ct_state, uint32_t mask);
>  void match_set_ct_zone(struct match *, uint16_t ct_zone);
> +void match_set_ct_zone_masked(struct match *match, uint16_t ct_zone, uint16_t mask);
>  void match_set_ct_mark(struct match *, uint32_t ct_mark);
>  void match_set_ct_mark_masked(struct match *, uint32_t ct_mark, uint32_t mask);
>  void match_set_ct_label(struct match *, ovs_u128 ct_label);
> diff --git a/lib/match.c b/lib/match.c
> index 052daee..5415251 100644
> --- a/lib/match.c
> +++ b/lib/match.c
> @@ -417,8 +417,14 @@ match_set_ct_state_masked(struct match *match, uint32_t ct_state, uint32_t mask)
>  void
>  match_set_ct_zone(struct match *match, uint16_t ct_zone)
>  {
> -    match->flow.ct_zone = ct_zone;
> -    match->wc.masks.ct_zone = UINT16_MAX;
> +    match_set_ct_zone_masked(match, ct_zone, UINT16_MAX);
> +}
> +
> +void
> +match_set_ct_zone_masked(struct match *match, uint16_t ct_zone, uint16_t mask)
> +{
> +    match->flow.ct_zone = ct_zone & mask;
> +    match->wc.masks.ct_zone = mask;
>  }
>  
>  void
> -- 
> 1.8.3.1
>

Patch
diff mbox series

diff --git a/include/openvswitch/match.h b/include/openvswitch/match.h
index 05ecee7..219de00 100644
--- a/include/openvswitch/match.h
+++ b/include/openvswitch/match.h
@@ -127,6 +127,7 @@  void match_set_pkt_mark_masked(struct match *, uint32_t pkt_mark, uint32_t mask)
 void match_set_ct_state(struct match *, uint32_t ct_state);
 void match_set_ct_state_masked(struct match *, uint32_t ct_state, uint32_t mask);
 void match_set_ct_zone(struct match *, uint16_t ct_zone);
+void match_set_ct_zone_masked(struct match *match, uint16_t ct_zone, uint16_t mask);
 void match_set_ct_mark(struct match *, uint32_t ct_mark);
 void match_set_ct_mark_masked(struct match *, uint32_t ct_mark, uint32_t mask);
 void match_set_ct_label(struct match *, ovs_u128 ct_label);
diff --git a/lib/match.c b/lib/match.c
index 052daee..5415251 100644
--- a/lib/match.c
+++ b/lib/match.c
@@ -417,8 +417,14 @@  match_set_ct_state_masked(struct match *match, uint32_t ct_state, uint32_t mask)
 void
 match_set_ct_zone(struct match *match, uint16_t ct_zone)
 {
-    match->flow.ct_zone = ct_zone;
-    match->wc.masks.ct_zone = UINT16_MAX;
+    match_set_ct_zone_masked(match, ct_zone, UINT16_MAX);
+}
+
+void
+match_set_ct_zone_masked(struct match *match, uint16_t ct_zone, uint16_t mask)
+{
+    match->flow.ct_zone = ct_zone & mask;
+    match->wc.masks.ct_zone = mask;
 }
 
 void