On Sun, Dec 01, 2019 at 07:38:35AM +0000, Paul Blakey wrote:
> 
> On 11/27/2019 4:07 PM, Marcelo Ricardo Leitner wrote:
> > On Wed, Nov 27, 2019 at 02:55:09PM +0200, Roi Dayan wrote: >> From: Paul 
> > Blakey <[email protected]><mailto:[email protected]> >> >> Move all that 
> > is needed to identify a tc filter to a new structure, >> tc_id. This 
> > removes a lot of duplication in accessing/creating tc >> filters. > > Ok, 
> > gotta say, 'tc_id' is a bit confusing here. Every time I read > it, I try 
> > to find a correlating id in the kernel land but it doesn't > exist, because 
> > 'tc' itself refers to a much bigger thing and not > just filters. > > That 
> > said, I was thinking, what about 'tcf_id', or even a longer > one, 
> > 'tc_filter_id'? But before answering, please read below. :-)
> I don't mind either way, but from this I'd rater have the short one tcf_id.
> 
> > >> >> --- >> >> Changelog: V1->V2: In tc_del_matchall_policer - reverse 
> > >> >> xmas param >> order Added and used helper is_tc_id_eq(id1, id2) in 
> > >> >> find_ufid In >> netdev_tc_flow_dump_next - use make_tc_id() instead 
> > >> >> of manualy >> filling id > ^^^^^^^^^^ > >> In netdev_tc_flow_put - 
> > >> >> use if (get_ufid_tc_mapping(ufid, &id) == >> 0) to be mor explict we 
> > >> >> found the mapping not failed to get In >> make_tc_id - fill id 
> > >> >> explictily and removed memset. Moved >> tc_make_id to be static 
> > >> >> inline in tc.h > ^^^^^^^^^^ > > The patch is currently using 
> > >> >> make_tc_id. I see other functions > already using the latter one, 
> > >> >> tc_make_*, like tc_make_handle and > tc_make_request. Though it also 
> > >> >> already has make_tc_id_chain. Which > then, with this last one, it 
> > >> >> gets easier to understand what a 'tc_id' > means. > > I don't have a 
> > >> >> strong opinion here, so I'm just highlighting the > topic.
> Not sure what you meant here.
> 
> 
> Would you rather  we have tc_make_tcf_id() and tc_make_tcf_id_chain() (in 
> later patch) to be more consistent?

Yes. That seems the best to me.

Thanks,
  Marcelo

_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to