Mon, Jul 29, 2019 at 02:47:07PM CEST, wenxu@xxxxxxxxx wrote:
>
>在 2019/7/29 19:13, Jiri Pirko 写道:
>> Sun, Jul 28, 2019 at 08:52:47AM CEST, wenxu@xxxxxxxxx wrote:
>>> From: wenxu <wenxu@xxxxxxxxx>
>>>
>>> move tc indirect block to flow_offload and rename
>>> it to flow indirect block.The nf_tables can use the
>>> indr block architecture.
>>>
>>> Signed-off-by: wenxu <wenxu@xxxxxxxxx>
>>> ---
>>> v3: subsys_initcall for init_flow_indr_rhashtable
>>> v4: no change
>>>
>> [...]
>>
>>
>>> diff --git a/include/net/flow_offload.h b/include/net/flow_offload.h
>>> index 00b9aab..66f89bc 100644
>>> --- a/include/net/flow_offload.h
>>> +++ b/include/net/flow_offload.h
>>> @@ -4,6 +4,7 @@
>>> #include <linux/kernel.h>
>>> #include <linux/list.h>
>>> #include <net/flow_dissector.h>
>>> +#include <linux/rhashtable.h>
>>>
>>> struct flow_match {
>>> struct flow_dissector *dissector;
>>> @@ -366,4 +367,42 @@ static inline void flow_block_init(struct flow_block *flow_block)
>>> INIT_LIST_HEAD(&flow_block->cb_list);
>>> }
>>>
>>> +typedef int flow_indr_block_bind_cb_t(struct net_device *dev, void *cb_priv,
>>> + enum tc_setup_type type, void *type_data);
>>> +
>>> +struct flow_indr_block_cb {
>>> + struct list_head list;
>>> + void *cb_priv;
>>> + flow_indr_block_bind_cb_t *cb;
>>> + void *cb_ident;
>>> +};
>> I don't understand why are you pushing this struct out of the c file to
>> the header. Please don't.
>>
>>
>>> +
>>> +typedef void flow_indr_block_ing_cmd_t(struct net_device *dev,
>>> + struct flow_block *flow_block,
>>> + struct flow_indr_block_cb *indr_block_cb,
>>> + enum flow_block_command command);
>>> +
>>> +struct flow_indr_block_dev {
>>> + struct rhash_head ht_node;
>>> + struct net_device *dev;
>>> + unsigned int refcnt;
>>> + struct list_head cb_list;
>>> + flow_indr_block_ing_cmd_t *ing_cmd_cb;
>>> + struct flow_block *flow_block;
>> I don't understand why are you pushing this struct out of the c file to
>> the header. Please don't.
>
>the flow_indr_block_dev and indr_block_cb in the h file used for the function
You don't need it, same as before. Please don't expose this struct.
>
>tc_indr_block_ing_cmd in cls_api.c
>
>>> -static void tc_indr_block_ing_cmd(struct tc_indr_block_dev *indr_dev,
>>> - struct tc_indr_block_cb *indr_block_cb,
>>> +static void tc_indr_block_ing_cmd(struct net_device *dev,
>> I don't understand why you change struct tc_indr_block_dev * to
>> struct net_device * here. If you want to do that, please do that in a
>> separate patch, not it this one where only "the move" should happen.
Did you see the rest of my comments???
>>
