Re: [PATCHv4 net-next 09/10] openvswitch: Allow matching on conntrack label

[Pravin Shelar]

On Tue, Aug 18, 2015 at 4:39 PM, Joe Stringer <joestringer@xxxxxxxxxx> wrote:
> Allow matching and setting the conntrack label field. As with ct_mark,
> this is populated by executing the CT action, and is a writable field.
> Specifying a label and optional mask allows the label to be modified,
> which takes effect on the entry found by the lookup of the CT action.
>
> E.g.: actions:ct(zone=1,label=1)
>
> This will perform conntrack lookup in zone 1, then modify the label for
> that entry. The conntrack entry itself must be committed using the
> "commit" flag in the conntrack action flags for this change to persist.
>
> Signed-off-by: Joe Stringer <joestringer@xxxxxxxxxx>
I got compilation error after applying this patch:
net/openvswitch/conntrack.c: In function âovs_ct_initâ:
net/openvswitch/conntrack.c:713: error: expected â)â before â;â token
net/openvswitch/conntrack.c:715: error: expected expression before â}â token

> ---
> v2: Split out setting the connlabel size for the current namespace.
> v3: No change.
> v4: Only allow setting label via ct action.
>     Update documentation.
> ---
...
>                                   type);
> @@ -432,6 +521,10 @@ bool ovs_ct_verify(enum ovs_key_attr attr)
>         if (attr & OVS_KEY_ATTR_CT_MARK)
>                 return true;
>  #endif
> +#ifdef CONFIG_NF_CONNTRACK_LABELS
> +       if (attr & OVS_KEY_ATTR_CT_LABEL)
> +               return true;
> +#endif
>
OVS_KEY_ATTR_CT_LABEL is not bit field so bitwise AND operation does
not work here. This applies to all check done in this function.

>         return false;
>  }
> @@ -508,8 +601,12 @@ void ovs_ct_free_action(const struct nlattr *a)
>
>  void ovs_ct_init(struct net *net, struct ovs_ct_perdp_data *data)
>  {
> +       unsigned int n_bits = sizeof(struct ovs_key_ct_label) * BITS_PER_BYTE;
> +
>         data->xt_v4 = !nf_ct_l3proto_try_module_get(PF_INET);
>         data->xt_v6 = !nf_ct_l3proto_try_module_get(PF_INET6);
> +       if (nf_connlabels_get(net, n_bits);
> +               OVS_NLERR(true, "Failed to set connlabel length");
>  }
>
In case of error should we reject conntrack label actions? Otherwise
user will never see any error. But action could drop packets.

>  void ovs_ct_exit(struct net *net, struct ovs_ct_perdp_data *data)
> @@ -518,4 +615,5 @@ void ovs_ct_exit(struct net *net, struct ovs_ct_perdp_data *data)
>                 nf_ct_l3proto_module_put(PF_INET);
>         if (data->xt_v6)
>                 nf_ct_l3proto_module_put(PF_INET6);
> +       nf_connlabels_put(net);
>  }
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/