On 16-02-22 06:57 PM, Cong Wang wrote:

[..]


diff --git a/include/net/act_api.h b/include/net/act_api.h
index 8c4e3ff..342be6c 100644
--- a/include/net/act_api.h
+++ b/include/net/act_api.h
@@ -7,6 +7,8 @@

  #include <net/sch_generic.h>
  #include <net/pkt_sched.h>
+#include <net/net_namespace.h>
+#include <net/netns/generic.h>

  struct tcf_common {
        struct hlist_node               tcfc_head;
@@ -87,31 +89,65 @@ struct tc_action {
        __u32                   type; /* for backward compat(TCA_OLD_COMPAT) */
        __u32                   order;
        struct list_head        list;
+       struct tcf_hashinfo     *hinfo;
  };


It doesnt seem neccessary to have hinfo in tc_action. Quick scan:
__tcf_hash_release() seems to be the only other place that uses it.
And the callers to that appear capable of passing the struct
net or tn  which eventually propagates up...

  struct tc_action_ops {
        struct list_head head;
-       struct tcf_hashinfo *hinfo;
        char    kind[IFNAMSIZ];
        __u32   type; /* TBD to match kind */
        struct module           *owner;
        int     (*act)(struct sk_buff *, const struct tc_action *, struct 
tcf_result *);
        int     (*dump)(struct sk_buff *, struct tc_action *, int, int);
        void    (*cleanup)(struct tc_action *, int bind);
-       int     (*lookup)(struct tc_action *, u32);
+       int     (*lookup)(struct net *, struct tc_action *, u32);
        int     (*init)(struct net *net, struct nlattr *nla,
                        struct nlattr *est, struct tc_action *act, int ovr,
                        int bind);
-       int     (*walk)(struct sk_buff *, struct netlink_callback *, int, 
struct tc_action *);
+       int     (*walk)(struct net *, struct sk_buff *,
+                       struct netlink_callback *, int, struct tc_action *);
+};
+

Do you really need to pass struct net to walk(); is deriving from skb
not sufficient?


+       int err = 0;


diff --git a/net/sched/act_api.c b/net/sched/act_api.c
index acafaf7..9606666 100644
--- a/net/sched/act_api.c
+++ b/net/sched/act_api.c
@@ -36,10 +36,9 @@ static void free_tcf(struct rcu_head *head)
        kfree(p);
  }

-static void tcf_hash_destroy(struct tc_action *a)
+static void tcf_hash_destroy(struct tcf_hashinfo *hinfo, struct tc_action *a)
  {
        struct tcf_common *p = a->priv;
-       struct tcf_hashinfo *hinfo = a->ops->hinfo;

        spin_lock_bh(&hinfo->lock);
        hlist_del(&p->tcfc_head);
@@ -68,7 +67,7 @@ int __tcf_hash_release(struct tc_action *a, bool bind, bool 
strict)
                if (p->tcfc_bindcnt <= 0 && p->tcfc_refcnt <= 0) {
                        if (a->ops->cleanup)
                                a->ops->cleanup(a, bind);
-                       tcf_hash_destroy(a);
+                       tcf_hash_destroy(a->hinfo, a);


So this seems to be the only place where a->hinfo is read from. The
rest seems to just set a->hinfo.
I took a quick look at __tcf_hash_release() and all calling sites
are net/tn aware already.

-u32 tcf_hash_new_index(struct tcf_hashinfo *hinfo)
+u32 tcf_hash_new_index(struct tc_action_net *tn)
  {
+       struct tcf_hashinfo *hinfo = tn->hinfo;
        u32 val = hinfo->index;



That also seemed unneeded. You could have derived hinfo
from tn.

Otherwise looks reasonable. I was hoping we could get rid of the per
action pernet ops but that could come later.

cheers,
jamal

Reply via email to