On 28 February 2017 at 17:17, Jarno Rajahalme <[email protected]> wrote:
> Upstream commit:
>
> Refactoring conntrack labels initialization makes changes in later
> patches easier to review.
>
> Signed-off-by: Jarno Rajahalme <[email protected]>
> Acked-by: Pravin B Shelar <[email protected]>
> Acked-by: Joe Stringer <[email protected]>
> Signed-off-by: David S. Miller <[email protected]>
>
> Signed-off-by: Jarno Rajahalme <[email protected]>
> ---
> datapath/conntrack.c | 113
> ++++++++++++++++++++++++++++++---------------------
> 1 file changed, 66 insertions(+), 47 deletions(-)
>
> diff --git a/datapath/conntrack.c b/datapath/conntrack.c
> index dacf34c..adc4315 100644
> --- a/datapath/conntrack.c
> +++ b/datapath/conntrack.c
> @@ -243,19 +243,12 @@ int ovs_ct_put_key(const struct sw_flow_key *key,
> struct sk_buff *skb)
> return 0;
> }
>
> -static int ovs_ct_set_mark(struct sk_buff *skb, struct sw_flow_key *key,
> +static int ovs_ct_set_mark(struct nf_conn *ct, struct sw_flow_key *key,
> u32 ct_mark, u32 mask)
> {
> #if IS_ENABLED(CONFIG_NF_CONNTRACK_MARK)
> - enum ip_conntrack_info ctinfo;
> - struct nf_conn *ct;
> u32 new_mark;
>
> - /* The connection could be invalid, in which case set_mark is no-op.
> */
> - ct = nf_ct_get(skb, &ctinfo);
> - if (!ct)
> - return 0;
> -
> new_mark = ct_mark | (ct->mark & ~(mask));
> if (ct->mark != new_mark) {
> ct->mark = new_mark;
> @@ -270,56 +263,71 @@ static int ovs_ct_set_mark(struct sk_buff *skb, struct
> sw_flow_key *key,
> #endif
> }
>
> -static int ovs_ct_set_labels(struct sk_buff *skb, struct sw_flow_key *key,
> - const struct ovs_key_ct_labels *labels,
> - const struct ovs_key_ct_labels *mask)
> +static struct nf_conn_labels *ovs_ct_get_conn_labels(struct nf_conn *ct)
> {
> - enum ip_conntrack_info ctinfo;
> struct nf_conn_labels *cl;
> - struct nf_conn *ct;
> -
> - /* The connection could be invalid, in which case set_label is
> no-op.*/
> - ct = nf_ct_get(skb, &ctinfo);
> - if (!ct)
> - return 0;
>
> cl = nf_ct_labels_find(ct);
> if (!cl) {
> nf_ct_labels_ext_add(ct);
> cl = nf_ct_labels_find(ct);
> }
> + if (cl && ovs_ct_get_labels_len(cl) < OVS_CT_LABELS_LEN)
> + return NULL;
The above two lines were not introduced in the upstream code. Do you
intend to introduce them?
For my current working tree for review/build/test, I will drop these lines.
> + return cl;
> +}
> +
> +/* Initialize labels for a new, yet to be committed conntrack entry. Note
> that
> + * since the new connection is not yet confirmed, and thus no-one else has
> + * access to it's labels, we simply write them over.
> + */
> +static int ovs_ct_init_labels(struct nf_conn *ct, struct sw_flow_key *key,
> + const struct ovs_key_ct_labels *labels,
> + const struct ovs_key_ct_labels *mask)
> +{
> + struct nf_conn_labels *cl;
> + u32 *dst;
> + int i;
>
> - if (!cl || ovs_ct_get_labels_len(cl) < OVS_CT_LABELS_LEN)
> + cl = ovs_ct_get_conn_labels(ct);
> + if (!cl)
> return -ENOSPC;
>
> - if (nf_ct_is_confirmed(ct)) {
> - /* Triggers a change event, which makes sense only for
> - * confirmed connections.
> - */
> - int err = nf_connlabels_replace(ct, labels->ct_labels_32,
> - mask->ct_labels_32,
> - OVS_CT_LABELS_LEN_32);
> - if (err)
> - return err;
> - } else {
> - u32 *dst = (u32 *)cl->bits;
> - const u32 *msk = mask->ct_labels_32;
> - const u32 *lbl = labels->ct_labels_32;
> - int i;
> + dst = (u32 *)cl->bits;
> + for (i = 0; i < OVS_CT_LABELS_LEN_32; i++)
> + dst[i] = (dst[i] & ~mask->ct_labels_32[i]) |
> + (labels->ct_labels_32[i] & mask->ct_labels_32[i]);
>
> - /* No-one else has access to the non-confirmed entry, copy
> - * labels over, keeping any bits we are not explicitly
> setting.
> - */
> - for (i = 0; i < OVS_CT_LABELS_LEN_32; i++)
> - dst[i] = (dst[i] & ~msk[i]) | (lbl[i] & msk[i]);
> + /* Labels are included in the IPCTNL_MSG_CT_NEW event only if the
> + * IPCT_LABEL bit it set in the event cache.
> + */
> + nf_conntrack_event_cache(IPCT_LABEL, ct);
>
> - /* Labels are included in the IPCTNL_MSG_CT_NEW event only if
> - * the IPCT_LABEL bit it set in the event cache.
> - */
> - nf_conntrack_event_cache(IPCT_LABEL, ct);
> - }
> + memcpy(&key->ct.labels, cl->bits, OVS_CT_LABELS_LEN);
> +
> + return 0;
> +}
> +
> +static int ovs_ct_set_labels(struct nf_conn *ct, struct sw_flow_key *key,
> + const struct ovs_key_ct_labels *labels,
> + const struct ovs_key_ct_labels *mask)
> +{
> + struct nf_conn_labels *cl;
> + int err;
> +
> + cl = ovs_ct_get_conn_labels(ct);
> + if (!cl)
> + return -ENOSPC;
> +
> + err = nf_connlabels_replace(ct, labels->ct_labels_32,
> + mask->ct_labels_32,
> + OVS_CT_LABELS_LEN_32);
> + if (err)
> + return err;
> +
> + memcpy(&key->ct.labels, cl->bits, OVS_CT_LABELS_LEN);
>
> - ovs_ct_get_labels(ct, &key->ct.labels);
> return 0;
> }
>
> @@ -926,25 +934,36 @@ static int ovs_ct_commit(struct net *net, struct
> sw_flow_key *key,
> const struct ovs_conntrack_info *info,
> struct sk_buff *skb)
> {
> + enum ip_conntrack_info ctinfo;
> + struct nf_conn *ct;
> int err;
>
> err = __ovs_ct_lookup(net, key, info, skb);
> if (err)
> return err;
>
> + /* The connection could be invalid, in which case this is a no-op.*/
> + ct = nf_ct_get(skb, &ctinfo);
> + if (!ct)
> + return 0;
> +
> /* Apply changes before confirming the connection so that the initial
> * conntrack NEW netlink event carries the values given in the CT
> * action.
> */
> if (info->mark.mask) {
> - err = ovs_ct_set_mark(skb, key, info->mark.value,
> + err = ovs_ct_set_mark(ct, key, info->mark.value,
> info->mark.mask);
> if (err)
> return err;
> }
> if (labels_nonzero(&info->labels.mask)) {
> - err = ovs_ct_set_labels(skb, key, &info->labels.value,
> - &info->labels.mask);
> + if (!nf_ct_is_confirmed(ct))
> + err = ovs_ct_init_labels(ct, key, &info->labels.value,
> + &info->labels.mask);
> + else
> + err = ovs_ct_set_labels(ct, key, &info->labels.value,
> + &info->labels.mask);
> if (err)
> return err;
> }
> --
> 2.1.4
>
> _______________________________________________
> dev mailing list
> [email protected]
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
