Sun, Jul 28, 2019 at 08:52:47AM CEST, we...@ucloud.cn wrote:
>From: wenxu <we...@ucloud.cn>
>
>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 <we...@ucloud.cn>
>---
>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.


>+};
>+
>+struct flow_indr_block_dev *flow_indr_block_dev_lookup(struct net_device 
>*dev);
>+
>+int __flow_indr_block_cb_register(struct net_device *dev, void *cb_priv,
>+                                flow_indr_block_bind_cb_t *cb, void 
>*cb_ident);
>+
>+void __flow_indr_block_cb_unregister(struct net_device *dev,
>+                                   flow_indr_block_bind_cb_t *cb, void 
>*cb_ident);
>+
>+int flow_indr_block_cb_register(struct net_device *dev, void *cb_priv,
>+                              flow_indr_block_bind_cb_t *cb, void *cb_ident);
>+
>+void flow_indr_block_cb_unregister(struct net_device *dev,
>+                                 flow_indr_block_bind_cb_t *cb, void 
>*cb_ident);
>+
        
[...]

        
>+
>+int __flow_indr_block_cb_register(struct net_device *dev, void *cb_priv,
>+                                flow_indr_block_bind_cb_t *cb,
>+                                void *cb_ident)
>+{
>+      struct flow_indr_block_cb *indr_block_cb;
>+      struct flow_indr_block_dev *indr_dev;
>+      int err;
>+
>+      indr_dev = flow_indr_block_dev_get(dev);
>+      if (!indr_dev)
>+              return -ENOMEM;
>+
>+      indr_block_cb = flow_indr_block_cb_add(indr_dev, cb_priv, cb, cb_ident);
>+      err = PTR_ERR_OR_ZERO(indr_block_cb);
>+      if (err)
>+              goto err_dev_put;
>+
>+      if (indr_dev->ing_cmd_cb)
>+              indr_dev->ing_cmd_cb(indr_dev->dev, indr_dev->flow_block, 
>indr_block_cb,

This line is over 80cols. Please run checkpatch script for your patch
and obey the warnings.


>+                                   FLOW_BLOCK_BIND);
>+
>+      return 0;
>+
>+err_dev_put:
>+      flow_indr_block_dev_put(indr_dev);
>+      return err;
>+}
>+EXPORT_SYMBOL_GPL(__flow_indr_block_cb_register);

[...]


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


>+                                struct flow_block *flow_block,
>+                                struct flow_indr_block_cb *indr_block_cb,
>                                 enum flow_block_command command)
> {
>+      struct tcf_block *block = flow_block ?
>+                                container_of(flow_block,
>+                                             struct tcf_block,
>+                                             flow_block) : NULL;
>       struct flow_block_offload bo = {
>               .command        = command,
>               .binder_type    = FLOW_BLOCK_BINDER_TYPE_CLSACT_INGRESS,
>-              .net            = dev_net(indr_dev->dev),
>-              .block_shared   = tcf_block_non_null_shared(indr_dev->block),
>+              .net            = dev_net(dev),
>+              .block_shared   = tcf_block_non_null_shared(block),
>       };
>       INIT_LIST_HEAD(&bo.cb_list);
> 

[...]

Reply via email to