netdev - Re: [PATCH/RFC repost 4/8] datapath: execution of select group action

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <20140924081942.GB24298@casper.infradead.org>
Date:	Wed, 24 Sep 2014 09:19:42 +0100
From:	Thomas Graf <tgraf@...g.ch>
To:	Simon Horman <simon.horman@...ronome.com>
Cc:	dev@...nvswitch.org, netdev@...r.kernel.org,
	Pravin Shelar <pshelar@...ira.com>,
	Jesse Gross <jesse@...ira.com>
Subject: Re: [PATCH/RFC repost 4/8] datapath: execution of select group action

On 09/24/14 at 03:01pm, Simon Horman wrote:
> > > +	/* Only possible type of attributes is OVS_SELECT_GROUP_ATTR_BUCKET */
> > > +	for (bucket = nla_data(attr), rem = nla_len(attr); rem > 0;
> > > +	     bucket = nla_next(bucket, &rem)) {
> > > +		uint16_t weight = bucket_weight(bucket);
> > 
> > I think we should validate only once when we copy then assume it is
> > correct.

> That is the intention of this code, is it doing something else?

I must have treated the BUG_ON as validation when I wrote that
comment ;-)

> Nice idea. I think that would work out quite well.
> 
> The main question for me would be where to store such a table,
> which comes back to my remark above about more storing a more
> efficient efficient form of the action.

I had the same issue when working on zone support for the 
conntrack action that is in the works. It needs to keep a ref to
a ct template which is registered with the conntrack engine. I
ended up converting the Netlink attributes to a struct and storing
that in an attribute in sf_acts then converting it back.

You can find the WIP code in the following branch:
https://github.com/tgraf/ovs/tree/nat

datapath: Use central function to free sw_flow_actions 
RFC: Add support for connection tracking. 
datapath: Add zone support

> > Do we need a recursion limit here?
> 
> I believe that is already handled by the depth check that occurs
> when the actions are copied.

Right, I can see it now. Thanks.

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html