Re: [PATCH v3 net-next 1/1] net_sched: Introduce skbmod action

[Jamal Hadi Salim]

On 16-08-29 06:38 AM, Jamal Hadi Salim wrote:

Let me see if i can
try something. Trickery with RCU is not one of my virtues -
so i may get it wrong but hope you can comment.

Something like attached? Compile tested.
I tried to emulate gact - but greping around I havent
seen sample code which uses  rcu_read_un/lock + synchronize_rcu
this way
[so be gentle if i got it wrong ;->]

I am not sure where Cong is going but probably introducing
a rcu head in each policy will allow to swap and free old one
with call_rcu may be a better approach. The cost may come in
slowing down replace operations.

cheers,
jamal

static int tcf_skbmod_run(struct sk_buff *skb, const struct tc_action *a,
                          struct tcf_result *res)
{
        struct tcf_skbmod *d = to_skbmod(a);
        int action = READ_ONCE(d->tcf_action);
        u64 flags = READ_ONCE(d->flags);

        tcf_lastuse_update(&d->tcf_tm);
        bstats_cpu_update(this_cpu_ptr(d->common.cpu_bstats), skb);
        if (unlikely(action == TC_ACT_SHOT)) {
                qstats_drop_inc(this_cpu_ptr(d->common.cpu_qstats));
                return action;
        }

        flags = READ_ONCE(d->flags);
        rcu_read_lock();
        if (flags & SKBMOD_F_DMAC)
                ether_addr_copy(eth_hdr(skb)->h_dest, d->eth_dst);
        if (flags & SKBMOD_F_SMAC)
                ether_addr_copy(eth_hdr(skb)->h_source, d->eth_src);
        if (flags & SKBMOD_F_ETYPE)
                eth_hdr(skb)->h_proto = d->eth_type;
        if (flags & SKBMOD_F_SWAPMAC) {
                u8 tmpaddr[ETH_ALEN];
                /*XXX: I am sure we can come up with something more efficient */
                ether_addr_copy(tmpaddr, eth_hdr(skb)->h_dest);
                ether_addr_copy(eth_hdr(skb)->h_dest, eth_hdr(skb)->h_source);
                ether_addr_copy(eth_hdr(skb)->h_source, tmpaddr);
        }
        rcu_read_unlock();

        return action;
}

static int tcf_skbmod_init(struct net *net, struct nlattr *nla,
                           struct nlattr *est, struct tc_action **a,
                           int ovr, int bind)
{
        struct tc_action_net *tn = net_generic(net, skbmod_net_id);
        struct nlattr *tb[TCA_SKBMOD_MAX + 1];
        struct tc_skbmod *parm;
        struct tcf_skbmod *d;
        u32 lflags = 0;
        u8 *daddr = NULL;
        u8 *saddr = NULL;
        u16 eth_type = 0;

        bool exists = false;
        int ret = 0, err;

        if (nla == NULL)
                return -EINVAL;

        err = nla_parse_nested(tb, TCA_SKBMOD_MAX, nla, skbmod_policy);
        if (err < 0)
                return err;

        if (tb[TCA_SKBMOD_PARMS] == NULL)
                return -EINVAL;

        if (tb[TCA_SKBMOD_DMAC]) {
                daddr = nla_data(tb[TCA_SKBMOD_DMAC]);
                lflags |= SKBMOD_F_DMAC;
        }

        if (tb[TCA_SKBMOD_SMAC]) {
                saddr = nla_data(tb[TCA_SKBMOD_SMAC]);
                lflags |= SKBMOD_F_SMAC;
        }

        if (tb[TCA_SKBMOD_ETYPE]) {
                eth_type = nla_get_u16(tb[TCA_SKBMOD_ETYPE]);
                lflags |= SKBMOD_F_ETYPE;
        }

        parm = nla_data(tb[TCA_SKBMOD_PARMS]);

        if (parm->flags & SKBMOD_F_SWAPMAC)
                lflags = SKBMOD_F_SWAPMAC;

        exists = tcf_hash_check(tn, parm->index, a, bind);
        if (exists && bind)
                return 0;

        if (!lflags) {
                return -EINVAL;
        }

        if (!exists) {
                ret = tcf_hash_create(tn, parm->index, est, a,
                                      &act_skbmod_ops, bind, false);
                if (ret)
                        return ret;

                d = to_skbmod(*a);
                ret = ACT_P_CREATED;
        } else {
                d = to_skbmod(*a);
                tcf_hash_release(*a, bind);
                if (!ovr)
                        return -EEXIST;
        }

        ASSERT_RTNL();
        d->flags = lflags;
        d->tcf_action = parm->action;

        if (ovr)
                spin_lock_bh(&d->tcf_lock);

        if (lflags & SKBMOD_F_DMAC)
                ether_addr_copy(d->eth_dst, daddr);
        if (lflags & SKBMOD_F_SMAC)
                ether_addr_copy(d->eth_src, saddr);
        if (lflags & SKBMOD_F_ETYPE)
                d->eth_type = htons(eth_type);


        if (ovr) {
                spin_unlock_bh(&d->tcf_lock);
                synchronize_rcu();
        }

        if (ret == ACT_P_CREATED)
                tcf_hash_insert(tn, *a);
        return ret;
}