On Sun, Nov 04, 2018 at 01:22:34AM -0700, Ori Kam wrote: > > > -----Original Message----- > > From: Yongseok Koh > > Sent: Friday, November 2, 2018 11:08 PM > > To: Shahaf Shuler <shah...@mellanox.com> > > Cc: dev@dpdk.org; Yongseok Koh <ys...@mellanox.com>; Ori Kam > > <or...@mellanox.com> > > Subject: [PATCH 2/3] net/mlx5: fix Direct Verbs flow tunnel > > > > 1) Fix layer parsing > > In translation of tunneled flows, dev_flow->layers must not be used to > > check tunneled layer as it contains all the layers parsed from > > flow_drv_prepare(). Checking tunneled layer is needed to distinguish > > between outer and inner item. This should be based on dynamic parsing. With > > dev_flow->layers on a tunneled flow, items will always be interpreted as > > inner as dev_flow->layer already has all the items. Dynamic parsing > > (item_flags) is added as there's no such code. > > > > 2) Refactoring code > > - flow_dv_create_item() and flow_dv_create_action() are merged into > > flow_dv_translate() for consistency with Verbs and *_validate(). > > I don't like the idea of combining 2 distinct functions into one. > I think a function should be as short as possible and do only one thing, > if there is no good reason why two functions should be combined they should > not > be combined. > If you want to align both the Direct Verbs and Verbs I think we can split > the Verbs > code.
Look at the other lengthy switch-case clauses in validate/prepare/translate in each driver. This DV translate is the only exception. I'd rather like to ask why. I didn't like the lengthy function from the beginning but you wanted to keep it. Of course, I considered to split the Verbs one but that's the reason why I chose to merge DV code. If we feel this lengthy func is really complex and gets error prone, then we can refactor all the code at once later. Or, I still prefer the graph approach. That would be simpler. Thanks, Yongseok
