* Re: [PATCH] cls_rsvp: add sanity check for the packet length
From: Changli Gao @ 2010-08-04 13:44 UTC (permalink / raw)
To: hadi; +Cc: David S. Miller, netdev
In-Reply-To: <1280929327.5669.10.camel@bigi>
On Wed, Aug 4, 2010 at 9:42 PM, jamal <hadi@cyberus.ca> wrote:
> On Wed, 2010-08-04 at 16:55 +0800, Changli Gao wrote:
>> The packet length should be checked before the packet data is dereferenced.
>>
>> Signed-off-by: Changli Gao <xiaosuo@gmail.com>
>> ---
>> net/sched/cls_rsvp.h | 10 +++++++++-
>> 1 file changed, 9 insertions(+), 1 deletion(-)
>> diff --git a/net/sched/cls_rsvp.h b/net/sched/cls_rsvp.h
>> index dd9414e..4fa119d 100644
>> --- a/net/sched/cls_rsvp.h
>> +++ b/net/sched/cls_rsvp.h
>> @@ -143,9 +143,17 @@ static int rsvp_classify(struct sk_buff *skb, struct tcf_proto *tp,
>> u8 tunnelid = 0;
>> u8 *xprt;
>> #if RSVP_DST_LEN == 4
>> - struct ipv6hdr *nhptr = ipv6_hdr(skb);
>> + struct ipv6hdr *nhptr;
>> +
>> + if (!pskb_may_pull(skb, skb_network_offset(skb) + sizeof(*nhptr)))
>> + return -1;
>> + nhptr = ipv6_hdr(skb);
>> #else
>> struct iphdr *nhptr = ip_hdr(skb);
>> +
>> + if (!pskb_may_pull(skb, skb_network_offset(skb) + sizeof(*nhptr)))
>> + return -1;
>> + nhptr = ip_hdr(skb);
>> #endif
>
> Maybe a good time to move nhptr declaration outside #if since it is used
> in #else as well.
>
They are not the same type. I am afraid that it won't work.
--
Regards,
Changli Gao(xiaosuo@gmail.com)
^ permalink raw reply
* Re: [PATCH] cls_flow: add sanity check for the packet length
From: Changli Gao @ 2010-08-04 13:51 UTC (permalink / raw)
To: hadi; +Cc: Patrick McHardy, David S. Miller, netdev
In-Reply-To: <1280929183.5669.7.camel@bigi>
On Wed, Aug 4, 2010 at 9:39 PM, jamal <hadi@cyberus.ca> wrote:
> On Wed, 2010-08-04 at 15:08 +0800, Changli Gao wrote:
>> The packet length should be checked before the packet data is dereferenced.
>>
>> Signed-off-by: Changli Gao <xiaosuo@gmail.com>
>> ---
>> include/linux/skbuff.h | 6 ++--
>> net/sched/cls_flow.c | 69 +++++++++++++++++++++++++++++++++----------------
>> 2 files changed, 50 insertions(+), 25 deletions(-)
>> diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
>> index d20d9e7..3f0ac50 100644
>> --- a/include/linux/skbuff.h
>> +++ b/include/linux/skbuff.h
>> @@ -1210,12 +1210,12 @@ static inline unsigned char *pskb_pull(struct sk_buff *skb, unsigned int len)
>> return unlikely(len > skb->len) ? NULL : __pskb_pull(skb, len);
>> }
>>
>> -static inline int pskb_may_pull(struct sk_buff *skb, unsigned int len)
>> +static inline bool pskb_may_pull(struct sk_buff *skb, unsigned int len)
>> {
>> if (likely(len <= skb_headlen(skb)))
>> - return 1;
>> + return true;
>> if (unlikely(len > skb->len))
>> - return 0;
>> + return false;
>> return __pskb_pull_tail(skb, len - skb_headlen(skb)) != NULL;
>> }
>
> Above is a different patch?
Patrick thinks such things don't worth individual patches. I remember
my such patch was applied by David. It is for netdev, not netfilter. I
should obey David's rules. I'll remove this in the next version.
Thanks.
>
>> diff --git a/net/sched/cls_flow.c b/net/sched/cls_flow.c
>> index f73542d..d63a374 100644
>> --- a/net/sched/cls_flow.c
>> +++ b/net/sched/cls_flow.c
>> @@ -65,37 +65,48 @@ static inline u32 addr_fold(void *addr)
>> return (a & 0xFFFFFFFF) ^ (BITS_PER_LONG > 32 ? a >> 32 : 0);
>> }
>>
>> -static u32 flow_get_src(const struct sk_buff *skb)
>> +static inline bool flow_pskb_may_pull(struct sk_buff *skb, unsigned int len)
>> +{
>> + return pskb_may_pull(skb, skb_network_offset(skb) + len);
>> +}
>
> network_pskb_may_pull() would make it more generic (probably in
> skbuff.h) and reused everywhere you need skb_network_offset
>
I think pskb_network_may_pull() is better.
>
>> +static u32 flow_get_src(struct sk_buff *skb)
>> {
>> switch (skb->protocol) {
>> case htons(ETH_P_IP):
>> - return ntohl(ip_hdr(skb)->saddr);
>> + return flow_pskb_may_pull(skb, sizeof(struct iphdr)) ?
>> + ntohl(ip_hdr(skb)->saddr) : 0;
>> case htons(ETH_P_IPV6):
>> - return ntohl(ipv6_hdr(skb)->saddr.s6_addr32[3]);
>> + return flow_pskb_may_pull(skb, sizeof(struct ipv6hdr)) ?
>> + ntohl(ipv6_hdr(skb)->saddr.s6_addr32[3]) : 0;
>> default:
>
>
> BTW - does returning zero above make sense or is returning the default below better?
>
>> return addr_fold(skb->sk);
>
> Same comment applies in all other switch statements..
>
Oh, good suggestion. Thanks.
--
Regards,
Changli Gao(xiaosuo@gmail.com)
^ permalink raw reply
* Re: [PATCH] cls_rsvp: add sanity check for the packet length
From: Changli Gao @ 2010-08-04 13:52 UTC (permalink / raw)
To: hadi; +Cc: David S. Miller, netdev
In-Reply-To: <AANLkTinW6zSEWQZVVz=utH1zqL_+bRStT7a_Pdx5-705@mail.gmail.com>
On Wed, Aug 4, 2010 at 9:44 PM, Changli Gao <xiaosuo@gmail.com> wrote:
> On Wed, Aug 4, 2010 at 9:42 PM, jamal <hadi@cyberus.ca> wrote:
>> On Wed, 2010-08-04 at 16:55 +0800, Changli Gao wrote:
>>> The packet length should be checked before the packet data is dereferenced.
>>>
>>> Signed-off-by: Changli Gao <xiaosuo@gmail.com>
>>> ---
>>> net/sched/cls_rsvp.h | 10 +++++++++-
>>> 1 file changed, 9 insertions(+), 1 deletion(-)
>>> diff --git a/net/sched/cls_rsvp.h b/net/sched/cls_rsvp.h
>>> index dd9414e..4fa119d 100644
>>> --- a/net/sched/cls_rsvp.h
>>> +++ b/net/sched/cls_rsvp.h
>>> @@ -143,9 +143,17 @@ static int rsvp_classify(struct sk_buff *skb, struct tcf_proto *tp,
>>> u8 tunnelid = 0;
>>> u8 *xprt;
>>> #if RSVP_DST_LEN == 4
>>> - struct ipv6hdr *nhptr = ipv6_hdr(skb);
>>> + struct ipv6hdr *nhptr;
>>> +
>>> + if (!pskb_may_pull(skb, skb_network_offset(skb) + sizeof(*nhptr)))
>>> + return -1;
>>> + nhptr = ipv6_hdr(skb);
>>> #else
>>> struct iphdr *nhptr = ip_hdr(skb);
>>> +
>>> + if (!pskb_may_pull(skb, skb_network_offset(skb) + sizeof(*nhptr)))
>>> + return -1;
>>> + nhptr = ip_hdr(skb);
>>> #endif
>>
>> Maybe a good time to move nhptr declaration outside #if since it is used
>> in #else as well.
>>
>
> They are not the same type. I am afraid that it won't work.
>
I'll respin it with the routine pskb_network_may_pull(). Thanks.
--
Regards,
Changli Gao(xiaosuo@gmail.com)
^ permalink raw reply
* Re: [patch] usbnet: fix 100% CPU use on suspended device
From: Elly Jones @ 2010-08-04 14:04 UTC (permalink / raw)
To: Oliver Neukum
Cc: Alan Stern, David Miller, netdev-u79uwXL29TY76Z2rM5mHXA, USB list
In-Reply-To: <201008031639.24874.oliver-GvhC2dPhHPQdnm+yROfE0A@public.gmane.org>
On Tue, Aug 3, 2010 at 10:39 AM, Oliver Neukum <oliver-GvhC2dPhHPQdnm+yROfE0A@public.gmane.org> wrote:
> Am Montag, 2. August 2010, 15:31:33 schrieb Elly Jones:
>> On Mon, Jul 26, 2010 at 12:21 PM, Oliver Neukum <oliver-GvhC2dPhHPQdnm+yROfE0A@public.gmane.org> wrote:
>> > Am Montag, 26. Juli 2010, 17:13:23 schrieb Alan Stern:
>> >> On Mon, 26 Jul 2010, Elly Jones wrote:
>> >>
>> >> > > This isn't right. The problem should be fixed some other way. Under
>> >> > > what circumstances are URBs submitted incorrectly?
>> >> >
>> >> > When the device is autosuspended. What is the proper thing for a
>> >> > device to do here?
>> >>
>> >> From looking at the code, it appears that the EVENT_DEV_ASLEEP flag
>> >> should be tested in usbnet_bh() the way it is in rx_submit(). But I'm
>> >> not an expert on usbnet; we should ask someone who is, like Oliver.
>> >
>> > Sorry, I didn't notice this thread.
>> >
>> > The correct way to check for autosuspend in usbnet is to look
>> > at EVENT_DEV_ASLEEP under txq.lock. That being said, usbnet_bh()
>> > uses rx_submit() which does the correct check. The bug seems to be
>> > a lack of error handling in usbnet_bh() regarding the return of rx_submit()
>>
>> If rx_submit() fails, should usbnet_bh() just not tasklet_schedule() itself?
>
> That would not work unless the cause of the failure would be removed.
> If you get -ENOLINK the sane option seems to me to give up.
'Give up' meaning what? If we reschedule the tasklet, it'll just try
again (and fail again), won't it?
> Regards
> Oliver
-- Elly
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [patch] usbnet: fix 100% CPU use on suspended device
From: Oliver Neukum @ 2010-08-04 14:08 UTC (permalink / raw)
To: Elly Jones; +Cc: Alan Stern, David Miller, netdev, USB list
In-Reply-To: <AANLkTim-6VhV7AD_QQh-5j5RqxEDDh1oyX4CqZWGb-ZH@mail.gmail.com>
Am Mittwoch, 4. August 2010, 16:04:48 schrieben Sie:
> On Tue, Aug 3, 2010 at 10:39 AM, Oliver Neukum <oliver@neukum.org> wrote:
> > Am Montag, 2. August 2010, 15:31:33 schrieb Elly Jones:
> >> On Mon, Jul 26, 2010 at 12:21 PM, Oliver Neukum <oliver@neukum.org> wrote:
> >> > Am Montag, 26. Juli 2010, 17:13:23 schrieb Alan Stern:
> >> >> On Mon, 26 Jul 2010, Elly Jones wrote:
> >> >>
> >> >> > > This isn't right. The problem should be fixed some other way. Under
> >> >> > > what circumstances are URBs submitted incorrectly?
> >> >> >
> >> >> > When the device is autosuspended. What is the proper thing for a
> >> >> > device to do here?
> >> >>
> >> >> From looking at the code, it appears that the EVENT_DEV_ASLEEP flag
> >> >> should be tested in usbnet_bh() the way it is in rx_submit(). But I'm
> >> >> not an expert on usbnet; we should ask someone who is, like Oliver.
> >> >
> >> > Sorry, I didn't notice this thread.
> >> >
> >> > The correct way to check for autosuspend in usbnet is to look
> >> > at EVENT_DEV_ASLEEP under txq.lock. That being said, usbnet_bh()
> >> > uses rx_submit() which does the correct check. The bug seems to be
> >> > a lack of error handling in usbnet_bh() regarding the return of rx_submit()
> >>
> >> If rx_submit() fails, should usbnet_bh() just not tasklet_schedule() itself?
> >
> > That would not work unless the cause of the failure would be removed.
> > If you get -ENOLINK the sane option seems to me to give up.
>
> 'Give up' meaning what? If we reschedule the tasklet, it'll just try
> again (and fail again), won't it?
Yes, exactly. If the tasklet runs after the interface has been suspended,
it cannot replenish the rx URBs. That will be the job of resume()
Just stop trying and do nothing.
Regards
Oliver
^ permalink raw reply
* Re: [PATCH v4 1/2] core: Factor out flow calculation from get_rps_cpu
From: Krishna Kumar2 @ 2010-08-04 14:23 UTC (permalink / raw)
To: Changli Gao; +Cc: arnd, bhutchings, davem, mst, netdev, therbert
In-Reply-To: <AANLkTikURsh5wSdOE0_qXDw=XgNMSp5CJ1RytQnBvoyD@mail.gmail.com>
Changli Gao <xiaosuo@gmail.com> wrote on 08/04/2010 07:08:39 PM:
> Changli Gao <xiaosuo@gmail.com>
> 08/04/2010 07:08 PM
>
> To
>
> Krishna Kumar2/India/IBM@IBMIN
>
> cc
>
> arnd@arndb.de, bhutchings@solarflare.com, davem@davemloft.net,
> mst@redhat.com, netdev@vger.kernel.org, therbert@google.com
>
> Subject
>
> Re: [PATCH v4 1/2] core: Factor out flow calculation from get_rps_cpu
>
> On Wed, Aug 4, 2010 at 8:02 PM, Krishna Kumar2 <krkumar2@in.ibm.com>
wrote:
> > Changli Gao <xiaosuo@gmail.com> wrote on 08/04/2010 04:52:58 PM:
> >>
> >> the return value of skb_get_rxhash() maybe skb->rxhash, so we don't
> >> need to assign it to skb->rxhash again. Use a local variable to save
> >> the value and __skb_get_rxhash() should cache the rxhash into
> >> skb->rxhash for future use.
> >
> > I wanted the function to return a rxhash instead of save+return,
> > since other users, eg macvtap, doesn't need the rxhash beyond
> > calculating a rxq with that hash. So for those users, we can
> > avoid updating both skb->rxhash and a local variable (since with
> > your suggestion, rps will update either one or two variables
> > depending on whether rxhash is cached or not). I am not sure
> > which is better.
> >
>
> rxhash can also be generated by NIC(hardware) and used some where
> else. So for these users, generating the rxhash again is wasting time.
Yes, I expect that to happen once. By not saving it in skb,
atleast for macvtap, I didn't see any further rx path. I guess
I could do this:
get_rps_cpu()
{
if (!skb_get_rxhash(skb))
goto done;
/* use skb->rxhash from here on */
}
Replying to your other mail here:
> > macvtap *with* mq support would be used with mq devices - you
> > open multiple queues on the macvtap device depending on the
> > number of queues for the physical device. So since this is an
> > unlikely case (as can be seen in the patch, and I guess I
> > should add another "likely" to the "if (tap)" check since fd's
> > should not be closed), I guess a simple % can be used. Does
> > the following sound reasonable?
> >
> > 1. Use % to find the slot.
> It is slower than the method used by get_rps_cpu().
Yes, but it should be an unlikely case. Is it still important,
considering you add a lot of baggage for a rare case?
So next patch has these two changes, unless contradicted :)
1. Change skb_get_rxhash() to set if not cached already.
2. Use % for macvtap unlikely case.
Thanks,
- KK
^ permalink raw reply
* Re: KS8695: possible NAPI issue
From: Yegor Yefremov @ 2010-08-04 14:40 UTC (permalink / raw)
To: figo zhang; +Cc: Dick Hollenbeck, netdev, zealcook
In-Reply-To: <f69abfc31003150219u4e103b0fg3dc22628054e4327@mail.gmail.com>
On Mon, Mar 15, 2010 at 11:19 AM, Yegor Yefremov
<yegorslists@googlemail.com> wrote:
> On Tue, Mar 9, 2010 at 2:50 AM, figo zhang <figo1802@gmail.com> wrote:
>> 2010/3/8 Yegor Yefremov <yegorslists@googlemail.com>:
>>> This is ping afterwards:
>>>
>>> debian:~# ping -c 1 192.168.1.38
>>>
>>> PING 192.168.1.38 (192.168.1.38) 56(84) bytes of data.
>>>
>>>
>>> tx ram addr = c371c202 , data len = 62
>>>
>>> dst:00:18:f3:fe:84:84 src:00:04:d9:80:94:cd type: 0x0800
>>
>> => the print info is not clear, it had better just keep "print_mem()",
>> and remove other printk.
>>
>>>
>>> 0xc371c20e : 45 00
>>>
>>> 0xc371c212 : 00 54 00 00 40 00 40 01 b6 f0 c0 a8 01 42 c0 a8
>>>
>>> 0xc371c222 : 01 26 08 00 65 51 1a 07 00 01 92 00 95 4b 5f 57
>>>
>>> 0xc371c232 : 07 00 08 09 0a 0b 0c 0d 0e 0f 10 11 12 13 14 15
>>>
>>> 0xc371c242 : 16 17 18 19 1a 1b 1c 1d 1e 1f 20 21 22 23 24 25
>>>
>>> 0xc371c252 : 26 27 28 29 2a 2b 2c 2d 2e 2f 30 31 32 33 34 35
>>>
>>> 0xc371c262 : 36 37
>>>
>>>
>>>
>>> tx ram addr = c371c202 , data len = 2a
>>>
>>> dst:00:18:f3:fe:84:84 src:00:04:d9:80:94:cd type: 0x0806
>>>
>>> 0xc371c20e : 00 01
>>>
>>> 0xc371c212 : 08 00 06 04 00 01 00 04 d9 80 94 cd c0 a8 01 42
>> => it is a send arp packet, "c0 a8 01 42" is your curr ip addr
>> 192.168.1.66, tell the other side it's ip addr.
>>>
>>> 0xc371c222 : 00 00 00 00 00 00 c0 a8 01 26
>>>
>>>
>>>
>>> rx ram addr = c35d2020 , data len = 3c
>>>
>>> dst:00:04:d9:80:94:cd src:00:18:f3:fe:84:84 type: 0x0806
>>>
>>> 0xc35d202c : 00 01
>>>
>>> 0xc35d2030 : 08 00 06 04 00 02 00 18 f3 fe 84 84 c0 a8 01 26
>>
>> => it is a reply arp packet, "c0 a8 01 26" (192.168.1.38 ) is other
>> side ip addr. so the arp
>> request is finished.
>>
>>>
>>> 0xc35d2040 : 00 04 d9 80 94 cd c0 a8 01 42 00 00 00 00 00 00
>>>
>>> 0xc35d2050 : 00 00 00 00 00 00 00 00 00 00 00 00
>>>
>> => so , it should still send "IMCP" packet, you using "ping -c 1", it
>> send one IMCP packet , and wait for reply. at this point, it have not
>> send packet?
>
> I made some tests on Fr with a Netbook connected directly to our
> ks8695 based device and not to the companies network. During the test
> I couldn't encounter any problem. Than I connected ks8695 again to the
> companies network and the issue was there right away. So I decided to
> check how many broadcast I get and it turned out that I have one
> device on the network that is sending broadcast almost every second if
> not more frequent. It turned out that the network on ks8695 was not
> completely down, but very slow and many packets got lost. As soon as I
> removed the broadcast sending device I could ping ks8695. The
> consequences:
>
> 1. as long as I make no netcat transfer, the network responses are as
> usual regardless of many broadcasts
> 2. if during netcat a broadcast occurs, than the network is getting
> slow, so that if during a constant ping you get a broadcast that ping
> response gets lost
>
> Any idea how broadcast during transfer could affect NAPI in that way?
Tried 2.6.35. Have still the same problems. Another observation:
if I add eth0 to a bridge (br0), then I have no problems. So ks8695
network driver is still involved, but the upper level seems to make
the difference.
Any idea?
Regards,
Yegor
^ permalink raw reply
* [PATCH v2 1/4] sk_buff: introduce pskb_network_may_pull()
From: Changli Gao @ 2010-08-04 14:43 UTC (permalink / raw)
To: David S. Miller; +Cc: Jamal Hadi Salim, Patrick McHardy, netdev, Changli Gao
Signed-off-by: Changli Gao <xiaosuo@gmail.com>
---
include/linux/skbuff.h | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index d20d9e7..77eb60d 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -1379,6 +1379,11 @@ static inline int skb_network_offset(const struct sk_buff *skb)
return skb_network_header(skb) - skb->data;
}
+static inline int pskb_network_may_pull(struct sk_buff *skb, unsigned int len)
+{
+ return pskb_may_pull(skb, skb_network_offset(skb) + len);
+}
+
/*
* CPUs often take a performance hit when accessing unaligned memory
* locations. The actual performance hit varies, it can be small if the
^ permalink raw reply related
* [PATCH v2 2/4] cls_flow: add sanity check for the packet length
From: Changli Gao @ 2010-08-04 14:48 UTC (permalink / raw)
To: Patrick McHardy; +Cc: Jamal Hadi Salim, David S. Miller, netdev, Changli Gao
The packet length should be checked before the packet data is dereferenced.
Signed-off-by: Changli Gao <xiaosuo@gmail.com>
---
v2: use pskb_network_may_pull(), and handle the error path gracefully.
remove the return value change.
net/sched/cls_flow.c | 96 +++++++++++++++++++++++++++++----------------------
1 file changed, 56 insertions(+), 40 deletions(-)
diff --git a/net/sched/cls_flow.c b/net/sched/cls_flow.c
index f73542d..e17096e 100644
--- a/net/sched/cls_flow.c
+++ b/net/sched/cls_flow.c
@@ -65,37 +65,47 @@ static inline u32 addr_fold(void *addr)
return (a & 0xFFFFFFFF) ^ (BITS_PER_LONG > 32 ? a >> 32 : 0);
}
-static u32 flow_get_src(const struct sk_buff *skb)
+static u32 flow_get_src(struct sk_buff *skb)
{
switch (skb->protocol) {
case htons(ETH_P_IP):
- return ntohl(ip_hdr(skb)->saddr);
+ if (pskb_network_may_pull(skb, sizeof(struct iphdr)))
+ return ntohl(ip_hdr(skb)->saddr);
+ break;
case htons(ETH_P_IPV6):
- return ntohl(ipv6_hdr(skb)->saddr.s6_addr32[3]);
- default:
- return addr_fold(skb->sk);
+ if (pskb_network_may_pull(skb, sizeof(struct ipv6hdr)))
+ return ntohl(ipv6_hdr(skb)->saddr.s6_addr32[3]);
+ break;
}
+
+ return addr_fold(skb->sk);
}
-static u32 flow_get_dst(const struct sk_buff *skb)
+static u32 flow_get_dst(struct sk_buff *skb)
{
switch (skb->protocol) {
case htons(ETH_P_IP):
- return ntohl(ip_hdr(skb)->daddr);
+ if (pskb_network_may_pull(skb, sizeof(struct iphdr)))
+ return ntohl(ip_hdr(skb)->daddr);
+ break;
case htons(ETH_P_IPV6):
- return ntohl(ipv6_hdr(skb)->daddr.s6_addr32[3]);
- default:
- return addr_fold(skb_dst(skb)) ^ (__force u16)skb->protocol;
+ if (pskb_network_may_pull(skb, sizeof(struct ipv6hdr)))
+ return ntohl(ipv6_hdr(skb)->daddr.s6_addr32[3]);
+ break;
}
+
+ return addr_fold(skb_dst(skb)) ^ (__force u16)skb->protocol;
}
-static u32 flow_get_proto(const struct sk_buff *skb)
+static u32 flow_get_proto(struct sk_buff *skb)
{
switch (skb->protocol) {
case htons(ETH_P_IP):
- return ip_hdr(skb)->protocol;
+ return pskb_network_may_pull(skb, sizeof(struct iphdr)) ?
+ ip_hdr(skb)->protocol : 0;
case htons(ETH_P_IPV6):
- return ipv6_hdr(skb)->nexthdr;
+ return pskb_network_may_pull(skb, sizeof(struct ipv6hdr)) ?
+ ipv6_hdr(skb)->nexthdr : 0;
default:
return 0;
}
@@ -116,58 +126,64 @@ static int has_ports(u8 protocol)
}
}
-static u32 flow_get_proto_src(const struct sk_buff *skb)
+static u32 flow_get_proto_src(struct sk_buff *skb)
{
- u32 res = 0;
-
switch (skb->protocol) {
case htons(ETH_P_IP): {
- struct iphdr *iph = ip_hdr(skb);
+ struct iphdr *iph;
+ 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))
- res = ntohs(*(__be16 *)((void *)iph + iph->ihl * 4));
+ has_ports(iph->protocol) &&
+ pskb_network_may_pull(skb, iph->ihl * 4 + 2))
+ return ntohs(*(__be16 *)((void *)iph + iph->ihl * 4));
break;
}
case htons(ETH_P_IPV6): {
- struct ipv6hdr *iph = ipv6_hdr(skb);
+ struct ipv6hdr *iph;
+ if (!pskb_network_may_pull(skb, sizeof(*iph) + 2))
+ break;
+ iph = ipv6_hdr(skb);
if (has_ports(iph->nexthdr))
- res = ntohs(*(__be16 *)&iph[1]);
+ return ntohs(*(__be16 *)&iph[1]);
break;
}
- default:
- res = addr_fold(skb->sk);
}
- return res;
+ return addr_fold(skb->sk);
}
-static u32 flow_get_proto_dst(const struct sk_buff *skb)
+static u32 flow_get_proto_dst(struct sk_buff *skb)
{
- u32 res = 0;
-
switch (skb->protocol) {
case htons(ETH_P_IP): {
- struct iphdr *iph = ip_hdr(skb);
+ struct iphdr *iph;
+ 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))
- res = ntohs(*(__be16 *)((void *)iph + iph->ihl * 4 + 2));
+ has_ports(iph->protocol) &&
+ pskb_network_may_pull(skb, iph->ihl * 4 + 4))
+ return ntohs(*(__be16 *)((void *)iph + iph->ihl * 4 + 2));
break;
}
case htons(ETH_P_IPV6): {
- struct ipv6hdr *iph = ipv6_hdr(skb);
+ struct ipv6hdr *iph;
+ if (!pskb_network_may_pull(skb, sizeof(*iph) + 4))
+ break;
+ iph = ipv6_hdr(skb);
if (has_ports(iph->nexthdr))
- res = ntohs(*(__be16 *)((void *)&iph[1] + 2));
+ return ntohs(*(__be16 *)((void *)&iph[1] + 2));
break;
}
- default:
- res = addr_fold(skb_dst(skb)) ^ (__force u16)skb->protocol;
}
- return res;
+ return addr_fold(skb_dst(skb)) ^ (__force u16)skb->protocol;
}
static u32 flow_get_iif(const struct sk_buff *skb)
@@ -211,7 +227,7 @@ static u32 flow_get_nfct(const struct sk_buff *skb)
})
#endif
-static u32 flow_get_nfct_src(const struct sk_buff *skb)
+static u32 flow_get_nfct_src(struct sk_buff *skb)
{
switch (skb->protocol) {
case htons(ETH_P_IP):
@@ -223,7 +239,7 @@ fallback:
return flow_get_src(skb);
}
-static u32 flow_get_nfct_dst(const struct sk_buff *skb)
+static u32 flow_get_nfct_dst(struct sk_buff *skb)
{
switch (skb->protocol) {
case htons(ETH_P_IP):
@@ -235,14 +251,14 @@ fallback:
return flow_get_dst(skb);
}
-static u32 flow_get_nfct_proto_src(const struct sk_buff *skb)
+static u32 flow_get_nfct_proto_src(struct sk_buff *skb)
{
return ntohs(CTTUPLE(skb, src.u.all));
fallback:
return flow_get_proto_src(skb);
}
-static u32 flow_get_nfct_proto_dst(const struct sk_buff *skb)
+static u32 flow_get_nfct_proto_dst(struct sk_buff *skb)
{
return ntohs(CTTUPLE(skb, dst.u.all));
fallback:
@@ -281,7 +297,7 @@ static u32 flow_get_vlan_tag(const struct sk_buff *skb)
return tag & VLAN_VID_MASK;
}
-static u32 flow_key_get(const struct sk_buff *skb, int key)
+static u32 flow_key_get(struct sk_buff *skb, int key)
{
switch (key) {
case FLOW_KEY_SRC:
^ permalink raw reply related
* Re: [PATCH v4 2/2] macvtap: Implement multiqueue for macvtap driver
From: Arnd Bergmann @ 2010-08-04 14:54 UTC (permalink / raw)
To: Krishna Kumar2; +Cc: Changli Gao, bhutchings, davem, mst, netdev, therbert
In-Reply-To: <OF3D82198C.4738EB40-ON65257775.0046DC1D-65257775.004997E7@in.ibm.com>
On Wednesday 04 August 2010, Krishna Kumar2 wrote:
> 1. Use % to find the slot.
> 2. If slot is null - I don't want to handle this since I think
> it is better to return NULL if some fd's were closed by user.
> Typically this should never happen since fd's are opened and
> passed to vhost for setting up the backend. So if they are
> closed, then I think NULL is OK.
>
Having some of the file descriptors closed is not something that
we should optimize for, but then again it makes sense to still
keep going, mostly for consistency reasons: One day we may
have a use case for a dynamically growing and shrinking set of
queues. Falling back to the first queue is probably ok, we can
always add optimizations when this becomes a real use case.
Arnd
^ permalink raw reply
* [PATCH v2 3/4] cls_rsvp: add sanity check for the packet length
From: Changli Gao @ 2010-08-04 14:55 UTC (permalink / raw)
To: Jamal Hadi Salim; +Cc: David S. Miller, netdev, Changli Gao
The packet length should be checked before the packet data is dereferenced.
Signed-off-by: Changli Gao <xiaosuo@gmail.com>
---
v2: use pskb_network_may_pull()
net/sched/cls_rsvp.h | 12 ++++++++++--
1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/net/sched/cls_rsvp.h b/net/sched/cls_rsvp.h
index dd9414e..425a179 100644
--- a/net/sched/cls_rsvp.h
+++ b/net/sched/cls_rsvp.h
@@ -143,9 +143,17 @@ static int rsvp_classify(struct sk_buff *skb, struct tcf_proto *tp,
u8 tunnelid = 0;
u8 *xprt;
#if RSVP_DST_LEN == 4
- struct ipv6hdr *nhptr = ipv6_hdr(skb);
+ struct ipv6hdr *nhptr;
+
+ if (!pskb_network_may_pull(skb, sizeof(*nhptr)))
+ return -1;
+ nhptr = ipv6_hdr(skb);
#else
- struct iphdr *nhptr = ip_hdr(skb);
+ struct iphdr *nhptr;
+
+ if (!pskb_network_may_pull(skb, sizeof(*nhptr)))
+ return -1;
+ nhptr = ip_hdr(skb);
#endif
restart:
^ permalink raw reply related
* [PATCH v2 4/4] sch_sfq: add sanity check for the packet length
From: Changli Gao @ 2010-08-04 14:58 UTC (permalink / raw)
To: Jamal Hadi Salim; +Cc: David S. Miller, netdev, Changli Gao
The packet length should be checked before the packet data is dereferenced.
Signed-off-by: Changli Gao <xiaosuo@gmail.com>
---
v2: use pskb_network_may_pull()
net/sched/sch_sfq.c | 29 ++++++++++++++++++++---------
1 file changed, 20 insertions(+), 9 deletions(-)
diff --git a/net/sched/sch_sfq.c b/net/sched/sch_sfq.c
index c657628..e85352b 100644
--- a/net/sched/sch_sfq.c
+++ b/net/sched/sch_sfq.c
@@ -122,7 +122,11 @@ static unsigned sfq_hash(struct sfq_sched_data *q, struct sk_buff *skb)
switch (skb->protocol) {
case htons(ETH_P_IP):
{
- const struct iphdr *iph = ip_hdr(skb);
+ const struct iphdr *iph;
+
+ if (!pskb_network_may_pull(skb, sizeof(*iph)))
+ goto err;
+ iph = ip_hdr(skb);
h = (__force u32)iph->daddr;
h2 = (__force u32)iph->saddr ^ iph->protocol;
if (!(iph->frag_off&htons(IP_MF|IP_OFFSET)) &&
@@ -131,25 +135,32 @@ static unsigned sfq_hash(struct sfq_sched_data *q, struct sk_buff *skb)
iph->protocol == IPPROTO_UDPLITE ||
iph->protocol == IPPROTO_SCTP ||
iph->protocol == IPPROTO_DCCP ||
- iph->protocol == IPPROTO_ESP))
+ iph->protocol == IPPROTO_ESP) &&
+ pskb_network_may_pull(skb, iph->ihl * 4 + 4))
h2 ^= *(((u32*)iph) + iph->ihl);
break;
}
case htons(ETH_P_IPV6):
{
- struct ipv6hdr *iph = ipv6_hdr(skb);
+ struct ipv6hdr *iph;
+
+ if (!pskb_network_may_pull(skb, sizeof(*iph)))
+ goto err;
+ iph = ipv6_hdr(skb);
h = (__force u32)iph->daddr.s6_addr32[3];
h2 = (__force u32)iph->saddr.s6_addr32[3] ^ iph->nexthdr;
- if (iph->nexthdr == IPPROTO_TCP ||
- iph->nexthdr == IPPROTO_UDP ||
- iph->nexthdr == IPPROTO_UDPLITE ||
- iph->nexthdr == IPPROTO_SCTP ||
- iph->nexthdr == IPPROTO_DCCP ||
- iph->nexthdr == IPPROTO_ESP)
+ if ((iph->nexthdr == IPPROTO_TCP ||
+ iph->nexthdr == IPPROTO_UDP ||
+ iph->nexthdr == IPPROTO_UDPLITE ||
+ iph->nexthdr == IPPROTO_SCTP ||
+ iph->nexthdr == IPPROTO_DCCP ||
+ iph->nexthdr == IPPROTO_ESP) &&
+ pskb_network_may_pull(skb, sizeof(*iph) + 4))
h2 ^= *(u32*)&iph[1];
break;
}
default:
+err:
h = (unsigned long)skb_dst(skb) ^ (__force u32)skb->protocol;
h2 = (unsigned long)skb->sk;
}
^ permalink raw reply related
* [PATCH block#for-2.6.36] block_dev: always serialize exclusive open attempts
From: Tejun Heo @ 2010-08-04 15:40 UTC (permalink / raw)
To: Jens Axboe
Cc: Rafael J. Wysocki, Linux Kernel Mailing List, Maciej Rutecki,
Andrew Morton, Kernel Testers List, Network Development,
Linux ACPI, Linux PM List, Linux SCSI List, Linux Wireless List,
DRI, Linus Torvalds, stable, Maciej Rutecki
In-Reply-To: <4C56F30D.3020304@kernel.org>
bd_prepare_to_claim() incorrectly allowed multiple attempts for
exclusive open to progress in parallel if the attempting holders are
identical. This triggered BUG_ON() as reported in the following bug.
https://bugzilla.kernel.org/show_bug.cgi?id=16393
__bd_abort_claiming() is used to finish claiming blocks and doesn't
work if multiple openers are inside a claiming block. Allowing
multiple parallel open attempts to continue doesn't gain anything as
those are serialized down in the call chain anyway. Fix it by always
allowing only single open attempt in a claiming block.
This problem can easily be reproduced by adding a delay after
bd_prepare_to_claim() and attempting to mount two partitions of a
disk.
stable: only applicable to v2.6.35
Signed-off-by: Tejun Heo <tj@kernel.org>
Reported-by: Maciej Rutecki <maciej.rutecki@gmail.com>
Cc: stable@kernel.org
---
fs/block_dev.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/fs/block_dev.c b/fs/block_dev.c
index 99d6af8..b3171fb 100644
--- a/fs/block_dev.c
+++ b/fs/block_dev.c
@@ -681,8 +681,8 @@ retry:
if (!bd_may_claim(bdev, whole, holder))
return -EBUSY;
- /* if someone else is claiming, wait for it to finish */
- if (whole->bd_claiming && whole->bd_claiming != holder) {
+ /* if claiming is already in progress, wait for it to finish */
+ if (whole->bd_claiming) {
wait_queue_head_t *wq = bit_waitqueue(&whole->bd_claiming, 0);
DEFINE_WAIT(wait);
^ permalink raw reply related
* [PATCH RESEND block#for-2.6.36] block_dev: always serialize exclusive open attempts
From: Tejun Heo @ 2010-08-04 15:59 UTC (permalink / raw)
To: Jens Axboe
Cc: Rafael J. Wysocki, Linux Kernel Mailing List, Maciej Rutecki,
Andrew Morton, Kernel Testers List, Network Development,
Linux ACPI, Linux PM List, Linux SCSI List, Linux Wireless List,
DRI, Linus Torvalds, stable, Maciej Rutecki
In-Reply-To: <4C56F30D.3020304@kernel.org>
bd_prepare_to_claim() incorrectly allowed multiple attempts for
exclusive open to progress in parallel if the attempting holders are
identical. This triggered BUG_ON() as reported in the following bug.
https://bugzilla.kernel.org/show_bug.cgi?id=16393
__bd_abort_claiming() is used to finish claiming blocks and doesn't
work if multiple openers are inside a claiming block. Allowing
multiple parallel open attempts to continue doesn't gain anything as
those are serialized down in the call chain anyway. Fix it by always
allowing only single open attempt in a claiming block.
This problem can easily be reproduced by adding a delay after
bd_prepare_to_claim() and attempting to mount two partitions of a
disk.
stable: only applicable to v2.6.35
Signed-off-by: Tejun Heo <tj@kernel.org>
Reported-by: Markus Trippelsdorf <markus@trippelsdorf.de>
Cc: stable@kernel.org
---
Oops, had the wrong reported-by credit. Updated.
Thanks.
fs/block_dev.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/fs/block_dev.c b/fs/block_dev.c
index 99d6af8..b3171fb 100644
--- a/fs/block_dev.c
+++ b/fs/block_dev.c
@@ -681,8 +681,8 @@ retry:
if (!bd_may_claim(bdev, whole, holder))
return -EBUSY;
- /* if someone else is claiming, wait for it to finish */
- if (whole->bd_claiming && whole->bd_claiming != holder) {
+ /* if claiming is already in progress, wait for it to finish */
+ if (whole->bd_claiming) {
wait_queue_head_t *wq = bit_waitqueue(&whole->bd_claiming, 0);
DEFINE_WAIT(wait);
^ permalink raw reply related
* Re: [PATCH] cls_flow: add sanity check for the packet length
From: jamal @ 2010-08-04 14:18 UTC (permalink / raw)
To: Changli Gao; +Cc: Patrick McHardy, David S. Miller, netdev
In-Reply-To: <AANLkTi=th9x0yWv3h1eT4Za7pqh0UDcT1QC_D9X1POOQ@mail.gmail.com>
On Wed, 2010-08-04 at 21:51 +0800, Changli Gao wrote:
>
> Patrick thinks such things don't worth individual patches. I remember
> my such patch was applied by David. It is for netdev, not netfilter. I
> should obey David's rules. I'll remove this in the next version.
Probably Patrick was concerned about such a standalone patch with no
followup user; you need to have a followup user.
cheers,
jamal
^ permalink raw reply
* Re: [patch] netfilter: default to NF_DROP in sip_help_tcp()
From: Patrick McHardy @ 2010-08-04 16:10 UTC (permalink / raw)
To: Simon Horman; +Cc: netfilter-devel, netdev
In-Reply-To: <20100804080742.GC10740@verge.net.au>
Am 04.08.2010 10:07, schrieb Simon Horman:
> On Wed, Jul 14, 2010 at 02:23:01PM +0200, Patrick McHardy wrote:
>> On 10.07.2010 05:16, Simon Horman wrote:
>>> I initially noticed this because of the compiler warning below, but it does
>>> seem to be a valid concern in the case where ct_sip_get_header() returns 0
>>> in the first iteration of the while loop.
>>>
>>> net/netfilter/nf_conntrack_sip.c: In function 'sip_help_tcp':
>>> net/netfilter/nf_conntrack_sip.c:1379: warning: 'ret' may be used uninitialized in this function
>>
>> Thanks Simon. I've applied the patch, but changed NF_DROP to
>> NF_ACCEPT since we should avoid dropping packets with unknown
>> contents (not SIP) if possible.
>
> Hi Patrick,
>
> I'm not seeing this patch in nf-next-2.6.
> Am I looking in the wrong place?
I was struggling with some file system corruption and didn't manage
to send it out in time, sorry. I'll include it in the next batch of
patches for .36 and will also push it to -stable.
^ permalink raw reply
* Re: [PATCH v2 2/4] cls_flow: add sanity check for the packet length
From: Patrick McHardy @ 2010-08-04 16:12 UTC (permalink / raw)
To: Changli Gao; +Cc: Jamal Hadi Salim, David S. Miller, netdev
In-Reply-To: <1280933292-12176-1-git-send-email-xiaosuo@gmail.com>
Am 04.08.2010 16:48, schrieb Changli Gao:
> The packet length should be checked before the packet data is dereferenced.
>
Thanks.
Acked-by: Patrick McHardy <kaber@trash.net>
^ permalink raw reply
* [PATCH v5 1/2] core: Factor out flow calculation from get_rps_cpu
From: Krishna Kumar @ 2010-08-04 16:15 UTC (permalink / raw)
To: davem, arnd; +Cc: mst, netdev, xiaosuo, bhutchings, Krishna Kumar, therbert
From: Krishna Kumar <krkumar2@in.ibm.com>
Factor out flow calculation code from get_rps_cpu, since other
functions can use the same code.
Revisions:
v2 (Ben): Separate flow calcuation out and use in select queue.
v3 (Arnd): Don't re-implement MIN.
v4 (Changli): skb->data points to ethernet header in macvtap, and
make a fast path. Tested macvtap with this patch.
v5 (Changli):
- Cache skb->rxhash in skb_get_rxhash
- macvtap may not have pow(2) queues, so change code for
queue selection.
(Arnd):
- Use first available queue if all fails.
Signed-off-by: Krishna Kumar <krkumar2@in.ibm.com>
---
include/linux/skbuff.h | 9 +++
net/core/dev.c | 106 ++++++++++++++++++++++-----------------
2 files changed, 71 insertions(+), 44 deletions(-)
diff -ruNp org/include/linux/skbuff.h new/include/linux/skbuff.h
--- org/include/linux/skbuff.h 2010-08-04 20:45:19.000000000 +0530
+++ new/include/linux/skbuff.h 2010-08-04 20:47:02.000000000 +0530
@@ -558,6 +558,15 @@ extern unsigned int skb_find_text(stru
unsigned int to, struct ts_config *config,
struct ts_state *state);
+extern __u32 __skb_get_rxhash(struct sk_buff *skb);
+static inline __u32 skb_get_rxhash(struct sk_buff *skb)
+{
+ if (!skb->rxhash)
+ skb->rxhash = __skb_get_rxhash(skb);
+
+ return skb->rxhash;
+}
+
#ifdef NET_SKBUFF_DATA_USES_OFFSET
static inline unsigned char *skb_end_pointer(const struct sk_buff *skb)
{
diff -ruNp org/net/core/dev.c new/net/core/dev.c
--- org/net/core/dev.c 2010-08-04 20:45:19.000000000 +0530
+++ new/net/core/dev.c 2010-08-04 21:13:37.000000000 +0530
@@ -2259,69 +2259,41 @@ static inline void ____napi_schedule(str
__raise_softirq_irqoff(NET_RX_SOFTIRQ);
}
-#ifdef CONFIG_RPS
-
-/* One global table that all flow-based protocols share. */
-struct rps_sock_flow_table *rps_sock_flow_table __read_mostly;
-EXPORT_SYMBOL(rps_sock_flow_table);
-
/*
- * get_rps_cpu is called from netif_receive_skb and returns the target
- * CPU from the RPS map of the receiving queue for a given skb.
- * rcu_read_lock must be held on entry.
+ * __skb_get_rxhash: calculate a flow hash based on src/dst addresses
+ * and src/dst port numbers. Returns a non-zero hash number on success
+ * and 0 on failure.
*/
-static int get_rps_cpu(struct net_device *dev, struct sk_buff *skb,
- struct rps_dev_flow **rflowp)
+__u32 __skb_get_rxhash(struct sk_buff *skb)
{
+ int nhoff, hash = 0;
struct ipv6hdr *ip6;
struct iphdr *ip;
- struct netdev_rx_queue *rxqueue;
- struct rps_map *map;
- struct rps_dev_flow_table *flow_table;
- struct rps_sock_flow_table *sock_flow_table;
- int cpu = -1;
u8 ip_proto;
- u16 tcpu;
u32 addr1, addr2, ihl;
union {
u32 v32;
u16 v16[2];
} ports;
- if (skb_rx_queue_recorded(skb)) {
- u16 index = skb_get_rx_queue(skb);
- if (unlikely(index >= dev->num_rx_queues)) {
- WARN_ONCE(dev->num_rx_queues > 1, "%s received packet "
- "on queue %u, but number of RX queues is %u\n",
- dev->name, index, dev->num_rx_queues);
- goto done;
- }
- rxqueue = dev->_rx + index;
- } else
- rxqueue = dev->_rx;
-
- if (!rxqueue->rps_map && !rxqueue->rps_flow_table)
- goto done;
-
- if (skb->rxhash)
- goto got_hash; /* Skip hash computation on packet header */
+ nhoff = skb_network_offset(skb);
switch (skb->protocol) {
case __constant_htons(ETH_P_IP):
- if (!pskb_may_pull(skb, sizeof(*ip)))
+ if (!pskb_may_pull(skb, sizeof(*ip) + nhoff))
goto done;
- ip = (struct iphdr *) skb->data;
+ ip = (struct iphdr *) skb->data + nhoff;
ip_proto = ip->protocol;
addr1 = (__force u32) ip->saddr;
addr2 = (__force u32) ip->daddr;
ihl = ip->ihl;
break;
case __constant_htons(ETH_P_IPV6):
- if (!pskb_may_pull(skb, sizeof(*ip6)))
+ if (!pskb_may_pull(skb, sizeof(*ip6) + nhoff))
goto done;
- ip6 = (struct ipv6hdr *) skb->data;
+ ip6 = (struct ipv6hdr *) skb->data + nhoff;
ip_proto = ip6->nexthdr;
addr1 = (__force u32) ip6->saddr.s6_addr32[3];
addr2 = (__force u32) ip6->daddr.s6_addr32[3];
@@ -2330,6 +2302,7 @@ static int get_rps_cpu(struct net_device
default:
goto done;
}
+
switch (ip_proto) {
case IPPROTO_TCP:
case IPPROTO_UDP:
@@ -2338,8 +2311,9 @@ static int get_rps_cpu(struct net_device
case IPPROTO_AH:
case IPPROTO_SCTP:
case IPPROTO_UDPLITE:
- if (pskb_may_pull(skb, (ihl * 4) + 4)) {
- ports.v32 = * (__force u32 *) (skb->data + (ihl * 4));
+ if (pskb_may_pull(skb, (ihl * 4) + 4 + nhoff)) {
+ ports.v32 = * (__force u32 *) (skb->data + nhoff +
+ (ihl * 4));
if (ports.v16[1] < ports.v16[0])
swap(ports.v16[0], ports.v16[1]);
break;
@@ -2352,11 +2326,55 @@ static int get_rps_cpu(struct net_device
/* get a consistent hash (same value on both flow directions) */
if (addr2 < addr1)
swap(addr1, addr2);
- skb->rxhash = jhash_3words(addr1, addr2, ports.v32, hashrnd);
- if (!skb->rxhash)
- skb->rxhash = 1;
-got_hash:
+ hash = jhash_3words(addr1, addr2, ports.v32, hashrnd);
+ if (!hash)
+ hash = 1;
+
+done:
+ return hash;
+}
+EXPORT_SYMBOL(__skb_get_rxhash);
+
+#ifdef CONFIG_RPS
+
+/* One global table that all flow-based protocols share. */
+struct rps_sock_flow_table *rps_sock_flow_table __read_mostly;
+EXPORT_SYMBOL(rps_sock_flow_table);
+
+/*
+ * get_rps_cpu is called from netif_receive_skb and returns the target
+ * CPU from the RPS map of the receiving queue for a given skb.
+ * rcu_read_lock must be held on entry.
+ */
+static int get_rps_cpu(struct net_device *dev, struct sk_buff *skb,
+ struct rps_dev_flow **rflowp)
+{
+ struct netdev_rx_queue *rxqueue;
+ struct rps_map *map;
+ struct rps_dev_flow_table *flow_table;
+ struct rps_sock_flow_table *sock_flow_table;
+ int cpu = -1;
+ u16 tcpu;
+
+ if (skb_rx_queue_recorded(skb)) {
+ u16 index = skb_get_rx_queue(skb);
+ if (unlikely(index >= dev->num_rx_queues)) {
+ WARN_ONCE(dev->num_rx_queues > 1, "%s received packet "
+ "on queue %u, but number of RX queues is %u\n",
+ dev->name, index, dev->num_rx_queues);
+ goto done;
+ }
+ rxqueue = dev->_rx + index;
+ } else
+ rxqueue = dev->_rx;
+
+ if (!rxqueue->rps_map && !rxqueue->rps_flow_table)
+ goto done;
+
+ if (!skb_get_rxhash(skb))
+ goto done;
+
flow_table = rcu_dereference(rxqueue->rps_flow_table);
sock_flow_table = rcu_dereference(rps_sock_flow_table);
if (flow_table && sock_flow_table) {
^ permalink raw reply
* [PATCH v5 2/2] macvtap: Implement multiqueue for macvtap driver
From: Krishna Kumar @ 2010-08-04 16:15 UTC (permalink / raw)
To: davem, arnd; +Cc: mst, netdev, xiaosuo, bhutchings, Krishna Kumar, therbert
In-Reply-To: <20100804161552.3814.56839.sendpatchset@krkumar2.in.ibm.com>
From: Krishna Kumar <krkumar2@in.ibm.com>
Implement multiqueue facility for macvtap driver. The idea is that
a macvtap device can be opened multiple times and the fd's can be
used to register eg, as backend for vhost.
Signed-off-by: Krishna Kumar <krkumar2@in.ibm.com>
---
drivers/net/macvtap.c | 99 ++++++++++++++++++++++++++++-------
include/linux/if_macvlan.h | 9 ++-
2 files changed, 90 insertions(+), 18 deletions(-)
diff -ruNp org/include/linux/if_macvlan.h new/include/linux/if_macvlan.h
--- org/include/linux/if_macvlan.h 2010-08-04 20:45:19.000000000 +0530
+++ new/include/linux/if_macvlan.h 2010-08-04 20:45:19.000000000 +0530
@@ -40,6 +40,12 @@ struct macvlan_rx_stats {
unsigned long rx_errors;
};
+/*
+ * Maximum times a macvtap device can be opened. This can be used to
+ * configure the number of receive queue, e.g. for multiqueue virtio.
+ */
+#define MAX_MACVTAP_QUEUES (NR_CPUS < 16 ? NR_CPUS : 16)
+
struct macvlan_dev {
struct net_device *dev;
struct list_head list;
@@ -50,7 +56,8 @@ struct macvlan_dev {
enum macvlan_mode mode;
int (*receive)(struct sk_buff *skb);
int (*forward)(struct net_device *dev, struct sk_buff *skb);
- struct macvtap_queue *tap;
+ struct macvtap_queue *taps[MAX_MACVTAP_QUEUES];
+ int numvtaps;
};
static inline void macvlan_count_rx(const struct macvlan_dev *vlan,
diff -ruNp org/drivers/net/macvtap.c new/drivers/net/macvtap.c
--- org/drivers/net/macvtap.c 2010-08-04 20:45:19.000000000 +0530
+++ new/drivers/net/macvtap.c 2010-08-04 21:20:49.000000000 +0530
@@ -84,26 +84,45 @@ static const struct proto_ops macvtap_so
static DEFINE_SPINLOCK(macvtap_lock);
/*
- * Choose the next free queue, for now there is only one
+ * get_slot: return a [unused/occupied] slot in vlan->taps[]:
+ * - if 'q' is NULL, return the first empty slot;
+ * - otherwise, return the slot this pointer occupies.
*/
+static int get_slot(struct macvlan_dev *vlan, struct macvtap_queue *q)
+{
+ int i;
+
+ for (i = 0; i < MAX_MACVTAP_QUEUES; i++) {
+ if (rcu_dereference(vlan->taps[i]) == q)
+ return i;
+ }
+
+ /* Should never happen */
+ BUG_ON(1);
+}
+
static int macvtap_set_queue(struct net_device *dev, struct file *file,
struct macvtap_queue *q)
{
struct macvlan_dev *vlan = netdev_priv(dev);
+ int index;
int err = -EBUSY;
spin_lock(&macvtap_lock);
- if (rcu_dereference(vlan->tap))
+ if (vlan->numvtaps == MAX_MACVTAP_QUEUES)
goto out;
err = 0;
+ index = get_slot(vlan, NULL);
rcu_assign_pointer(q->vlan, vlan);
- rcu_assign_pointer(vlan->tap, q);
+ rcu_assign_pointer(vlan->taps[index], q);
sock_hold(&q->sk);
q->file = file;
file->private_data = q;
+ vlan->numvtaps++;
+
out:
spin_unlock(&macvtap_lock);
return err;
@@ -124,9 +143,12 @@ static void macvtap_put_queue(struct mac
spin_lock(&macvtap_lock);
vlan = rcu_dereference(q->vlan);
if (vlan) {
- rcu_assign_pointer(vlan->tap, NULL);
+ int index = get_slot(vlan, q);
+
+ rcu_assign_pointer(vlan->taps[index], NULL);
rcu_assign_pointer(q->vlan, NULL);
sock_put(&q->sk);
+ --vlan->numvtaps;
}
spin_unlock(&macvtap_lock);
@@ -136,39 +158,82 @@ static void macvtap_put_queue(struct mac
}
/*
- * Since we only support one queue, just dereference the pointer.
+ * Select a queue based on the rxq of the device on which this packet
+ * arrived. If the incoming device is not mq, calculate a flow hash
+ * to select a queue. If all fails, find the first available queue.
+ * Cache vlan->numvtaps since it can become zero during the execution
+ * of this function.
*/
static struct macvtap_queue *macvtap_get_queue(struct net_device *dev,
struct sk_buff *skb)
{
struct macvlan_dev *vlan = netdev_priv(dev);
+ struct macvtap_queue *tap = NULL;
+ int numvtaps = vlan->numvtaps;
+ __u32 rxq;
+
+ if (!numvtaps)
+ goto out;
+
+ if (likely(skb_rx_queue_recorded(skb))) {
+ rxq = skb_get_rx_queue(skb);
+
+ while (unlikely(rxq >= numvtaps))
+ rxq -= numvtaps;
+
+ tap = rcu_dereference(vlan->taps[rxq]);
+ if (tap)
+ goto out;
+ }
+
+ /* Check if we can use flow to select a queue */
+ rxq = skb_get_rxhash(skb);
+ if (rxq) {
+ tap = rcu_dereference(vlan->taps[rxq % numvtaps]);
+ if (tap)
+ goto out;
+ }
- return rcu_dereference(vlan->tap);
+ /* Everything failed - find first available queue */
+ for (rxq = 0; rxq < MAX_MACVTAP_QUEUES; rxq++) {
+ tap = rcu_dereference(vlan->taps[rxq]);
+ if (tap)
+ break;
+ }
+
+out:
+ return tap;
}
/*
* The net_device is going away, give up the reference
- * that it holds on the queue (all the queues one day)
- * and safely set the pointer from the queues to NULL.
+ * that it holds on all queues and safely set the pointer
+ * from the queues to NULL.
*/
static void macvtap_del_queues(struct net_device *dev)
{
struct macvlan_dev *vlan = netdev_priv(dev);
- struct macvtap_queue *q;
+ struct macvtap_queue *q, *qlist[MAX_MACVTAP_QUEUES];
+ int i, j = 0;
+ /* macvtap_put_queue can free some slots, so go through all slots */
spin_lock(&macvtap_lock);
- q = rcu_dereference(vlan->tap);
- if (!q) {
- spin_unlock(&macvtap_lock);
- return;
+ for (i = 0; i < MAX_MACVTAP_QUEUES && vlan->numvtaps; i++) {
+ q = rcu_dereference(vlan->taps[i]);
+ if (q) {
+ qlist[j++] = q;
+ rcu_assign_pointer(vlan->taps[i], NULL);
+ rcu_assign_pointer(q->vlan, NULL);
+ vlan->numvtaps--;
+ }
}
-
- rcu_assign_pointer(vlan->tap, NULL);
- rcu_assign_pointer(q->vlan, NULL);
+ BUG_ON(vlan->numvtaps != 0);
spin_unlock(&macvtap_lock);
synchronize_rcu();
- sock_put(&q->sk);
+
+ for (--j; j >= 0; j--)
+ sock_put(&qlist[j]->sk);
}
/*
^ permalink raw reply
* Re: [ANNOUNCE]: Release of iptables-1.4.9
From: Patrick McHardy @ 2010-08-04 16:16 UTC (permalink / raw)
To: Gabor Z. Papp
Cc: Jan Engelhardt, Netfilter Development Mailinglist,
Linux Netdev List, 'netfilter@vger.kernel.org',
netfilter-announce
In-Reply-To: <x6sk2vo9eq@gzp>
Am 03.08.2010 20:09, schrieb Gabor Z. Papp:
> * Jan Engelhardt <jengelh@medozas.de>:
>
> | (BTW, with --disable-shared you remove the possibility to use any .so
> | files whatsoever. You can use --enable-static --enable-shared to get
> | both "all in one binary" and ".so support".)
>
> And how to force linking the binaries against the static libs?
>
> | The following changes since commit 371cea299f0b2eb100b9fc9fb99089640d2d606f:
>
> | xtables: remove unnecessary cast (2010-08-03 19:56:11 +0200)
>
> | are available in the git repository at:
> | git://dev.medozas.de/iptables master
>
> Fixed, compiled fine.
Thanks, I'll release a .1 with this patch tommorrow.
^ permalink raw reply
* Re: Yet another bridge netfilter crash
From: Patrick McHardy @ 2010-08-04 16:30 UTC (permalink / raw)
To: Herbert Xu; +Cc: Stephen Hemminger, netdev
In-Reply-To: <20100723152609.GA7576@gondor.apana.org.au>
Am 23.07.2010 17:26, schrieb Herbert Xu:
> On Fri, Jul 23, 2010 at 05:17:42PM +0200, Patrick McHardy wrote:
>>
>>> There's also the matter of fragments jumping between bridges.
>>
>> Conntrack zones can be used to avoid that, but that currently needs
>> manual configuration.
>
> I think this is something that we need to fix. Because as it
> stands, it can still crash if you get the wrong nf_bridge.
>
> The reason is that skb->dev does not hold a ref count. So the
> reassembly code just throws it away and always uses the dev of
> the last fragment.
>
> This breaks when two bridges combine to reassemble a single
> packet, as the nf_bridge attribute of the reassembled packet
> may come from an skb whose device is now dead. This is then
> used to fill in the skb->dev (via nf_bridge->physindev).
We could perform a new device lookup on reassembly as we do
when expiring a fragment queue, but we probably shouldn't even
be reassembling fragments from different bridges. One way to
avoid this would be to automatically assign each bridge device
to a different conntrack zone, but conntrack zones are limited
to 2^16 and this might also have other unwanted side-effects.
Until we come up with something better the best fix seems to
be to perform the device lookup based on the iif.
^ permalink raw reply
* Re: [ANNOUNCE]: Release of iptables-1.4.9
From: Jan Engelhardt @ 2010-08-04 16:32 UTC (permalink / raw)
To: Patrick McHardy
Cc: Gabor Z. Papp, Netfilter Development Mailinglist,
Linux Netdev List, 'netfilter@vger.kernel.org',
netfilter-announce
In-Reply-To: <4C599275.2080007@trash.net>
On Wednesday 2010-08-04 18:16, Patrick McHardy wrote:
>Am 03.08.2010 20:09, schrieb Gabor Z. Papp:
>> * Jan Engelhardt <jengelh@medozas.de>:
>>
>> | (BTW, with --disable-shared you remove the possibility to use any .so
>> | files whatsoever. You can use --enable-static --enable-shared to get
>> | both "all in one binary" and ".so support".)
>>
>> And how to force linking the binaries against the static libs?
>>
>> | The following changes since commit 371cea299f0b2eb100b9fc9fb99089640d2d606f:
>>
>> | xtables: remove unnecessary cast (2010-08-03 19:56:11 +0200)
>>
>> | are available in the git repository at:
>> | git://dev.medozas.de/iptables master
>>
>> Fixed, compiled fine.
>
>Thanks, I'll release a .1 with this patch tommorrow.
You probably want to add to your personal release script section one
that testcompiles all configurations before possibly creating a tarball.
I do so too with Xtables-addons (all kernels from 2.6.17 onwards,
quite a disk eater).
./configure --enable-static --enable-shared && make
^ permalink raw reply
* Re: Yet another bridge netfilter crash
From: Herbert Xu @ 2010-08-04 16:41 UTC (permalink / raw)
To: Patrick McHardy; +Cc: Stephen Hemminger, netdev
In-Reply-To: <4C5995C2.1010909@trash.net>
On Wed, Aug 04, 2010 at 06:30:58PM +0200, Patrick McHardy wrote:
>
> We could perform a new device lookup on reassembly as we do
> when expiring a fragment queue, but we probably shouldn't even
> be reassembling fragments from different bridges. One way to
> avoid this would be to automatically assign each bridge device
> to a different conntrack zone, but conntrack zones are limited
> to 2^16 and this might also have other unwanted side-effects.
>
> Until we come up with something better the best fix seems to
> be to perform the device lookup based on the iif.
I don't think we can as the iif will point to the bridge device.
The physindev contains the original physical device where the
packet came in.
Cheers,
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* [PATCH net-next] drivers/s390/net/qeth_l3_main.c: Use %pI6
From: Joe Perches @ 2010-08-04 16:50 UTC (permalink / raw)
To: Frank Blaschka; +Cc: Ursula Braun, linux390, linux-s390, netdev, LKML
Format an ipv6 address using vsprintf extensions.
Signed-off-by: Joe Perches <joe@perches.com>
---
drivers/s390/net/qeth_l3_main.c | 7 +------
1 files changed, 1 insertions(+), 6 deletions(-)
diff --git a/drivers/s390/net/qeth_l3_main.c b/drivers/s390/net/qeth_l3_main.c
index e22ae24..561bdc8 100644
--- a/drivers/s390/net/qeth_l3_main.c
+++ b/drivers/s390/net/qeth_l3_main.c
@@ -103,12 +103,7 @@ int qeth_l3_string_to_ipaddr4(const char *buf, __u8 *addr)
void qeth_l3_ipaddr6_to_string(const __u8 *addr, char *buf)
{
- sprintf(buf, "%02x%02x:%02x%02x:%02x%02x:%02x%02x"
- ":%02x%02x:%02x%02x:%02x%02x:%02x%02x",
- addr[0], addr[1], addr[2], addr[3],
- addr[4], addr[5], addr[6], addr[7],
- addr[8], addr[9], addr[10], addr[11],
- addr[12], addr[13], addr[14], addr[15]);
+ sprintf(buf, "%pI6", addr);
}
int qeth_l3_string_to_ipaddr6(const char *buf, __u8 *addr)
^ permalink raw reply related
* Re: Yet another bridge netfilter crash
From: Patrick McHardy @ 2010-08-04 16:50 UTC (permalink / raw)
To: Herbert Xu; +Cc: Stephen Hemminger, netdev
In-Reply-To: <20100804164119.GA6256@gondor.apana.org.au>
Am 04.08.2010 18:41, schrieb Herbert Xu:
> On Wed, Aug 04, 2010 at 06:30:58PM +0200, Patrick McHardy wrote:
>>
>> We could perform a new device lookup on reassembly as we do
>> when expiring a fragment queue, but we probably shouldn't even
>> be reassembling fragments from different bridges. One way to
>> avoid this would be to automatically assign each bridge device
>> to a different conntrack zone, but conntrack zones are limited
>> to 2^16 and this might also have other unwanted side-effects.
>>
>> Until we come up with something better the best fix seems to
>> be to perform the device lookup based on the iif.
>
> I don't think we can as the iif will point to the bridge device.
> The physindev contains the original physical device where the
> packet came in.
If it originally points to the bridge device, there doesn't
seem anything wrong with the device pointing to the bridge
device after reassembly. Am I missing something?
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox