| Message ID | 20190627081047.24537-2-nikolay@cumulusnetworks.com |
|---|---|
| State | Accepted |
| Delegated to: | David Miller |
| Headers | show |
| Series | em_ipt: add support for addrtype | expand |
Hi Nik, On Thu, 27 Jun 2019 11:10:44 +0300 Nikolay Aleksandrov <nikolay@cumulusnetworks.com> wrote: > Restrict matching only to ip/ipv6 traffic and make sure we can use the > headers, otherwise matches will be attempted on any protocol which can > be unexpected by the xt matches. Currently policy supports only > ipv4/6. > > Signed-off-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com> > --- > v3: no change > v2: no change > > net/sched/em_ipt.c | 13 +++++++++++++ > 1 file changed, 13 insertions(+) > > diff --git a/net/sched/em_ipt.c b/net/sched/em_ipt.c > index 243fd22f2248..64dbafe4e94c 100644 > --- a/net/sched/em_ipt.c > +++ b/net/sched/em_ipt.c > @@ -185,6 +185,19 @@ static int em_ipt_match(struct sk_buff *skb, > struct tcf_ematch *em, struct nf_hook_state state; > int ret; > > + switch (tc_skb_protocol(skb)) { > + case htons(ETH_P_IP): > + if (!pskb_network_may_pull(skb, sizeof(struct > iphdr))) > + return 0; > + break; > + case htons(ETH_P_IPV6): > + if (!pskb_network_may_pull(skb, sizeof(struct > ipv6hdr))) > + return 0; > + break; > + default: > + return 0; > + } > + I just realized that I didn't consider the egress direction in my review. Don't we need an skb_pull() in that direction to make the skb->data point to L3? I see this is done e.g. in em_ipset. Eyal.
On 27 June 2019 19:02:37 EEST, Eyal Birger <eyal.birger@gmail.com> wrote: >Hi Nik, > >On Thu, 27 Jun 2019 11:10:44 +0300 >Nikolay Aleksandrov <nikolay@cumulusnetworks.com> wrote: > >> Restrict matching only to ip/ipv6 traffic and make sure we can use >the >> headers, otherwise matches will be attempted on any protocol which >can >> be unexpected by the xt matches. Currently policy supports only >> ipv4/6. >> >> Signed-off-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com> >> --- >> v3: no change >> v2: no change >> >> net/sched/em_ipt.c | 13 +++++++++++++ >> 1 file changed, 13 insertions(+) >> >> diff --git a/net/sched/em_ipt.c b/net/sched/em_ipt.c >> index 243fd22f2248..64dbafe4e94c 100644 >> --- a/net/sched/em_ipt.c >> +++ b/net/sched/em_ipt.c >> @@ -185,6 +185,19 @@ static int em_ipt_match(struct sk_buff *skb, >> struct tcf_ematch *em, struct nf_hook_state state; >> int ret; >> >> + switch (tc_skb_protocol(skb)) { >> + case htons(ETH_P_IP): >> + if (!pskb_network_may_pull(skb, sizeof(struct >> iphdr))) >> + return 0; >> + break; >> + case htons(ETH_P_IPV6): >> + if (!pskb_network_may_pull(skb, sizeof(struct >> ipv6hdr))) >> + return 0; >> + break; >> + default: >> + return 0; >> + } >> + > >I just realized that I didn't consider the egress direction in my >review. >Don't we need an skb_pull() in that direction to make the skb->data >point >to L3? I see this is done e.g. in em_ipset. > >Eyal. Hi Eyal, Not for addrtype, it doesn't have such expectations. I also tested it, everything matches properly. Cheers, Nik
diff --git a/net/sched/em_ipt.c b/net/sched/em_ipt.c index 243fd22f2248..64dbafe4e94c 100644 --- a/net/sched/em_ipt.c +++ b/net/sched/em_ipt.c @@ -185,6 +185,19 @@ static int em_ipt_match(struct sk_buff *skb, struct tcf_ematch *em, struct nf_hook_state state; int ret; + switch (tc_skb_protocol(skb)) { + case htons(ETH_P_IP): + if (!pskb_network_may_pull(skb, sizeof(struct iphdr))) + return 0; + break; + case htons(ETH_P_IPV6): + if (!pskb_network_may_pull(skb, sizeof(struct ipv6hdr))) + return 0; + break; + default: + return 0; + } + rcu_read_lock(); if (skb->skb_iif)
Restrict matching only to ip/ipv6 traffic and make sure we can use the headers, otherwise matches will be attempted on any protocol which can be unexpected by the xt matches. Currently policy supports only ipv4/6. Signed-off-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com> --- v3: no change v2: no change net/sched/em_ipt.c | 13 +++++++++++++ 1 file changed, 13 insertions(+)