Re: [PATCHv3 net-next 06/10] openvswitch: Allow matching on conntrack mark

On Wed, Aug 12, 2015 at 4:41 PM, Joe Stringer [off-list ref] wrote:

On 12 August 2015 at 16:00, Pravin Shelar [off-list ref] wrote:

quoted

On Tue, Aug 11, 2015 at 3:59 PM, Joe Stringer [off-list ref] wrote:

quoted

From: Justin Pettit <redacted>

Allow matching and setting the conntrack mark field. As with conntrack
state and zone, these are populated by executing the ct() action. Unlike
these, the ct_mark is also a writable field. The set_field() action may
be used to modify the mark, which will take effect on the most recent
conntrack entry.

E.g.: actions:ct(zone=0),ct(zone=1),set_field(1->ct_mark)

This will perform conntrack lookup in zone 0, then lookup in zone 1,
then modify the mark for the entry in zone 1. The mark for the entry in
zone 0 is unchanged. The conntrack entry itself must be committed using
the "commit" flag in the conntrack action flags for this change to persist.

Signed-off-by: Justin Pettit <redacted>
Signed-off-by: Joe Stringer <redacted>
---

...

quoted

+int ovs_ct_set_mark(struct sk_buff *skb, struct sw_flow_key *key,
+                   u32 ct_mark, u32 mask)
+{
+#ifdef CONFIG_NF_CONNTRACK_MARK
+       enum ip_conntrack_info ctinfo;
+       struct nf_conn *ct;
+       u32 new_mark;
+
+       /* This must happen directly after lookup/commit. */
+       ct = nf_ct_get(skb, &ctinfo);
+       if (!ct)
+               return -EINVAL;
+
+       new_mark = ct_mark | (ct->mark & ~(mask));
+       if (ct->mark != new_mark) {
+               ct->mark = new_mark;
+               nf_conntrack_event_cache(IPCT_MARK, ct);
+               key->ct.mark = ct_mark;
+       }
+

Is it fine to set just set mark and not initialize reset of key->ct members?

I don't quite follow. This action acts upon the current connection,
and modifies its metadata. key->ct should already be populated with
the existing connection info.

I had offline discussion with Joe. The fields are initialized in prior
conntrack action. So now he is exploring if we can bring conntrack,
set mark and set lable actions under one single conntrack action using
parameters.