* [PATCH 5/8] net_sched: cls_flow: use proto_ports_offset() to support AH message
@ 2010-08-18 5:05 Changli Gao
2010-08-18 11:02 ` jamal
0 siblings, 1 reply; 7+ messages in thread
From: Changli Gao @ 2010-08-18 5:05 UTC (permalink / raw)
To: Jamal Hadi Salim; +Cc: David S. Miller, netdev, Changli Gao
Signed-off-by: Changli Gao <xiaosuo@gmail.com>
---
net/sched/cls_flow.c | 67 ++++++++++++++++++++++++++++-----------------------
1 file changed, 38 insertions(+), 29 deletions(-)
diff --git a/net/sched/cls_flow.c b/net/sched/cls_flow.c
index e17096e..cd709f1 100644
--- a/net/sched/cls_flow.c
+++ b/net/sched/cls_flow.c
@@ -111,44 +111,41 @@ static u32 flow_get_proto(struct sk_buff *skb)
}
}
-static int has_ports(u8 protocol)
-{
- switch (protocol) {
- case IPPROTO_TCP:
- case IPPROTO_UDP:
- case IPPROTO_UDPLITE:
- case IPPROTO_SCTP:
- case IPPROTO_DCCP:
- case IPPROTO_ESP:
- return 1;
- default:
- return 0;
- }
-}
-
static u32 flow_get_proto_src(struct sk_buff *skb)
{
switch (skb->protocol) {
case htons(ETH_P_IP): {
struct iphdr *iph;
+ int poff;
if (!pskb_network_may_pull(skb, sizeof(*iph)))
break;
iph = ip_hdr(skb);
- if (!(iph->frag_off&htons(IP_MF|IP_OFFSET)) &&
- has_ports(iph->protocol) &&
- pskb_network_may_pull(skb, iph->ihl * 4 + 2))
- return ntohs(*(__be16 *)((void *)iph + iph->ihl * 4));
+ if (iph->frag_off & htons(IP_MF|IP_OFFSET))
+ break;
+ poff = proto_ports_offset(iph->protocol);
+ if (poff >= 0 &&
+ pskb_network_may_pull(skb, iph->ihl * 4 + 2 + poff)) {
+ iph = ip_hdr(skb);
+ return ntohs(*(__be16 *)((void *)iph + iph->ihl * 4 +
+ poff));
+ }
break;
}
case htons(ETH_P_IPV6): {
struct ipv6hdr *iph;
+ int poff;
- if (!pskb_network_may_pull(skb, sizeof(*iph) + 2))
+ if (!pskb_network_may_pull(skb, sizeof(*iph)))
break;
iph = ipv6_hdr(skb);
- if (has_ports(iph->nexthdr))
- return ntohs(*(__be16 *)&iph[1]);
+ poff = proto_ports_offset(iph->nexthdr);
+ if (poff >= 0 &&
+ pskb_network_may_pull(skb, sizeof(*iph) + poff + 2)) {
+ iph = ipv6_hdr(skb);
+ return ntohs(*(__be16 *)((void *)iph + sizeof(*iph) +
+ poff));
+ }
break;
}
}
@@ -161,24 +158,36 @@ static u32 flow_get_proto_dst(struct sk_buff *skb)
switch (skb->protocol) {
case htons(ETH_P_IP): {
struct iphdr *iph;
+ int poff;
if (!pskb_network_may_pull(skb, sizeof(*iph)))
break;
iph = ip_hdr(skb);
- if (!(iph->frag_off&htons(IP_MF|IP_OFFSET)) &&
- has_ports(iph->protocol) &&
- pskb_network_may_pull(skb, iph->ihl * 4 + 4))
- return ntohs(*(__be16 *)((void *)iph + iph->ihl * 4 + 2));
+ if (iph->frag_off & htons(IP_MF|IP_OFFSET))
+ break;
+ poff = proto_ports_offset(iph->protocol);
+ if (poff >= 0 &&
+ pskb_network_may_pull(skb, iph->ihl * 4 + 4 + poff)) {
+ iph = ip_hdr(skb);
+ return ntohs(*(__be16 *)((void *)iph + iph->ihl * 4 +
+ 2 + poff));
+ }
break;
}
case htons(ETH_P_IPV6): {
struct ipv6hdr *iph;
+ int poff;
- if (!pskb_network_may_pull(skb, sizeof(*iph) + 4))
+ if (!pskb_network_may_pull(skb, sizeof(*iph)))
break;
iph = ipv6_hdr(skb);
- if (has_ports(iph->nexthdr))
- return ntohs(*(__be16 *)((void *)&iph[1] + 2));
+ poff = proto_ports_offset(iph->nexthdr);
+ if (poff >= 0 &&
+ pskb_network_may_pull(skb, sizeof(*iph) + poff + 4)) {
+ iph = ipv6_hdr(skb);
+ return ntohs(*(__be16 *)((void *)iph + sizeof(*iph) +
+ poff + 2));
+ }
break;
}
}
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH 5/8] net_sched: cls_flow: use proto_ports_offset() to support AH message
2010-08-18 5:05 [PATCH 5/8] net_sched: cls_flow: use proto_ports_offset() to support AH message Changli Gao
@ 2010-08-18 11:02 ` jamal
2010-08-18 12:42 ` Changli Gao
0 siblings, 1 reply; 7+ messages in thread
From: jamal @ 2010-08-18 11:02 UTC (permalink / raw)
To: Changli Gao; +Cc: David S. Miller, netdev
On Wed, 2010-08-18 at 13:05 +0800, Changli Gao wrote:
> -static int has_ports(u8 protocol)
> -{
> - switch (protocol) {
> - case IPPROTO_TCP:
> - case IPPROTO_UDP:
> - case IPPROTO_UDPLITE:
> - case IPPROTO_SCTP:
> - case IPPROTO_DCCP:
> - case IPPROTO_ESP:
> - return 1;
> - default:
> - return 0;
> - }
> -}
> -
> static u32 flow_get_proto_src(struct sk_buff *skb)
> {
> switch (skb->protocol) {
> case htons(ETH_P_IP): {
> struct iphdr *iph;
> + int poff;
>
> if (!pskb_network_may_pull(skb, sizeof(*iph)))
> break;
> iph = ip_hdr(skb);
> - if (!(iph->frag_off&htons(IP_MF|IP_OFFSET)) &&
> - has_ports(iph->protocol) &&
> - pskb_network_may_pull(skb, iph->ihl * 4 + 2))
> - return ntohs(*(__be16 *)((void *)iph + iph->ihl * 4));
> + if (iph->frag_off & htons(IP_MF|IP_OFFSET))
> + break;
> + poff = proto_ports_offset(iph->protocol);
> + if (poff >= 0 &&
I dont think this maintains the same semantic. Ex: In the original code
AH returns 0. In your case it returns 4 and passes the above test.
Same with the other spot.
cheers,
jamal
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH 5/8] net_sched: cls_flow: use proto_ports_offset() to support AH message
2010-08-18 11:02 ` jamal
@ 2010-08-18 12:42 ` Changli Gao
2010-08-18 12:54 ` jamal
0 siblings, 1 reply; 7+ messages in thread
From: Changli Gao @ 2010-08-18 12:42 UTC (permalink / raw)
To: hadi; +Cc: David S. Miller, netdev
On Wed, Aug 18, 2010 at 7:02 PM, jamal <hadi@cyberus.ca> wrote:
> On Wed, 2010-08-18 at 13:05 +0800, Changli Gao wrote:
>
>> -static int has_ports(u8 protocol)
>> -{
>> - switch (protocol) {
>> - case IPPROTO_TCP:
>> - case IPPROTO_UDP:
>> - case IPPROTO_UDPLITE:
>> - case IPPROTO_SCTP:
>> - case IPPROTO_DCCP:
>> - case IPPROTO_ESP:
>> - return 1;
>> - default:
>> - return 0;
>> - }
>> -}
>> -
>> static u32 flow_get_proto_src(struct sk_buff *skb)
>> {
>> switch (skb->protocol) {
>> case htons(ETH_P_IP): {
>> struct iphdr *iph;
>> + int poff;
>>
>> if (!pskb_network_may_pull(skb, sizeof(*iph)))
>> break;
>> iph = ip_hdr(skb);
>> - if (!(iph->frag_off&htons(IP_MF|IP_OFFSET)) &&
>> - has_ports(iph->protocol) &&
>> - pskb_network_may_pull(skb, iph->ihl * 4 + 2))
>> - return ntohs(*(__be16 *)((void *)iph + iph->ihl * 4));
>> + if (iph->frag_off & htons(IP_MF|IP_OFFSET))
>> + break;
>> + poff = proto_ports_offset(iph->protocol);
>> + if (poff >= 0 &&
>
>
> I dont think this maintains the same semantic. Ex: In the original code
> AH returns 0. In your case it returns 4 and passes the above test.
> Same with the other spot.
>
I suppose we want to spread the traffic as possible as we can. For
ESP, we use the SPI as a key. And I think we can also use SPI in the
AH header as a key. It does change the semantic slightly for AH, but I
should not hurt, as the only effect is that the AH traffic is
distributed into different flows according to their different SPI.
--
Regards,
Changli Gao(xiaosuo@gmail.com)
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH 5/8] net_sched: cls_flow: use proto_ports_offset() to support AH message
2010-08-18 12:42 ` Changli Gao
@ 2010-08-18 12:54 ` jamal
2010-08-18 13:18 ` Changli Gao
0 siblings, 1 reply; 7+ messages in thread
From: jamal @ 2010-08-18 12:54 UTC (permalink / raw)
To: Changli Gao; +Cc: David S. Miller, netdev
On Wed, 2010-08-18 at 20:42 +0800, Changli Gao wrote:
> I suppose we want to spread the traffic as possible as we can. For
> ESP, we use the SPI as a key. And I think we can also use SPI in the
> AH header as a key. It does change the semantic slightly for AH, but I
> should not hurt, as the only effect is that the AH traffic is
> distributed into different flows according to their different SPI.
Sounds sensible - but please go through the code and double check that
nothing breaks (since i dont think you tested and i sympathize it would
be very time consuming to test) and more importantly when you make such
fundamental changes at least make comments to that effect in the logs.
cheers,
jamal
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 5/8] net_sched: cls_flow: use proto_ports_offset() to support AH message
2010-08-18 12:54 ` jamal
@ 2010-08-18 13:18 ` Changli Gao
2010-08-18 13:53 ` jamal
0 siblings, 1 reply; 7+ messages in thread
From: Changli Gao @ 2010-08-18 13:18 UTC (permalink / raw)
To: hadi; +Cc: David S. Miller, netdev
On Wed, Aug 18, 2010 at 8:54 PM, jamal <hadi@cyberus.ca> wrote:
>
> Sounds sensible - but please go through the code and double check that
> nothing breaks (since i dont think you tested and i sympathize it would
> be very time consuming to test) and more importantly when you make such
> fundamental changes at least make comments to that effect in the logs.
>
I had gone through the code before submitting this code. But it isn't
easy to test if it breaks sth. Would you please give a case to test ?
Thanks.
--
Regards,
Changli Gao(xiaosuo@gmail.com)
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 5/8] net_sched: cls_flow: use proto_ports_offset() to support AH message
2010-08-18 13:18 ` Changli Gao
@ 2010-08-18 13:53 ` jamal
2010-08-20 0:17 ` David Miller
0 siblings, 1 reply; 7+ messages in thread
From: jamal @ 2010-08-18 13:53 UTC (permalink / raw)
To: Changli Gao; +Cc: David S. Miller, netdev
On Wed, 2010-08-18 at 21:18 +0800, Changli Gao wrote:
> I had gone through the code before submitting this code. But it isn't
> easy to test if it breaks sth.
That works well actually and it is the LinuxWay(tm);->
> Would you please give a case to test ?
Just conformance testing. Since you added new AH SPI behavior
at least find some use case of what people normally use and make sure it
isnt broken. But if youve looked at code sufficiently i think thats good
enough.
cheers,
jamal
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 5/8] net_sched: cls_flow: use proto_ports_offset() to support AH message
2010-08-18 13:53 ` jamal
@ 2010-08-20 0:17 ` David Miller
0 siblings, 0 replies; 7+ messages in thread
From: David Miller @ 2010-08-20 0:17 UTC (permalink / raw)
To: hadi; +Cc: xiaosuo, netdev
From: jamal <hadi@cyberus.ca>
Date: Wed, 18 Aug 2010 09:53:09 -0400
> On Wed, 2010-08-18 at 21:18 +0800, Changli Gao wrote:
>
>> I had gone through the code before submitting this code. But it isn't
>> easy to test if it breaks sth.
>
> That works well actually and it is the LinuxWay(tm);->
>
>> Would you please give a case to test ?
>
> Just conformance testing. Since you added new AH SPI behavior
> at least find some use case of what people normally use and make sure it
> isnt broken. But if youve looked at code sufficiently i think thats good
> enough.
I agree and I've applied his patch.
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2010-08-20 0:17 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-08-18 5:05 [PATCH 5/8] net_sched: cls_flow: use proto_ports_offset() to support AH message Changli Gao
2010-08-18 11:02 ` jamal
2010-08-18 12:42 ` Changli Gao
2010-08-18 12:54 ` jamal
2010-08-18 13:18 ` Changli Gao
2010-08-18 13:53 ` jamal
2010-08-20 0:17 ` David Miller
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).