[ovs-dev] [PATCH RFC v2 1/8] match: Add match_set_ct_zone_masked helper

Simon Horman simon.horman at netronome.com
Fri Jul 26 09:18:23 UTC 2019


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 at 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
> 


More information about the dev mailing list