Thread (30 messages) 30 messages, 9 authors, 2013-01-09

Re: [PATCH] pkt_sched: act_xt support new Xtables interface

From: Jamal Hadi Salim <jhs@mojatatu.com>
Date: 2012-12-24 12:19:26
Also in: netfilter-devel

Possibly related (same subject, not in this thread)

On 12-12-24 06:49 AM, Felix Fietkau wrote:
After I added it as an experiment, I got distracted with other projects
again and forgot about submitting it. Take a look at the code - if the
approach is reasonable, I'll submit this thing for inclusion soon.
Excellent ;-> Simple and elegant.

Usable as is  - some minor comments.
First nitpick: The name is not very reflective, how about:
GetMarkFromConntrack or something along those lines?

+static int tcf_connmark(struct sk_buff *skb, const struct tc_action *a,
+		       struct tcf_result *res)
+{
+	struct nf_conn *c;
+	enum ip_conntrack_info ctinfo;
+	int proto;
+	int r;
+
+	if (skb->protocol == htons(ETH_P_IP)) {
+		if (skb->len < sizeof(struct iphdr))
+			goto out;
+		proto = PF_INET;
+	} else if (skb->protocol == htons(ETH_P_IPV6)) {
+		if (skb->len < sizeof(struct ipv6hdr))
+			goto out;
+		proto = PF_INET6;
+	} else
+		goto out;
+
I would have said that this action is probably also not useful for 
egress qdisc path since skb->mark would already be set. It maybe worth 
checking skb->tc_verd and skipping overhead of nf_conntrack_in() call.
Look at act_mirred for such a check.
+	r = nf_conntrack_in(dev_net(skb->dev), proto, NF_INET_PRE_ROUTING, skb);
+	if (r != NF_ACCEPT)
+		goto out;
+
+	c = nf_ct_get(skb, &ctinfo);
+	if (!c)
+		goto out;
+
+	skb->mark = c->mark;
+	nf_conntrack_put(skb->nfct);
+	skb->nfct = NULL;
+
+out:
+	return TC_ACT_PIPE;
Ok, perhaps set tcf_action in (iproute2) user space to TC_ACT_PIPE then 
just return policy->tcf_action here.

Even better is to have a different TC_ACT_XXX returned for failure
vs success... Your success path becomes TC_ACT_PIPE and let the
user program the failure branch optionally. This would allow for 
branching to different actions if success/failure, example:
if mark is found {
    if mark is 0xa redirect to ifb0
    else
      redirect to ifb1
} else
       set mark to 3 then redirect to ifb9

etc.

Not sure if that made sense. I am under the influence of nyquil ;->

cheers,
jamal

Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help