Wed, Sep 27, 2017 at 10:29:35AM CEST, pab...@redhat.com wrote: >Hi, > >Moving to a separate theread, since I think this is more related to the >flower core infrastructure than to the netrome patches. > >On Wed, 2017-09-27 at 09:40 +0200, Jiri Pirko wrote: >> This kind of hooks are giving me nightmares. The code is screwed up as >> it is already. I'm currently working on conversion to callbacks. This >> part is handled in: >> https://github.com/jpirko/linux_mlxsw/commits/jiri_devel_egdevcb > >Thanks for the pointer. > >I skimmed quickly on the code and indeed it cleans this area a lot. >If I read it correctly the ('good') command: > >tc filter add dev vxlan0 protocol ip parent ffff: flower enc_key_id 102 > enc_dst_port 4789 src_ip 3.4.5.6 skip_sw action [...]
I suppose "action mirred redirect eth0". Then yes, it will generate the callpath you described below. > >will generate a call to: > >mlx5e_setup_tc(eth0, TC_SETUP_CLSFLOWER, &cls_flower) via: > >fl_hw_replace_filter() -> > tc_setup_cb_call() -> > tc_exts_setup_cb_egdev_call() -> > tc_setup_cb_egdev_call() -> > tcf_action_egdev_cb_call() -> > mlx5e_rep_setup_tc_cb() > >and the 'bad' command: > >tc filter add dev eth0 protocol ip parent ffff: flower enc_key_id 102 \ > enc_dst_port 4789 src_ip 3.4.5.6 skip_sw action [...] > >will also call: > >mlx5e_setup_tc(eth0, TC_SETUP_CLSFLOWER, &cls_flower) via: > >fl_hw_replace_filter() -> > ndo_setup_tc() Sure. You are adding a rule to eth0, the call goes down to eth0 driver. I'm missing why is it a problem? Why the call should not go down to the eth0 driver? > >So it looks like the H/W offload hook will still be called with the >same arguments in both case, and 'bad' rule will still be pushed to the >H/W as the driver itself has no way to distinct between the two >scenarios. Why "bad"? Regarding the distinction, driver knows if user add a rule directly to the eth0, or if the eth0 is egress device in the action. Those are 2 separete driver entrypoints - of course, talking about code with my changes. > >[ Note: I referred to the mlx hook just for convenience, should be the >same with any driver implementing the same APIs ] > >Am I missing something? > >Thanks, > >Paolo
