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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:	Thu, 13 Aug 2015 10:14:02 -0700
From:	Pravin Shelar <pshelar@...ira.com>
To:	Joe Stringer <joestringer@...ira.com>
Cc:	netdev <netdev@...r.kernel.org>,
	Justin Pettit <jpettit@...ira.com>,
	LKML <linux-kernel@...r.kernel.org>, pablo <pablo@...filter.org>,
	Patrick McHardy <kaber@...sh.net>,
	Andy Zhou <azhou@...ira.com>, Jesse Gross <jesse@...ira.com>,
	Florian Westphal <fwestpha@...hat.com>,
	Hannes Sowa <hannes@...hat.com>,
	Thomas Graf <tgraf@...ronetworks.com>
Subject: Re: [PATCHv3 net-next 06/10] openvswitch: Allow matching on conntrack mark

On Wed, Aug 12, 2015 at 4:41 PM, Joe Stringer <joestringer@...ira.com> wrote:
> On 12 August 2015 at 16:00, Pravin Shelar <pshelar@...ira.com> wrote:
>> On Tue, Aug 11, 2015 at 3:59 PM, Joe Stringer <joestringer@...ira.com> wrote:
>>> From: Justin Pettit <jpettit@...ira.com>
>>>
>>> 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 <jpettit@...ira.com>
>>> Signed-off-by: Joe Stringer <joestringer@...ira.com>
>>> ---
>> ...
>>
>>>
>>> +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.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/