Date:	Wed, 12 Nov 2014 22:31:06 -0800
From:	John Fastabend <john.r.fastabend@...el.com>
To:	Simon Horman <simon.horman@...ronome.com>,
	John Fastabend <john.fastabend@...il.com>
CC:	Jamal Hadi Salim <jhs@...atatu.com>, Jiri Pirko <jiri@...nulli.us>,
	netdev@...r.kernel.org, davem@...emloft.net, nhorman@...driver.com,
	andy@...yhouse.net, tgraf@...g.ch, dborkman@...hat.com,
	ogerlitz@...lanox.com, jesse@...ira.com, pshelar@...ira.com,
	azhou@...ira.com, ben@...adent.org.uk, stephen@...workplumber.org,
	jeffrey.t.kirsher@...el.com, vyasevic@...hat.com,
	xiyou.wangcong@...il.com, edumazet@...gle.com, sfeldma@...il.com,
	f.fainelli@...il.com, roopa@...ulusnetworks.com,
	linville@...driver.com, jasowang@...hat.com, ebiederm@...ssion.com,
	nicolas.dichtel@...nd.com, ryazanov.s.a@...il.com,
	buytenh@...tstofly.org, aviadr@...lanox.com, nbd@...nwrt.org,
	alexei.starovoitov@...il.com, Neil.Jerram@...aswitch.com,
	ronye@...lanox.com, alexander.h.duyck@...hat.com,
	john.ronciak@...el.com, mleitner@...hat.com, shrijeet@...il.com,
	gospo@...ulusnetworks.com, bcrl@...ck.org
Subject: Re: [patch net-next v2 00/10] introduce rocker switch driver with
 hardware accelerated datapath api - phase 1: bridge fdb offload

On 11/12/2014 09:44 PM, Simon Horman wrote:
> [snip]
> 
>> Simon, if your feeling adventurous any feedback on the repo link
>> would be great. I still need to smash the commit log into something
>> coherent though at the moment you can see all the errors and rewrites,
>> etc as I made them.
> 
> Hi John,
> 
> here is some preliminary feedback:
> 
> * I notice that the parse graph code isn't present yet.
>   I suppose this is a difficult piece that naturally follows many
>   other piece. None the less it is possibly the piece of most
>   interest to me :-)

I can add this over the next few days. Also I wanted to publish some
more complex examples on top of rocker switch. The nic drivers are
interesting but not as complex as some of the switch devices.

There is also the table graph layout which I wanted tweak a bit. At
the moment I have hardware that can run tables in parallel and some
that executes tables in sequence. It might not be clear from the code
(why I need the cleanup) but the source id is being used to indicate
if the tables are executed in parallel or not.

> 
> * Will del and update flows require flows to already exist?
>   And similarly, will add flow require flows with the same match to not
>   already exist?  If so, the error handling seems tricky of more than one
>   flow is to be deleted/updated. IIRC there was some discussion of that
>   kind of issue at the (double) round table discussion on the last day of
>   LPC14 in Düsseldorf.

I would expect del/updates for flows that don't exist should fail.

I didn't intend to add any checks in the kernel to verify the matches
are unique. My opinion on this is that user space shouldn't add new
duplicate flows. And if it does hardware resources will be wasted.

> 
> * Should the .node_count value of ixgbe_table_node_l2 be 3?
>   ixgbe_table_graph_nodes has three elements but perhaps you
>   are intentionally excluding the last element ixgbe_table_node_nil?
> 

Actually I could just drop the node_count at the moment because I've
been null terminating the arrays with null items.

I should either add a count field to all the structures or null terminate
the arrays. For now I mostly null terminate the arrays when I use
them. For example matches is null terminates same for actions.

.John
--
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

Powered by blists - more mailing lists