From: Jamal Hadi Salim <jhs@mojatatu•com>
To: Felix Fietkau <nbd@openwrt•org>
Cc: Yury Stankevich <urykhy@gmail•com>,
Hasan Chowdhury <shemonc@gmail•com>,
Stephen Hemminger <shemminger@vyatta•com>,
Jan Engelhardt <jengelh@inai•de>,
"netdev@vger•kernel.org" <netdev@vger•kernel.org>,
pablo@netfilter•org, netfilter-devel@vger•kernel.org
Subject: Re: [PATCH] pkt_sched: act_xt support new Xtables interface
Date: Mon, 24 Dec 2012 07:19:26 -0500 [thread overview]
Message-ID: <50D8484E.5070109@mojatatu.com> (raw)
In-Reply-To: <50D8413C.8050508@openwrt.org>
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
next prev parent reply other threads:[~2012-12-24 12:19 UTC|newest]
Thread overview: 69+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-12-09 12:20 tc ipt action Yury Stankevich
2012-12-13 10:58 ` Jamal Hadi Salim
2012-12-15 21:19 ` Jamal Hadi Salim
2012-12-15 23:06 ` Jan Engelhardt
2012-12-16 0:26 ` Jan Engelhardt
2012-12-16 0:32 ` [PATCH] build: unbreak linkage of m_xt.so Jan Engelhardt
2012-12-16 10:30 ` Jamal Hadi Salim
2012-12-16 17:03 ` Jamal Hadi Salim
2012-12-16 17:43 ` Jan Engelhardt
2012-12-16 18:05 ` Jamal Hadi Salim
2012-12-16 22:02 ` Mike Frysinger
2012-12-18 17:21 ` Stephen Hemminger
2012-12-18 18:47 ` Mike Frysinger
2012-12-20 0:03 ` Stephen Hemminger
2012-12-16 10:22 ` tc ipt action Jamal Hadi Salim
[not found] ` <CAASe=fQT2pVOK0uctdaKL+aOrF8nYeTMfoF15kmd-rC02+7Vnw@mail.gmail.com>
2012-12-16 16:48 ` Jamal Hadi Salim
2012-12-16 18:59 ` Jamal Hadi Salim
2012-12-16 19:13 ` Jan Engelhardt
2012-12-16 20:36 ` Jamal Hadi Salim
2012-12-16 20:41 ` [PATCH] iproute2: act_ipt fix xtables breakage Jamal Hadi Salim
2012-12-17 12:30 ` RFC [PATCH] iproute2: temporary solution to fix xt breakage Jamal Hadi Salim
2012-12-17 16:12 ` Stephen Hemminger
2012-12-19 11:36 ` Jamal Hadi Salim
[not found] ` <CAASe=fRuJdtisEvp7uo=PHwN3nKHqsYDW4Om1gk2MK-vyNvBrA@mail.gmail.com>
2012-12-18 12:28 ` Jamal Hadi Salim
[not found] ` <CAASe=fR6Hm2dxp=1wDchtrzqnaH6qacHpg2wrsqLfmGpPbQ9Fg@mail.gmail.com>
2012-12-19 11:44 ` Jamal Hadi Salim
2012-12-19 11:56 ` [PATCH] pkt_sched: act_xt support new Xtables interface Jamal Hadi Salim
2012-12-19 15:52 ` Jan Engelhardt
2012-12-19 23:05 ` Jamal Hadi Salim
[not found] ` <CAASe=fQZGwjM_2PStRE0tje33Doi6TuwJJ3p7x-SRcwq3mQvRg@mail.gmail.com>
2012-12-19 23:00 ` Jamal Hadi Salim
2012-12-20 8:54 ` Yury Stankevich
2012-12-20 12:35 ` Jamal Hadi Salim
2012-12-20 14:59 ` Yury Stankevich
2012-12-21 13:03 ` Jamal Hadi Salim
2012-12-21 13:13 ` Yury Stankevich
2012-12-21 13:50 ` Jamal Hadi Salim
2012-12-21 14:14 ` Yury Stankevich
2012-12-22 13:19 ` Jamal Hadi Salim
2012-12-22 13:43 ` Jan Engelhardt
2012-12-22 13:56 ` Jamal Hadi Salim
2012-12-22 13:58 ` Yury Stankevich
2012-12-22 14:04 ` Florian Westphal
2012-12-22 14:09 ` Jamal Hadi Salim
2012-12-24 11:34 ` Jamal Hadi Salim
2012-12-24 11:49 ` Felix Fietkau
2012-12-24 12:19 ` Jamal Hadi Salim [this message]
2012-12-24 13:12 ` Pablo Neira Ayuso
2012-12-24 14:05 ` Jamal Hadi Salim
2012-12-24 18:19 ` Pablo Neira Ayuso
2012-12-26 23:10 ` Pablo Neira Ayuso
2012-12-21 14:35 ` Jan Engelhardt
2012-12-21 15:45 ` Eric Dumazet
2012-12-22 13:42 ` Jamal Hadi Salim
2013-01-07 19:28 ` [PATCH net-next] net: introduce skb_transport_header_was_set() Eric Dumazet
2013-01-08 14:05 ` Jamal Hadi Salim
2013-01-09 1:52 ` David Miller
2012-12-16 0:27 ` tc ipt action Pablo Neira Ayuso
2012-12-16 0:59 ` Jan Engelhardt
2012-12-16 10:43 ` Jamal Hadi Salim
2012-12-16 17:21 ` Jan Engelhardt
2012-12-16 17:47 ` Jamal Hadi Salim
2012-12-16 18:59 ` Jan Engelhardt
2012-12-16 20:35 ` Jamal Hadi Salim
2012-12-16 21:21 ` Jan Engelhardt
2012-12-17 12:58 ` Jamal Hadi Salim
2012-12-17 13:28 ` Jan Engelhardt
2012-12-18 13:23 ` Jamal Hadi Salim
2012-12-18 13:58 ` Jan Engelhardt
2012-12-19 11:43 ` Jamal Hadi Salim
2012-12-16 10:26 ` Jamal Hadi Salim
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=50D8484E.5070109@mojatatu.com \
--to=jhs@mojatatu$(echo .)com \
--cc=jengelh@inai$(echo .)de \
--cc=nbd@openwrt$(echo .)org \
--cc=netdev@vger$(echo .)kernel.org \
--cc=netfilter-devel@vger$(echo .)kernel.org \
--cc=pablo@netfilter$(echo .)org \
--cc=shemminger@vyatta$(echo .)com \
--cc=shemonc@gmail$(echo .)com \
--cc=urykhy@gmail$(echo .)com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox