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

[Jamal Hadi Salim <jhs@mojatatu.com>]

[-- Attachment #1: Type: text/plain, Size: 498 bytes --]

On 16-08-30 07:12 AM, Jamal Hadi Salim wrote:

>
> Thanks Eric.
> This is the approach i thought Cong was going to take but in a way that
> it applies to all actions.
> It requires I do this extra allocation per update/create - I am not sure
> how much of a big deal that is (we take pride in our update rate).
> Let me work and post something simple that captures these ideas
> then wait to see what Cong has in mind for the general approach.

As attached here ... compile tested.

cheers,
jamal


[-- Attachment #2: skbmod-rcu-maybe2 --]
[-- Type: text/plain, Size: 3946 bytes --]

From header file:
-----
struct tcf_skbmod_params {
        u64     flags; /*up to 64 types of operations; extend if needed */
        u8      eth_dst[ETH_ALEN];
        u16     eth_type;
        u8      eth_src[ETH_ALEN];
};

struct tcf_skbmod {
        struct tc_action        common;
        struct tcf_skbmod_params  *skbmod_p;
};
#define to_skbmod(a) ((struct tcf_skbmod *)a)
------

#define MAX_EDIT_LEN ETH_HLEN
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);
	struct tcf_skbmod_params *p;
	u64 flags;
	int err;

	/* XXX: if you are going to edit more fields beyond ethernet header
	 * (example when you add IP header replacement or vlan swap)
	 * then MAX_EDIT_LEN needs to change appropriately
	*/
	err = skb_ensure_writable(skb, ETH_HLEN);
	if (unlikely(err)) /* best policy is to drop on the floor */
		action = TC_ACT_SHOT;

	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;
	}

	rcu_read_lock();
	p = rcu_dereference(d->skbmod_p);
	flags = p->flags;
	if (flags & SKBMOD_F_DMAC)
		ether_addr_copy(eth_hdr(skb)->h_dest, p->eth_dst);
	if (flags & SKBMOD_F_SMAC)
		ether_addr_copy(eth_hdr(skb)->h_source, p->eth_src);
	if (flags & SKBMOD_F_ETYPE)
		eth_hdr(skb)->h_proto = p->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;
	struct tcf_skbmod_params *p, *p_old;
	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();
	p = kzalloc(sizeof(struct tcf_skbmod_params), GFP_KERNEL);
	if (unlikely (!p)) {
		if (ovr)
			tcf_hash_release(*a, bind);
		return -ENOMEM;
	}

	p->flags = lflags;
	d->tcf_action = parm->action;

	p_old = rtnl_dereference(d->skbmod_p);

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

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

	rcu_assign_pointer(d->skbmod_p, p);
	if (ovr) {
		spin_unlock_bh(&d->tcf_lock);
		synchronize_rcu();
	}

	kfree(p_old);

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