* Re: [PATCH v13 net-next 07/11] bpf: verifier (add ability to receive verification log)
From: Daniel Borkmann @ 2014-09-18 14:50 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: David S. Miller, Ingo Molnar, Linus Torvalds, Andy Lutomirski,
Hannes Frederic Sowa, Chema Gonzalez, Eric Dumazet,
Peter Zijlstra, Pablo Neira Ayuso, H. Peter Anvin, Andrew Morton,
Kees Cook, Linux API, Network Development, LKML
In-Reply-To: <CAMEtUuyZCHh7SGeLF+g0ye3OrEFWnTZCn3SBeK0-8qz-y8U30w@mail.gmail.com>
On 09/18/2014 04:34 PM, Alexei Starovoitov wrote:
> On Wed, Sep 17, 2014 at 11:44 PM, Daniel Borkmann <dborkman@redhat.com> wrote:
...
>> Sure, you will never get a full compatibility on that regard
>> while backwards compatibility needs to be guaranteed on the
>> other hand. I looked at perf_copy_attr() implementation and I
>> think that we should mimic it in a very similar way as it
>> exactly solves what we need.
>>
>> For example, it will return with -EINVAL for (size > PAGE_SIZE)
>> and (size < PERF_ATTR_SIZE_VER0) where PAGE_SIZE has been chosen
>> as an arbitrary hard upper limit where it is believed that it will
>> never grow beyond that large limit in future.
>>
>> So this is a more loose constraint than what we currently do,
>> that is, -EINVAL on (size > sizeof(attr)) where attr is the
>> currently known size of a specific kernel. That would at least
>> be a start, you won't be able to cover everything though, but
>> it would allow to address the issue raised when running with
>> a basic feature set.
>
> you missed my point. We should not 'do a start', since it
> doesn't help user space in the long run and only makes
> kernel more complex.
Sorry, I don't think I missed your point. But if you see things
differently, fair enough, it was just a suggestion.
^ permalink raw reply
* (unknown),
From: Maria Caballero @ 2014-09-18 14:15 UTC (permalink / raw)
Loan Offer contact us for more details (gibonline11@gmail.com<mailto:gibonline11@gmail.com>)
All Details should be forward to this E-mail address for fast respond: gibonline11@gmail.com<mailto:gibonline11@gmail.com>
^ permalink raw reply
* Re: Figuring out how vti works
From: Joe M @ 2014-09-18 15:00 UTC (permalink / raw)
To: Steffen Klassert; +Cc: netdev@vger.kernel.org
In-Reply-To: <20140917052811.GT6390@secunet.com>
Hello Steffen,
Thanks for responding. Sorry that it took me some time to gather all
the information.
> Do you know where the packets are getting dropped?
All I can see from the below statistics is that the ip_vti0 tunnel is
getting picked up instead of the vtil tunnel.
> netstat -i or /proc/net/xfrm_stat could give a hint.
master# netstat -i
Kernel Interface table
Iface MTU RX-OK RX-ERR RX-DRP RX-OVR TX-OK TX-ERR TX-DRP TX-OVR Flg
enp4s0 1500 2310 0 57 0 1974 0 0 0 BMRU
ip_vti0 1428 0 2 0 0 0 10 0 0 ORU
lo 65536 600 0 0 0 600 0 0 0 LRU
vtil 1428 0 0 0 0 0 0 0 0 OPRU
master# ip -statistics xfrm state
src 192.168.0.11 dst <client or alice or bnglr public ip>
proto esp spi 0xc0b44648(3233039944) reqid 1(0x00000001) mode tunnel
replay-window 32 seq 0x00000000 flag af-unspec (0x00100000)
mark 1/0xffffffff
auth-trunc hmac(sha1)
0x6fb52dc437eb26b65bd0dced995aa27e78a7e869 (160 bits) 96
enc cbc(aes) 0x7e431bdc0ec138d0d8476c4afb0ccd63 (128 bits)
encap type espinudp sport 4500 dport 4500 addr 0.0.0.0
lifetime config:
limit: soft (INF)(bytes), hard (INF)(bytes)
limit: soft (INF)(packets), hard (INF)(packets)
expire add: soft 902(sec), hard 1200(sec)
expire use: soft 0(sec), hard 0(sec)
lifetime current:
0(bytes), 0(packets)
add 2014-09-17 17:38:06 use -
stats:
replay-window 0 replay 0 failed 0
src <client or alice or bnglr public ip> dst 192.168.0.11
proto esp spi 0xc514e2d3(3306480339) reqid 1(0x00000001) mode tunnel
replay-window 32 seq 0x00000000 flag af-unspec (0x00100000)
mark 1/0xffffffff
auth-trunc hmac(sha1)
0x8b4fd0749314d3656c962124e69c554ca03c9e11 (160 bits) 96
enc cbc(aes) 0x27e2a7cae3a24c20584e841a16dcf89d (128 bits)
encap type espinudp sport 4500 dport 4500 addr 0.0.0.0
lifetime config:
limit: soft (INF)(bytes), hard (INF)(bytes)
limit: soft (INF)(packets), hard (INF)(packets)
expire add: soft 848(sec), hard 1200(sec)
expire use: soft 0(sec), hard 0(sec)
lifetime current:
0(bytes), 0(packets)
add 2014-09-17 17:38:05 use -
stats:
replay-window 0 replay 0 failed 0
master# netstat -i
Kernel Interface table
Iface MTU RX-OK RX-ERR RX-DRP RX-OVR TX-OK TX-ERR TX-DRP TX-OVR Flg
enp4s0 1500 3214 0 69 0 2886 0 0 0 BMRU
ip_vti0 1428 0 2 0 0 0 21 0 0 ORU
lo 65536 629 0 0 0 629 0 0 0 LRU
vtil 1428 0 0 0 0 0 1 0 0 OPRU
master# ping -c 1 -I 192.168.0.11 192.168.1.232
PING 192.168.1.232 (192.168.1.232) from 192.168.0.11 : 56(84) bytes of data.
--- 192.168.1.232 ping statistics ---
1 packets transmitted, 0 received, 100% packet loss, time 0ms
master# netstat -i
Kernel Interface table
Iface MTU RX-OK RX-ERR RX-DRP RX-OVR TX-OK TX-ERR TX-DRP TX-OVR Flg
enp4s0 1500 3217 0 69 0 2890 0 0 0 BMRU
ip_vti0 1428 0 2 0 0 0 21 0 0 ORU
lo 65536 629 0 0 0 629 0 0 0 LRU
vtil 1428 0 0 0 0 0 1 0 0 OPRU
master# ping -c 1 -I 192.168.0.11 192.168.1.232
PING 192.168.1.232 (192.168.1.232) from 192.168.0.11 : 56(84) bytes of data.
--- 192.168.1.232 ping statistics ---
1 packets transmitted, 0 received, 100% packet loss, time 0ms
master# netstat -i
Kernel Interface table
Iface MTU RX-OK RX-ERR RX-DRP RX-OVR TX-OK TX-ERR TX-DRP TX-OVR Flg
enp4s0 1500 3230 0 69 0 2897 0 0 0 BMRU
ip_vti0 1428 0 2 0 0 0 22 0 0 ORU
lo 65536 629 0 0 0 629 0 0 0 LRU
vtil 1428 0 0 0 0 0 1 0 0 OPRU
master# ping -c 1 -I 192.168.0.11 192.168.1.232
PING 192.168.1.232 (192.168.1.232) from 192.168.0.11 : 56(84) bytes of data.
--- 192.168.1.232 ping statistics ---
1 packets transmitted, 0 received, 100% packet loss, time 0ms
master# netstat -i
Kernel Interface table
Iface MTU RX-OK RX-ERR RX-DRP RX-OVR TX-OK TX-ERR TX-DRP TX-OVR Flg
enp4s0 1500 3250 0 69 0 2912 0 0 0 BMRU
ip_vti0 1428 0 2 0 0 0 22 0 0 ORU
lo 65536 629 0 0 0 629 0 0 0 LRU
vtil 1428 0 0 0 0 0 1 0 0 OPRU
master#
>From what I gather, nothing seems to be going through the vtil tunnel.
The ip_vti0 seems to get used instead.
>> The pings worked fine if I remove the ip_vti and ip_tunnel modules,
>> the "mark=1" from /etc/ipsec.conf and the iptables mangle rules to
>> set-mark.
>
> You don't need to set the mark with iptables.
> You just have to ensure that the policy and state marks
> match the tunnel keys. I.e. direction in and forward must
> match the ikey, direction out must match the okey.
>
>>
>> sudo ip tunnel list
>> vtil: ip/ip remote 192.168.1.232 local 192.168.0.11 ttl inherit ikey 0 okey 1
>
> Your ikey does not match the policy and the state mark.
>
>>
>> sudo ip xfrm policy
>> src 192.168.1.0/24 dst 192.168.0.0/24
>> dir fwd priority 2883
>> mark 1/0xffffffff
>> tmpl src <bnglr public ip> dst 192.168.0.11
>> proto esp reqid 2 mode tunnel
>> src 192.168.1.0/24 dst 192.168.0.0/24
>> dir in priority 2883
>> mark 1/0xffffffff
>
> If you set mark 1 here, the tunnel should set ikey 1.
>
>>
>> I tried setting the mangle rules to set-mark but that did not help. I
>> could not find any more documentation.
>>
>
> Please try without setting a mark with netfilter.
>
I removed the iptables rules and set all policy to ACCEPT in iptables
raw, nat, mangle and raw tables.
master# echo "1" | sudo tee /proc/sys/net/ipv4/ip_forward
1
master# modprobe ip_vti
master# ipsec start
Starting strongSwan 5.2.0 IPsec [starter]...
master# ip tunnel add vtil mode vti local 192.168.0.11 remote
192.168.1.232 ikey 1 okey 1
master# ip link set vtil up
master# sleep 60
master# ip route add 192.168.1.0/24 dev vtil
master# ip xfrm state
src 192.168.0.11 dst <client or alice or bnglr public ip>
proto esp spi 0xc0b44648 reqid 1 mode tunnel
replay-window 32 flag af-unspec
mark 1/0xffffffff
auth-trunc hmac(sha1) 0x6fb52dc437eb26b65bd0dced995aa27e78a7e869 96
enc cbc(aes) 0x7e431bdc0ec138d0d8476c4afb0ccd63
encap type espinudp sport 4500 dport 4500 addr 0.0.0.0
src <client or alice or bnglr public ip> dst 192.168.0.11
proto esp spi 0xc514e2d3 reqid 1 mode tunnel
replay-window 32 flag af-unspec
mark 1/0xffffffff
auth-trunc hmac(sha1) 0x8b4fd0749314d3656c962124e69c554ca03c9e11 96
enc cbc(aes) 0x27e2a7cae3a24c20584e841a16dcf89d
encap type espinudp sport 4500 dport 4500 addr 0.0.0.0
master# ip xfrm policy
src 192.168.1.0/24 dst 192.168.0.0/24
dir fwd priority 2883
mark 1/0xffffffff
tmpl src <client or alice or bnglr public ip> dst 192.168.0.11
proto esp reqid 1 mode tunnel
src 192.168.1.0/24 dst 192.168.0.0/24
dir in priority 2883
mark 1/0xffffffff
tmpl src <client or alice or bnglr public ip> dst 192.168.0.11
proto esp reqid 1 mode tunnel
src 192.168.0.0/24 dst 192.168.1.0/24
dir out priority 2883
mark 1/0xffffffff
tmpl src 192.168.0.11 dst <client or alice or bnglr public ip>
proto esp reqid 1 mode tunnel
src 0.0.0.0/0 dst 0.0.0.0/0
socket in priority 0
src 0.0.0.0/0 dst 0.0.0.0/0
socket out priority 0
src 0.0.0.0/0 dst 0.0.0.0/0
socket in priority 0
src 0.0.0.0/0 dst 0.0.0.0/0
socket out priority 0
src ::/0 dst ::/0
socket in priority 0
src ::/0 dst ::/0
socket out priority 0
src ::/0 dst ::/0
socket in priority 0
src ::/0 dst ::/0
socket out priority 0
master# ip tunnel list
ip_vti0: ip/ip remote any local any ttl inherit nopmtudisc key 0
vtil: ip/ip remote 192.168.1.232 local 192.168.0.11 ttl inherit key 1
master# ip route list
default via 192.168.0.1 dev enp4s0 metric 202
127.0.0.0/8 dev lo scope host
192.168.0.0/24 dev enp4s0 proto kernel scope link src 192.168.0.11
metric 202
192.168.1.0/24 dev vtil scope link
master# uname -a
Linux master 3.16.3 #90 SMP PREEMPT Wed Sep 17 13:39:17 CDT 2014
x86_64 Intel(R) Pentium(R) CPU G620 @ 2.60GHz GenuineIntel GNU/Linux
master# ip -V
ip utility, iproute2-ss140804
sudo tcpdump -nS 'src port 500 or dst port 500 or src port 4500 or dst
port 4500' -i enp4s0
Password:
error : ret -1
tcpdump: verbose output suppressed, use -v or -vv for full protocol decode
listening on enp4s0, link-type EN10MB (Ethernet), capture size 65535 bytes
17:56:48.651871 IP 192.168.0.11.4500 > <client or alice or bnglr
public ip>.4500: isakmp-nat-keep-alive
17:56:56.934729 IP <client or alice or bnglr public ip>.4500 >
192.168.0.11.4500: isakmp-nat-keep-alive
17:57:08.652113 IP 192.168.0.11.4500 > <client or alice or bnglr
public ip>.4500: isakmp-nat-keep-alive
17:57:16.934548 IP <client or alice or bnglr public ip>.4500 >
192.168.0.11.4500: isakmp-nat-keep-alive
17:57:28.652359 IP 192.168.0.11.4500 > <client or alice or bnglr
public ip>.4500: isakmp-nat-keep-alive
17:57:36.938056 IP <client or alice or bnglr public ip>.4500 >
192.168.0.11.4500: isakmp-nat-keep-alive
17:57:48.652624 IP 192.168.0.11.4500 > <client or alice or bnglr
public ip>.4500: isakmp-nat-keep-alive
17:57:56.935926 IP <client or alice or bnglr public ip>.4500 >
192.168.0.11.4500: isakmp-nat-keep-alive
17:58:08.652872 IP 192.168.0.11.4500 > <client or alice or bnglr
public ip>.4500: isakmp-nat-keep-alive
17:58:17.005488 IP <client or alice or bnglr public ip>.4500 >
192.168.0.11.4500: isakmp-nat-keep-alive
and there is no tcpdump output on vtil interface.
master# cat /etc/ipsec.conf
# /etc/ipsec.conf - strongSwan IPsec configuration file
config setup
conn %default
ikelifetime=60m
keylife=20m
rekeymargin=3m
keyingtries=1
keyexchange=ikev2
mobike=no
conn master-bnglr
leftid="C=CH, O=strongSwan, CN=master"
leftcert=masterCert.der
left=192.168.0.11
leftsubnet=192.168.0.0/24
rightid="C=CH, O=strongSwan, CN=bnglr"
right=%any
rightsubnet=192.168.1.0/24
auto=add
mark=1
Any other thoughts, please?
Thanks
Joe
^ permalink raw reply
* [PATCH net] net: sched: shrink struct qdisc_skb_cb to 28 bytes
From: Eric Dumazet @ 2014-09-18 15:02 UTC (permalink / raw)
To: Or Gerlitz
Cc: Govindarajulu Varadarajan, Yinghai Lu, David Miller, NetDev,
ssujith, gvaradar, Christian Benvenuti (benve)
In-Reply-To: <CAJZOPZJ7+n0t1LCqaa3JCBByRw=f_Tp6rk8vm0ZnCzmtKFKX_g@mail.gmail.com>
From: Eric Dumazet <edumazet@google.com>
We cannot make struct qdisc_skb_cb bigger without impacting IPoIB,
or increasing skb->cb[] size.
Commit e0f31d849867 ("flow_keys: Record IP layer protocol in
skb_flow_dissect()") broke IPoIB.
Only current offender is sch_choke, and this one do not need an
absolutely precise flow key.
If we store 17 bytes of flow key, its more than enough. (Its the actual
size of flow_keys if it was a packed structure, but we might add new
fields at the end of it later)
Signed-off-by: Eric Dumazet <edumazet@google.com>
Fixes: e0f31d849867 ("flow_keys: Record IP layer protocol in skb_flow_dissect()")
---
include/net/sch_generic.h | 3 ++-
net/sched/sch_choke.c | 18 ++++++++++++++----
2 files changed, 16 insertions(+), 5 deletions(-)
diff --git a/include/net/sch_generic.h b/include/net/sch_generic.h
index a3cfb8ebeb53..620e086c0cbe 100644
--- a/include/net/sch_generic.h
+++ b/include/net/sch_generic.h
@@ -231,7 +231,8 @@ struct qdisc_skb_cb {
unsigned int pkt_len;
u16 slave_dev_queue_mapping;
u16 _pad;
- unsigned char data[24];
+#define QDISC_CB_PRIV_LEN 20
+ unsigned char data[QDISC_CB_PRIV_LEN];
};
static inline void qdisc_cb_private_validate(const struct sk_buff *skb, int sz)
diff --git a/net/sched/sch_choke.c b/net/sched/sch_choke.c
index ed30e436128b..fb666d1e4de3 100644
--- a/net/sched/sch_choke.c
+++ b/net/sched/sch_choke.c
@@ -133,10 +133,16 @@ static void choke_drop_by_idx(struct Qdisc *sch, unsigned int idx)
--sch->q.qlen;
}
+/* private part of skb->cb[] that a qdisc is allowed to use
+ * is limited to QDISC_CB_PRIV_LEN bytes.
+ * As a flow key might be too large, we store a part of it only.
+ */
+#define CHOKE_K_LEN min_t(u32, sizeof(struct flow_keys), QDISC_CB_PRIV_LEN - 3)
+
struct choke_skb_cb {
u16 classid;
u8 keys_valid;
- struct flow_keys keys;
+ u8 keys[QDISC_CB_PRIV_LEN - 3];
};
static inline struct choke_skb_cb *choke_skb_cb(const struct sk_buff *skb)
@@ -163,22 +169,26 @@ static u16 choke_get_classid(const struct sk_buff *skb)
static bool choke_match_flow(struct sk_buff *skb1,
struct sk_buff *skb2)
{
+ struct flow_keys temp;
+
if (skb1->protocol != skb2->protocol)
return false;
if (!choke_skb_cb(skb1)->keys_valid) {
choke_skb_cb(skb1)->keys_valid = 1;
- skb_flow_dissect(skb1, &choke_skb_cb(skb1)->keys);
+ skb_flow_dissect(skb1, &temp);
+ memcpy(&choke_skb_cb(skb1)->keys, &temp, CHOKE_K_LEN);
}
if (!choke_skb_cb(skb2)->keys_valid) {
choke_skb_cb(skb2)->keys_valid = 1;
- skb_flow_dissect(skb2, &choke_skb_cb(skb2)->keys);
+ skb_flow_dissect(skb2, &temp);
+ memcpy(&choke_skb_cb(skb2)->keys, &temp, CHOKE_K_LEN);
}
return !memcmp(&choke_skb_cb(skb1)->keys,
&choke_skb_cb(skb2)->keys,
- sizeof(struct flow_keys));
+ CHOKE_K_LEN);
}
/*
^ permalink raw reply related
* Re: [PATCH v13 net-next 07/11] bpf: verifier (add ability to receive verification log)
From: Alexei Starovoitov @ 2014-09-18 15:24 UTC (permalink / raw)
To: Daniel Borkmann
Cc: David S. Miller, Ingo Molnar, Linus Torvalds, Andy Lutomirski,
Hannes Frederic Sowa, Chema Gonzalez, Eric Dumazet,
Peter Zijlstra, Pablo Neira Ayuso, H. Peter Anvin, Andrew Morton,
Kees Cook, Linux API, Network Development, LKML
In-Reply-To: <541AF11C.9050405-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
On Thu, Sep 18, 2014 at 7:50 AM, Daniel Borkmann <dborkman-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
> On 09/18/2014 04:34 PM, Alexei Starovoitov wrote:
>>
>> On Wed, Sep 17, 2014 at 11:44 PM, Daniel Borkmann <dborkman-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
>> wrote:
>
> ...
>>>
>>> Sure, you will never get a full compatibility on that regard
>>> while backwards compatibility needs to be guaranteed on the
>>> other hand. I looked at perf_copy_attr() implementation and I
>>> think that we should mimic it in a very similar way as it
>>> exactly solves what we need.
>>>
>>> For example, it will return with -EINVAL for (size > PAGE_SIZE)
>>> and (size < PERF_ATTR_SIZE_VER0) where PAGE_SIZE has been chosen
>>> as an arbitrary hard upper limit where it is believed that it will
>>> never grow beyond that large limit in future.
>>>
>>> So this is a more loose constraint than what we currently do,
>>> that is, -EINVAL on (size > sizeof(attr)) where attr is the
>>> currently known size of a specific kernel. That would at least
>>> be a start, you won't be able to cover everything though, but
>>> it would allow to address the issue raised when running with
>>> a basic feature set.
>>
>>
>> you missed my point. We should not 'do a start', since it
>> doesn't help user space in the long run and only makes
>> kernel more complex.
>
>
> Sorry, I don't think I missed your point. But if you see things
> differently, fair enough, it was just a suggestion.
now you probably think I'm shutting you up. Sorry. Was not
my intention. Let me rephrase what I meant:
I think we should decide right now whether
'new user + old kernel' is really a problem we're going to
solve or not. If we decide to solve it, we need to have
a plan to solve it all the way. Partial fix for size of bpf_attr
is not a plan. It's something that is not addressing the problem
completely. Little bit of help is not useful for userspace. It
would need to deal with new types, verifier differences and
other things that I mentioned earlier. So we either decide
that we're going to spend time to solve all of them (not
necessarily today, but over long haul) or we're not doing
any of it.
^ permalink raw reply
* Re: CPU scheduler to TXQ binding? (ixgbe vs. igb)
From: Eric Dumazet @ 2014-09-18 15:42 UTC (permalink / raw)
To: Jesper Dangaard Brouer
Cc: Alexander Duyck, netdev@vger.kernel.org, Tom Herbert
In-Reply-To: <1411047689.7106.258.camel@edumazet-glaptop2.roam.corp.google.com>
On Thu, 2014-09-18 at 06:41 -0700, Eric Dumazet wrote:
> Last but not least, there is the fact that networking stacks use
> mod_timer() to arm timers, and that by default, timer migration is on
> ( cf /proc/sys/kernel/timer_migration )
>
> We probably should use mod_timer_pinned(), but I could not really see
> any difference.
Hmm... actually its quite noticeable :
# ./super_netperf 500 --google-pacing-rate 3000000 -H lpaa24 -l 1000 &
...
# echo 1 >/proc/sys/kernel/timer_migration
# vmstat 5
procs -----------memory---------- ---swap-- -----io---- -system-- ----cpu----
r b swpd free buff cache si so bi bo in cs us sy id wa
2 0 0 261178336 15812 1001880 0 0 5 1 185 217 0 4 96 0
0 0 0 261173456 15812 1001884 0 0 0 0 1548055 35472 0 15 85 0
2 0 0 261174880 15812 1001888 0 0 0 0 1533309 35163 0 15 85 0
3 0 0 261176768 15812 1001896 0 0 0 0 1533442 35694 0 15 85 0
2 0 0 261173584 15812 1001912 0 0 0 3 1524024 35489 0 16 83 0
3 0 0 261173344 15812 1001912 0 0 0 4 1525034 35392 0 15 85 0
2 0 0 261175840 15812 1001920 0 0 0 0 1545652 35772 0 15 84 0
3 0 0 261176800 15812 1001920 0 0 0 0 1513413 35703 0 15 85 0
0 0 0 261175136 15812 1001920 0 0 0 2 1528775 35639 0 15 85 0
1 0 0 261176480 15812 1001924 0 0 0 0 1510346 35364 0 15 85 0
0 0 0 261174624 15812 1001924 0 0 0 0 1523893 35669 0 15 85 0
0 0 0 261175568 15812 1001928 0 0 0 5 1524099 35605 0 15 85 0
2 0 0 261175776 15812 1001932 0 0 0 5 1510481 35631 0 15 85 0
2 0 0 261173776 15812 1001932 0 0 0 0 1528381 36127 0 15 84 0
3 0 0 261175424 15812 1001932 0 0 0 0 1508722 35402 0 15 85 0
1 0 0 261176048 15812 1001932 0 0 0 0 1495438 35280 0 15 85 0
^C
# echo 0 >/proc/sys/kernel/timer_migration
# vmstat 5
procs -----------memory---------- ---swap-- -----io---- -system-- ----cpu----
r b swpd free buff cache si so bi bo in cs us sy id wa
2 0 0 261172784 15812 1001936 0 0 5 1 165 228 0 5 95 0
1 0 0 261175776 15812 1001940 0 0 0 0 1187446 32238 0 12 88 0
2 0 0 261172752 15812 1001940 0 0 0 3 1166697 32060 0 12 88 0
1 0 0 261174528 15812 1001944 0 0 0 3 1156846 32048 0 12 88 0
1 0 0 261172688 15812 1001944 0 0 0 0 1152953 32048 0 12 88 0
0 0 0 261169888 15812 1001952 0 0 0 0 1143630 32710 0 12 88 0
2 0 0 261159936 15812 1001748 0 0 0 1016 1153256 32616 0 12 88 0
2 0 0 261162128 15812 1001936 0 0 0 0 1153065 32689 0 12 88 0
1 0 0 261171984 15812 1001936 0 0 0 3 1164407 32041 0 12 88 0
2 0 0 261169552 15812 1001936 0 0 0 5 1162068 31917 0 12 88 0
I am tempted to simply :
diff --git a/net/core/sock.c b/net/core/sock.c
index 9c3f823e76a9..868c6bcd7221 100644
--- a/net/core/sock.c
+++ b/net/core/sock.c
@@ -2288,10 +2288,10 @@ void sk_send_sigurg(struct sock *sk)
}
EXPORT_SYMBOL(sk_send_sigurg);
-void sk_reset_timer(struct sock *sk, struct timer_list* timer,
+void sk_reset_timer(struct sock *sk, struct timer_list *timer,
unsigned long expires)
{
- if (!mod_timer(timer, expires))
+ if (!mod_timer_pinned(timer, expires))
sock_hold(sk);
}
EXPORT_SYMBOL(sk_reset_timer);
^ permalink raw reply related
* Re: [PATCHv2 1/1] bluetooth: Check for SCO type before setting retransmission effort
From: Marcel Holtmann @ 2014-09-18 15:43 UTC (permalink / raw)
To: Bernhard Thaler
Cc: Gustavo F. Padovan, Johan Hedberg, David S. Miller,
BlueZ development, Network Development, linux-kernel
In-Reply-To: <1411035321-10215-1-git-send-email-bernhard.thaler@r-it.at>
Hi Bernhard,
> SCO connection cannot be setup to devices that do not support retransmission.
> Patch based on http://permalink.gmane.org/gmane.linux.bluez.kernel/7779 and
> adapted for this kernel version.
> Code changed to check SCO/eSCO type before setting retransmission effort
> and max. latency. The purpose of the patch is to support older devices not
> capable of eSCO.
>
> Tested on Blackberry 655+ headset which does not support retransmission.
> Credits go to Alexander Sommerhuber.
>
> Signed-off-by: Bernhard Thaler <bernhard.thaler@r-it.at>
> ---
> net/bluetooth/hci_conn.c | 16 ++++++++++++----
> 1 file changed, 12 insertions(+), 4 deletions(-)
>
> diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
> index faff624..f6e3183 100644
> --- a/net/bluetooth/hci_conn.c
> +++ b/net/bluetooth/hci_conn.c
> @@ -194,10 +194,18 @@ bool hci_setup_sync(struct hci_conn *conn, __u16 handle)
> param = &sco_param_wideband[conn->attempt - 1];
> break;
> case SCO_AIRMODE_CVSD:
> - if (conn->attempt > ARRAY_SIZE(sco_param_cvsd))
> - return false;
> - cp.retrans_effort = 0x01;
> - param = &sco_param_cvsd[conn->attempt - 1];
> + if (!lmp_esco_capable(conn->link)) {
> + /* sco_param_cvsd contains SCO in last two elements */
> + if ((conn->attempt + 3) > ARRAY_SIZE(sco_param_cvsd))
> + return false;
> + cp.retrans_effort = 0xff;
> + param = &sco_param_cvsd[(conn->attempt + 2)];
so if anybody creates a new mode and forgets to update this code, we are hitting a nice crash and/or wrong behavior.
I would prefer that we create separate parameter arrays here. It might means that we have to rename the current ones to esco_param_* and the introduce sco_param_cvsd.
> + } else {
> + if (conn->attempt > ARRAY_SIZE(sco_param_cvsd))
> + return false;
> + cp.retrans_effort = 0x01;
> + param = &sco_param_cvsd[conn->attempt - 1];
> + }
> break;
> default:
> return false;
Regards
Marcel
^ permalink raw reply
* Re: [PATCHv2 1/1] bluetooth: Check for SCO type before setting retransmission effort
From: Marcel Holtmann @ 2014-09-18 15:48 UTC (permalink / raw)
To: Bernhard Thaler
Cc: Gustavo F. Padovan, Johan Hedberg, David S. Miller,
BlueZ development, Network Development, linux-kernel
In-Reply-To: <1411035321-10215-1-git-send-email-bernhard.thaler@r-it.at>
Hi Bernhard,
> SCO connection cannot be setup to devices that do not support retransmission.
> Patch based on http://permalink.gmane.org/gmane.linux.bluez.kernel/7779 and
> adapted for this kernel version.
> Code changed to check SCO/eSCO type before setting retransmission effort
> and max. latency. The purpose of the patch is to support older devices not
> capable of eSCO.
>
> Tested on Blackberry 655+ headset which does not support retransmission.
> Credits go to Alexander Sommerhuber.
>
> Signed-off-by: Bernhard Thaler <bernhard.thaler@r-it.at>
> ---
> net/bluetooth/hci_conn.c | 16 ++++++++++++----
> 1 file changed, 12 insertions(+), 4 deletions(-)
>
> diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
> index faff624..f6e3183 100644
> --- a/net/bluetooth/hci_conn.c
> +++ b/net/bluetooth/hci_conn.c
> @@ -194,10 +194,18 @@ bool hci_setup_sync(struct hci_conn *conn, __u16 handle)
> param = &sco_param_wideband[conn->attempt - 1];
> break;
> case SCO_AIRMODE_CVSD:
> - if (conn->attempt > ARRAY_SIZE(sco_param_cvsd))
> - return false;
> - cp.retrans_effort = 0x01;
> - param = &sco_param_cvsd[conn->attempt - 1];
> + if (!lmp_esco_capable(conn->link)) {
> + /* sco_param_cvsd contains SCO in last two elements */
> + if ((conn->attempt + 3) > ARRAY_SIZE(sco_param_cvsd))
> + return false;
> + cp.retrans_effort = 0xff;
> + param = &sco_param_cvsd[(conn->attempt + 2)];
so if anybody creates a new mode and forgets to update this code, we are hitting a nice crash and/or wrong behavior.
I would prefer that we create separate parameter arrays here. It might means that we have to rename the current ones to esco_param_* and the introduce sco_param_cvsd.
> + } else {
> + if (conn->attempt > ARRAY_SIZE(sco_param_cvsd))
> + return false;
> + cp.retrans_effort = 0x01;
> + param = &sco_param_cvsd[conn->attempt - 1];
> + }
> break;
> default:
> return false;
Regards
Marcel
^ permalink raw reply
* Re: CPU scheduler to TXQ binding? (ixgbe vs. igb)
From: Jesper Dangaard Brouer @ 2014-09-18 15:59 UTC (permalink / raw)
To: Eric Dumazet; +Cc: Alexander Duyck, netdev@vger.kernel.org, Tom Herbert
In-Reply-To: <1411054951.7106.272.camel@edumazet-glaptop2.roam.corp.google.com>
On Thu, 18 Sep 2014 08:42:31 -0700
Eric Dumazet <eric.dumazet@gmail.com> wrote:
> On Thu, 2014-09-18 at 06:41 -0700, Eric Dumazet wrote:
>
> > Last but not least, there is the fact that networking stacks use
> > mod_timer() to arm timers, and that by default, timer migration is on
> > ( cf /proc/sys/kernel/timer_migration )
I don't have this proc file on my system, as I didn't select CONFIG_SCHED_DEBUG.
> > We probably should use mod_timer_pinned(), but I could not really see
> > any difference.
>
> Hmm... actually its quite noticeable :
Interesting impact.
I'm looking for some 1G hardware without multiqueue, so I can get
around this measurement constraint. And possibly turning it down to
100Mbit/s, so I can more easily measure the HoL blocking effect.
> # ./super_netperf 500 --google-pacing-rate 3000000 -H lpaa24 -l 1000 &
> ...
Interesting option "--google-pacing-rate" ;-)
> # echo 1 >/proc/sys/kernel/timer_migration
> # vmstat 5
> procs -----------memory---------- ---swap-- -----io---- -system-- ----cpu----
> r b swpd free buff cache si so bi bo in cs us sy id wa
> 2 0 0 261178336 15812 1001880 0 0 5 1 185 217 0 4 96 0
> 0 0 0 261173456 15812 1001884 0 0 0 0 1548055 35472 0 15 85 0
> 2 0 0 261174880 15812 1001888 0 0 0 0 1533309 35163 0 15 85 0
> 3 0 0 261176768 15812 1001896 0 0 0 0 1533442 35694 0 15 85 0
[]
> # echo 0 >/proc/sys/kernel/timer_migration
> # vmstat 5
> procs -----------memory---------- ---swap-- -----io---- -system-- ----cpu----
> r b swpd free buff cache si so bi bo in cs us sy id wa
> 2 0 0 261172784 15812 1001936 0 0 5 1 165 228 0 5 95 0
> 1 0 0 261175776 15812 1001940 0 0 0 0 1187446 32238 0 12 88 0
> 2 0 0 261172752 15812 1001940 0 0 0 3 1166697 32060 0 12 88 0
Quite significant, both interrupts and especially CPU system usage drop.
> I am tempted to simply :
>
> diff --git a/net/core/sock.c b/net/core/sock.c
> index 9c3f823e76a9..868c6bcd7221 100644
> --- a/net/core/sock.c
> +++ b/net/core/sock.c
> @@ -2288,10 +2288,10 @@ void sk_send_sigurg(struct sock *sk)
> }
> EXPORT_SYMBOL(sk_send_sigurg);
>
> -void sk_reset_timer(struct sock *sk, struct timer_list* timer,
> +void sk_reset_timer(struct sock *sk, struct timer_list *timer,
> unsigned long expires)
> {
> - if (!mod_timer(timer, expires))
> + if (!mod_timer_pinned(timer, expires))
> sock_hold(sk);
> }
> EXPORT_SYMBOL(sk_reset_timer);
>
--
Best regards,
Jesper Dangaard Brouer
MSc.CS, Sr. Network Kernel Developer at Red Hat
Author of http://www.iptv-analyzer.org
LinkedIn: http://www.linkedin.com/in/brouer
^ permalink raw reply
* Re: CPU scheduler to TXQ binding? (ixgbe vs. igb)
From: Eric Dumazet @ 2014-09-18 16:07 UTC (permalink / raw)
To: Jesper Dangaard Brouer
Cc: Alexander Duyck, netdev@vger.kernel.org, Tom Herbert
In-Reply-To: <1411054951.7106.272.camel@edumazet-glaptop2.roam.corp.google.com>
On Thu, 2014-09-18 at 08:42 -0700, Eric Dumazet wrote:
> On Thu, 2014-09-18 at 06:41 -0700, Eric Dumazet wrote:
>
> > Last but not least, there is the fact that networking stacks use
> > mod_timer() to arm timers, and that by default, timer migration is on
> > ( cf /proc/sys/kernel/timer_migration )
> >
> > We probably should use mod_timer_pinned(), but I could not really see
> > any difference.
>
> Hmm... actually its quite noticeable :
>
> # ./super_netperf 500 --google-pacing-rate 3000000 -H lpaa24 -l 1000 &
> ...
> # echo 1 >/proc/sys/kernel/timer_migration
> # vmstat 5
> procs -----------memory---------- ---swap-- -----io---- -system-- ----cpu----
> r b swpd free buff cache si so bi bo in cs us sy id wa
> 2 0 0 261178336 15812 1001880 0 0 5 1 185 217 0 4 96 0
> 0 0 0 261173456 15812 1001884 0 0 0 0 1548055 35472 0 15 85 0
> 2 0 0 261174880 15812 1001888 0 0 0 0 1533309 35163 0 15 85 0
> 3 0 0 261176768 15812 1001896 0 0 0 0 1533442 35694 0 15 85 0
> 2 0 0 261173584 15812 1001912 0 0 0 3 1524024 35489 0 16 83 0
> 3 0 0 261173344 15812 1001912 0 0 0 4 1525034 35392 0 15 85 0
> 2 0 0 261175840 15812 1001920 0 0 0 0 1545652 35772 0 15 84 0
> 3 0 0 261176800 15812 1001920 0 0 0 0 1513413 35703 0 15 85 0
> 0 0 0 261175136 15812 1001920 0 0 0 2 1528775 35639 0 15 85 0
> 1 0 0 261176480 15812 1001924 0 0 0 0 1510346 35364 0 15 85 0
> 0 0 0 261174624 15812 1001924 0 0 0 0 1523893 35669 0 15 85 0
> 0 0 0 261175568 15812 1001928 0 0 0 5 1524099 35605 0 15 85 0
> 2 0 0 261175776 15812 1001932 0 0 0 5 1510481 35631 0 15 85 0
> 2 0 0 261173776 15812 1001932 0 0 0 0 1528381 36127 0 15 84 0
> 3 0 0 261175424 15812 1001932 0 0 0 0 1508722 35402 0 15 85 0
> 1 0 0 261176048 15812 1001932 0 0 0 0 1495438 35280 0 15 85 0
> ^C
> # echo 0 >/proc/sys/kernel/timer_migration
> # vmstat 5
> procs -----------memory---------- ---swap-- -----io---- -system-- ----cpu----
> r b swpd free buff cache si so bi bo in cs us sy id wa
> 2 0 0 261172784 15812 1001936 0 0 5 1 165 228 0 5 95 0
> 1 0 0 261175776 15812 1001940 0 0 0 0 1187446 32238 0 12 88 0
> 2 0 0 261172752 15812 1001940 0 0 0 3 1166697 32060 0 12 88 0
> 1 0 0 261174528 15812 1001944 0 0 0 3 1156846 32048 0 12 88 0
> 1 0 0 261172688 15812 1001944 0 0 0 0 1152953 32048 0 12 88 0
> 0 0 0 261169888 15812 1001952 0 0 0 0 1143630 32710 0 12 88 0
> 2 0 0 261159936 15812 1001748 0 0 0 1016 1153256 32616 0 12 88 0
> 2 0 0 261162128 15812 1001936 0 0 0 0 1153065 32689 0 12 88 0
> 1 0 0 261171984 15812 1001936 0 0 0 3 1164407 32041 0 12 88 0
> 2 0 0 261169552 15812 1001936 0 0 0 5 1162068 31917 0 12 88 0
>
> I am tempted to simply :
>
> diff --git a/net/core/sock.c b/net/core/sock.c
> index 9c3f823e76a9..868c6bcd7221 100644
> --- a/net/core/sock.c
> +++ b/net/core/sock.c
> @@ -2288,10 +2288,10 @@ void sk_send_sigurg(struct sock *sk)
> }
> EXPORT_SYMBOL(sk_send_sigurg);
>
> -void sk_reset_timer(struct sock *sk, struct timer_list* timer,
> +void sk_reset_timer(struct sock *sk, struct timer_list *timer,
> unsigned long expires)
> {
> - if (!mod_timer(timer, expires))
> + if (!mod_timer_pinned(timer, expires))
> sock_hold(sk);
> }
> EXPORT_SYMBOL(sk_reset_timer);
>
And/or changing all occurences of HRTIMER_MODE_ABS in net/sched
into HRTIMER_MODE_ABS_PINNED
Because we _want_ qdisc being restarted on the right cpu for sure.
^ permalink raw reply
* Re: [PATCH net] net: sched: shrink struct qdisc_skb_cb to 28 bytes
From: Stephen Hemminger @ 2014-09-18 16:26 UTC (permalink / raw)
To: Eric Dumazet
Cc: Or Gerlitz, Govindarajulu Varadarajan, Yinghai Lu, David Miller,
NetDev, ssujith, gvaradar, Christian Benvenuti (benve)
In-Reply-To: <1411052525.7106.269.camel@edumazet-glaptop2.roam.corp.google.com>
On Thu, 18 Sep 2014 08:02:05 -0700
Eric Dumazet <eric.dumazet@gmail.com> wrote:
> From: Eric Dumazet <edumazet@google.com>
>
> We cannot make struct qdisc_skb_cb bigger without impacting IPoIB,
> or increasing skb->cb[] size.
>
> Commit e0f31d849867 ("flow_keys: Record IP layer protocol in
> skb_flow_dissect()") broke IPoIB.
>
> Only current offender is sch_choke, and this one do not need an
> absolutely precise flow key.
>
> If we store 17 bytes of flow key, its more than enough. (Its the actual
> size of flow_keys if it was a packed structure, but we might add new
> fields at the end of it later)
>
> Signed-off-by: Eric Dumazet <edumazet@google.com>
Can we add BUILD_BUG to stop next time something smacks this.
^ permalink raw reply
* Re: [net-next PATCH 2/2] net: sched: cls_u32 changes to knode must appear atomic to readers
From: Cong Wang @ 2014-09-18 16:28 UTC (permalink / raw)
To: John Fastabend
Cc: Cong Wang, David Miller, Eric Dumazet, netdev, Jamal Hadi Salim
In-Reply-To: <541A21F8.3000206@gmail.com>
On Wed, Sep 17, 2014 at 5:06 PM, John Fastabend
<john.fastabend@gmail.com> wrote:
>
> Sure how about this,
>
> /* u32_delete_key_rcu_pf should be called when free'ing a copied
> * version of a tc_u_knode obtained from u32_init_knode(). When
> * copies are obtained from u32_init_knode() the statistics are
> * shared between the old and new copies to allow readers to
> * continue to update the statistics during the copy. To support
> * this the u32_delete_key_rcu_pf variant does not free the percpu
> * statistics.
> */
>
Looks good, but why you pick the name u32_delete_key_rcu_pf()?
It sounds like you will free pf, but actually you will skip freeing pf. :)
^ permalink raw reply
* Re: [PATCH net] net: sched: shrink struct qdisc_skb_cb to 28 bytes
From: Eric Dumazet @ 2014-09-18 16:32 UTC (permalink / raw)
To: Stephen Hemminger
Cc: Or Gerlitz, Govindarajulu Varadarajan, Yinghai Lu, David Miller,
NetDev, ssujith, gvaradar, Christian Benvenuti (benve)
In-Reply-To: <20140918092628.566eae2b@urahara>
On Thu, 2014-09-18 at 09:26 -0700, Stephen Hemminger wrote:
> On Thu, 18 Sep 2014 08:02:05 -0700
> Eric Dumazet <eric.dumazet@gmail.com> wrote:
>
> > From: Eric Dumazet <edumazet@google.com>
> >
> > We cannot make struct qdisc_skb_cb bigger without impacting IPoIB,
> > or increasing skb->cb[] size.
> >
> > Commit e0f31d849867 ("flow_keys: Record IP layer protocol in
> > skb_flow_dissect()") broke IPoIB.
> >
> > Only current offender is sch_choke, and this one do not need an
> > absolutely precise flow key.
> >
> > If we store 17 bytes of flow key, its more than enough. (Its the actual
> > size of flow_keys if it was a packed structure, but we might add new
> > fields at the end of it later)
> >
> > Signed-off-by: Eric Dumazet <edumazet@google.com>
>
> Can we add BUILD_BUG to stop next time something smacks this.
I though we had.
Maybe IPoIB lacks one.
Or, do you have an idea ?
^ permalink raw reply
* Re: CPU scheduler to TXQ binding? (ixgbe vs. igb)
From: Eric Dumazet @ 2014-09-18 16:34 UTC (permalink / raw)
To: Jesper Dangaard Brouer
Cc: Alexander Duyck, netdev@vger.kernel.org, Tom Herbert
In-Reply-To: <20140918175910.5fc67efa@redhat.com>
On Thu, 2014-09-18 at 17:59 +0200, Jesper Dangaard Brouer wrote:
> On Thu, 18 Sep 2014 08:42:31 -0700
> Eric Dumazet <eric.dumazet@gmail.com> wrote:
>
> > On Thu, 2014-09-18 at 06:41 -0700, Eric Dumazet wrote:
> >
> > > Last but not least, there is the fact that networking stacks use
> > > mod_timer() to arm timers, and that by default, timer migration is on
> > > ( cf /proc/sys/kernel/timer_migration )
>
> I don't have this proc file on my system, as I didn't select CONFIG_SCHED_DEBUG.
Interesting... this timer_migration stuff seems a bit scary to me.
>
> > > We probably should use mod_timer_pinned(), but I could not really see
> > > any difference.
> >
> > Hmm... actually its quite noticeable :
>
> Interesting impact.
>
> I'm looking for some 1G hardware without multiqueue, so I can get
> around this measurement constraint. And possibly turning it down to
> 100Mbit/s, so I can more easily measure the HoL blocking effect.
>
ethtool -L eth0 rx 1 tx 1
(Or similar if combined is used)
>
> > # ./super_netperf 500 --google-pacing-rate 3000000 -H lpaa24 -l 1000 &
> > ...
>
> Interesting option "--google-pacing-rate" ;-)
Its using upstream SO_MAX_PACING_RATE, nothing fancy ;)
>
> > # echo 1 >/proc/sys/kernel/timer_migration
> > # vmstat 5
> > procs -----------memory---------- ---swap-- -----io---- -system-- ----cpu----
> > r b swpd free buff cache si so bi bo in cs us sy id wa
> > 2 0 0 261178336 15812 1001880 0 0 5 1 185 217 0 4 96 0
> > 0 0 0 261173456 15812 1001884 0 0 0 0 1548055 35472 0 15 85 0
> > 2 0 0 261174880 15812 1001888 0 0 0 0 1533309 35163 0 15 85 0
> > 3 0 0 261176768 15812 1001896 0 0 0 0 1533442 35694 0 15 85 0
> []
>
> > # echo 0 >/proc/sys/kernel/timer_migration
> > # vmstat 5
> > procs -----------memory---------- ---swap-- -----io---- -system-- ----cpu----
> > r b swpd free buff cache si so bi bo in cs us sy id wa
> > 2 0 0 261172784 15812 1001936 0 0 5 1 165 228 0 5 95 0
> > 1 0 0 261175776 15812 1001940 0 0 0 0 1187446 32238 0 12 88 0
> > 2 0 0 261172752 15812 1001940 0 0 0 3 1166697 32060 0 12 88 0
>
> Quite significant, both interrupts and especially CPU system usage drop.
>
Yep...
^ permalink raw reply
* Re: [net-next PATCH 2/2] net: sched: cls_u32 changes to knode must appear atomic to readers
From: John Fastabend @ 2014-09-18 16:39 UTC (permalink / raw)
To: Cong Wang; +Cc: Cong Wang, David Miller, Eric Dumazet, netdev, Jamal Hadi Salim
In-Reply-To: <CAHA+R7Nk6kO_eXLYppwimX5Cp-QpzHGVo4yHCO+4xSf-kcpqkw@mail.gmail.com>
On 09/18/2014 09:28 AM, Cong Wang wrote:
> On Wed, Sep 17, 2014 at 5:06 PM, John Fastabend
> <john.fastabend@gmail.com> wrote:
>>
>> Sure how about this,
>>
>> /* u32_delete_key_rcu_pf should be called when free'ing a copied
>> * version of a tc_u_knode obtained from u32_init_knode(). When
>> * copies are obtained from u32_init_knode() the statistics are
>> * shared between the old and new copies to allow readers to
>> * continue to update the statistics during the copy. To support
>> * this the u32_delete_key_rcu_pf variant does not free the percpu
>> * statistics.
>> */
>>
>
> Looks good, but why you pick the name u32_delete_key_rcu_pf()?
> It sounds like you will free pf, but actually you will skip freeing pf. :)
>
hmm it was just the first thing that came to mind.
I'll rename the two calls to make it clear when reading the code.
Thanks,
John
--
John Fastabend Intel Corporation
^ permalink raw reply
* [PATCH] net/netfilter/x_tables.c: use __seq_open_private()
From: Rob Jones @ 2014-09-18 16:48 UTC (permalink / raw)
To: davem; +Cc: netfilter, coreteam, netdev, linux-kernel, linux-kernel,
rob.jones
Reduce boilerplate code by using __seq_open_private() instead of seq_open()
in xt_match_open() and xt_target_open().
Signed-off-by: Rob Jones <rob.jones@codethink.co.uk>
---
This patch uses an existing variant of seq_open() to reduce the kernel code
size.
The only significant variation from the pre-existing code is the fact that
__seq_open_private() calls kzalloc() rather than kmalloc(), which could
conceivably have an impact on timing.
net/netfilter/x_tables.c | 30 ++++--------------------------
1 file changed, 4 insertions(+), 26 deletions(-)
diff --git a/net/netfilter/x_tables.c b/net/netfilter/x_tables.c
index 227aa11..89aa680 100644
--- a/net/netfilter/x_tables.c
+++ b/net/netfilter/x_tables.c
@@ -1137,22 +1137,11 @@ static const struct seq_operations xt_match_seq_ops = {
static int xt_match_open(struct inode *inode, struct file *file)
{
- struct seq_file *seq;
struct nf_mttg_trav *trav;
- int ret;
-
- trav = kmalloc(sizeof(*trav), GFP_KERNEL);
- if (trav == NULL)
+ trav = __seq_open_private(file, &xt_match_seq_ops, sizeof(*trav));
+ if (!trav)
return -ENOMEM;
- ret = seq_open(file, &xt_match_seq_ops);
- if (ret < 0) {
- kfree(trav);
- return ret;
- }
-
- seq = file->private_data;
- seq->private = trav;
trav->nfproto = (unsigned long)PDE_DATA(inode);
return 0;
}
@@ -1201,22 +1190,11 @@ static const struct seq_operations xt_target_seq_ops = {
static int xt_target_open(struct inode *inode, struct file *file)
{
- struct seq_file *seq;
struct nf_mttg_trav *trav;
- int ret;
-
- trav = kmalloc(sizeof(*trav), GFP_KERNEL);
- if (trav == NULL)
+ trav = __seq_open_private(file, &xt_target_seq_ops, sizeof(*trav));
+ if (!trav)
return -ENOMEM;
- ret = seq_open(file, &xt_target_seq_ops);
- if (ret < 0) {
- kfree(trav);
- return ret;
- }
-
- seq = file->private_data;
- seq->private = trav;
trav->nfproto = (unsigned long)PDE_DATA(inode);
return 0;
}
--
1.7.10.4
^ permalink raw reply related
* Re: Calltrace in process_backlog()
From: Cong Wang @ 2014-09-18 16:52 UTC (permalink / raw)
To: sdrb; +Cc: netdev
In-Reply-To: <alpine.LNX.2.03.1409181138160.5266@127.0.0.1>
On Thu, Sep 18, 2014 at 2:53 AM, <sdrb@onet.eu> wrote:
> I know that the this problem occures because there is napi->poll_list freed
> twice. But I cannot find out where in my code is the source
> of the problem. How can I find it?
>
> The system bases on single core ARM.
>
> Any suggestions?
Try to enable some VM debug option, for example if you use SLUB enable
CONFIG_SLUB_DEBUG=y, it should be able to catch double free bug.
^ permalink raw reply
* Re: [PATCH net v2 1/2] genetlink: add function genl_has_listeners()
From: Pravin Shelar @ 2014-09-18 17:11 UTC (permalink / raw)
To: Nicolas Dichtel; +Cc: David Miller, dev@openvswitch.org, netdev
In-Reply-To: <1411029064-15376-1-git-send-email-nicolas.dichtel@6wind.com>
On Thu, Sep 18, 2014 at 1:31 AM, Nicolas Dichtel
<nicolas.dichtel@6wind.com> wrote:
> This function is the counterpart of the function netlink_has_listeners().
>
> Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
Looks good.
Acked-by: Pravin B Shelar <pshelar@nicira.com>
> ---
>
> v2: add patch 1/2
>
> include/net/genetlink.h | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
> diff --git a/include/net/genetlink.h b/include/net/genetlink.h
> index 93695f0e22a5..af10c2cf8a1d 100644
> --- a/include/net/genetlink.h
> +++ b/include/net/genetlink.h
> @@ -394,4 +394,12 @@ static inline int genl_set_err(struct genl_family *family, struct net *net,
> return netlink_set_err(net->genl_sock, portid, group, code);
> }
>
> +static inline int genl_has_listeners(struct genl_family *family,
> + struct sock *sk, unsigned int group)
> +{
> + if (WARN_ON_ONCE(group >= family->n_mcgrps))
> + return -EINVAL;
> + group = family->mcgrp_offset + group;
> + return netlink_has_listeners(sk, group);
> +}
> #endif /* __NET_GENERIC_NETLINK_H */
> --
> 2.1.0
>
^ permalink raw reply
* Re: [PATCH net v2 2/2] openvswitch: restore OVS_FLOW_CMD_NEW notifications
From: Pravin Shelar @ 2014-09-18 17:11 UTC (permalink / raw)
To: Nicolas Dichtel
Cc: David Miller, dev@openvswitch.org, netdev, Samuel Gauthier
In-Reply-To: <1411029064-15376-2-git-send-email-nicolas.dichtel@6wind.com>
On Thu, Sep 18, 2014 at 1:31 AM, Nicolas Dichtel
<nicolas.dichtel@6wind.com> wrote:
> From: Samuel Gauthier <samuel.gauthier@6wind.com>
>
> Since commit fb5d1e9e127a ("openvswitch: Build flow cmd netlink reply only if needed."),
> the new flows are not notified to the listeners of OVS_FLOW_MCGROUP.
>
> This commit fixes the problem by using the genl function, ie
> genl_has_listerners() instead of netlink_has_listeners().
>
> Signed-off-by: Samuel Gauthier <samuel.gauthier@6wind.com>
> Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
Thanks for the fix.
Acked-by: Pravin B Shelar <pshelar@nicira.com>
> ---
>
> v2: add patch 1/2
>
> net/openvswitch/datapath.c | 9 +++++----
> 1 file changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/net/openvswitch/datapath.c b/net/openvswitch/datapath.c
> index 91d66b7e64ac..64dc864a417f 100644
> --- a/net/openvswitch/datapath.c
> +++ b/net/openvswitch/datapath.c
> @@ -78,11 +78,12 @@ static const struct genl_multicast_group ovs_dp_vport_multicast_group = {
>
> /* Check if need to build a reply message.
> * OVS userspace sets the NLM_F_ECHO flag if it needs the reply. */
> -static bool ovs_must_notify(struct genl_info *info,
> - const struct genl_multicast_group *grp)
> +static bool ovs_must_notify(struct genl_family *family, struct genl_info *info,
> + unsigned int group)
> {
> return info->nlhdr->nlmsg_flags & NLM_F_ECHO ||
> - netlink_has_listeners(genl_info_net(info)->genl_sock, 0);
> + genl_has_listeners(family, genl_info_net(info)->genl_sock,
> + group);
> }
>
> static void ovs_notify(struct genl_family *family,
> @@ -763,7 +764,7 @@ static struct sk_buff *ovs_flow_cmd_alloc_info(const struct sw_flow_actions *act
> {
> struct sk_buff *skb;
>
> - if (!always && !ovs_must_notify(info, &ovs_dp_flow_multicast_group))
> + if (!always && !ovs_must_notify(&dp_flow_genl_family, info, 0))
> return NULL;
>
> skb = genlmsg_new_unicast(ovs_flow_cmd_msg_size(acts), info, GFP_KERNEL);
> --
> 2.1.0
>
^ permalink raw reply
* Re: [PATCH v13 net-next 07/11] bpf: verifier (add ability to receive verification log)
From: Daniel Borkmann @ 2014-09-18 17:28 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: David S. Miller, Ingo Molnar, Linus Torvalds, Andy Lutomirski,
Hannes Frederic Sowa, Chema Gonzalez, Eric Dumazet,
Peter Zijlstra, Pablo Neira Ayuso, H. Peter Anvin, Andrew Morton,
Kees Cook, Linux API, Network Development, LKML
In-Reply-To: <CAMEtUuykgG70sP0tAmovLvPxgjwaR8h=R-c6Tyo6m96+Lg3sXQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On 09/18/2014 05:24 PM, Alexei Starovoitov wrote:
...
> solve or not. If we decide to solve it, we need to have
> a plan to solve it all the way. Partial fix for size of bpf_attr
> is not a plan. It's something that is not addressing the problem
> completely. Little bit of help is not useful for userspace. It
> would need to deal with new types, verifier differences and
> other things that I mentioned earlier.
Hm, I don't think it would be a strict requirement to solve it
all the way, and I think that perf_event_open() with perf_copy_attr()
is not trying to do so either. It, however, is trying on a ``best
effort basis'' to still load something if new features are unused
by the binary (I guess you saw the comment in perf_copy_attr()).
Iff, e.g. due to new types we fail at the verifier stage, sure,
that's life since we only have backwards-compatible guarantee,
but in case we tried to use features we support, we're still able
to load the eBPF program while right now, we're rejecting it right
up-front. That's just my $0.02 ...
^ permalink raw reply
* Re: Cannot move macvlan interface on top of bonding device
From: Francesco Ruggeri @ 2014-09-18 17:36 UTC (permalink / raw)
To: Florian Westphal
Cc: Vlad Yasevich, Cong Wang, netdev, David S. Miller, Weilong Chen
>
>> May be a better solution is:
>>
>> features = netdev_increment_features(vlan->lowerdev->features & MACVLAN_FEATURS,
>> features,
>> mask);
>>
>> This way we start with only the feature MACVLAN is interested in.
>
> Seems correct, since it will also prevent other non-macvlan features from
> appearing.
>
> Thanks Vlad.
I verified that this approach also solves my original problem (moving
a macvlan on top of a bonding device to a different namespace).
I do not know how to fully test it with different types of lower
devices and features though.
If anybody wants to take the lead on this it would be great. Otherwise
a more limited patch only clearing the NETIF_F_NETNS_LOCAL bit was
also submitted earlier
(http://marc.info/?l=linux-netdev&m=141097807310566&w=2).
Thanks,
Francesco
^ permalink raw reply
* Re: [PATCH net] net: sched: shrink struct qdisc_skb_cb to 28 bytes
From: Eric Dumazet @ 2014-09-18 18:00 UTC (permalink / raw)
To: Stephen Hemminger
Cc: Or Gerlitz, Govindarajulu Varadarajan, Yinghai Lu, David Miller,
NetDev, ssujith, gvaradar, Christian Benvenuti (benve)
In-Reply-To: <1411057934.7106.275.camel@edumazet-glaptop2.roam.corp.google.com>
On Thu, 2014-09-18 at 09:32 -0700, Eric Dumazet wrote:
> On Thu, 2014-09-18 at 09:26 -0700, Stephen Hemminger wrote:
> > On Thu, 18 Sep 2014 08:02:05 -0700
> > Eric Dumazet <eric.dumazet@gmail.com> wrote:
> >
> > > From: Eric Dumazet <edumazet@google.com>
> > >
> > > We cannot make struct qdisc_skb_cb bigger without impacting IPoIB,
> > > or increasing skb->cb[] size.
> > >
> > > Commit e0f31d849867 ("flow_keys: Record IP layer protocol in
> > > skb_flow_dissect()") broke IPoIB.
> > >
> > > Only current offender is sch_choke, and this one do not need an
> > > absolutely precise flow key.
> > >
> > > If we store 17 bytes of flow key, its more than enough. (Its the actual
> > > size of flow_keys if it was a packed structure, but we might add new
> > > fields at the end of it later)
> > >
> > > Signed-off-by: Eric Dumazet <edumazet@google.com>
> >
> > Can we add BUILD_BUG to stop next time something smacks this.
>
> I though we had.
>
> Maybe IPoIB lacks one.
>
> Or, do you have an idea ?
Seems straightforward ...
Or can you carry this fix for me ?
Thanks
[PATCH] ipoib: validate struct ipoib_cb size
To catch future errors sooner.
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
diff --git a/drivers/infiniband/ulp/ipoib/ipoib.h b/drivers/infiniband/ulp/ipoib/ipoib.h
index 3edce617c31b..d7562beb5423 100644
--- a/drivers/infiniband/ulp/ipoib/ipoib.h
+++ b/drivers/infiniband/ulp/ipoib/ipoib.h
@@ -131,6 +131,12 @@ struct ipoib_cb {
u8 hwaddr[INFINIBAND_ALEN];
};
+static inline struct ipoib_cb *ipoib_skb_cb(const struct sk_buff *skb)
+{
+ BUILD_BUG_ON(sizeof(skb->cb) < sizeof(struct ipoib_cb));
+ return (struct ipoib_cb *)skb->cb;
+}
+
/* Used for all multicast joins (broadcast, IPv4 mcast and IPv6 mcast) */
struct ipoib_mcast {
struct ib_sa_mcmember_rec mcmember;
diff --git a/drivers/infiniband/ulp/ipoib/ipoib_main.c b/drivers/infiniband/ulp/ipoib/ipoib_main.c
index 1310acf6bf92..13e6e0431592 100644
--- a/drivers/infiniband/ulp/ipoib/ipoib_main.c
+++ b/drivers/infiniband/ulp/ipoib/ipoib_main.c
@@ -716,7 +716,7 @@ static int ipoib_start_xmit(struct sk_buff *skb, struct net_device *dev)
{
struct ipoib_dev_priv *priv = netdev_priv(dev);
struct ipoib_neigh *neigh;
- struct ipoib_cb *cb = (struct ipoib_cb *) skb->cb;
+ struct ipoib_cb *cb = ipoib_skb_cb(skb);
struct ipoib_header *header;
unsigned long flags;
@@ -813,7 +813,7 @@ static int ipoib_hard_header(struct sk_buff *skb,
const void *daddr, const void *saddr, unsigned len)
{
struct ipoib_header *header;
- struct ipoib_cb *cb = (struct ipoib_cb *) skb->cb;
+ struct ipoib_cb *cb = ipoib_skb_cb(skb);
header = (struct ipoib_header *) skb_push(skb, sizeof *header);
^ permalink raw reply related
* Re: [PATCH net] net: sched: shrink struct qdisc_skb_cb to 28 bytes
From: Joe Perches @ 2014-09-18 18:07 UTC (permalink / raw)
To: Eric Dumazet
Cc: Stephen Hemminger, Or Gerlitz, Govindarajulu Varadarajan,
Yinghai Lu, David Miller, NetDev, ssujith, gvaradar,
Christian Benvenuti (benve)
In-Reply-To: <1411063227.7106.280.camel@edumazet-glaptop2.roam.corp.google.com>
On Thu, 2014-09-18 at 11:00 -0700, Eric Dumazet wrote:
> On Thu, 2014-09-18 at 09:32 -0700, Eric Dumazet wrote:
> > On Thu, 2014-09-18 at 09:26 -0700, Stephen Hemminger wrote:
[]
> > Or, do you have an idea ?
>
> Seems straightforward ...
>
> Or can you carry this fix for me ?
>
> Thanks
>
> [PATCH] ipoib: validate struct ipoib_cb size
>
> To catch future errors sooner.
[]
> diff --git a/drivers/infiniband/ulp/ipoib/ipoib.h b/drivers/infiniband/ulp/ipoib/ipoib.h
[]
> +static inline struct ipoib_cb *ipoib_skb_cb(const struct sk_buff *skb)
> +{
> + BUILD_BUG_ON(sizeof(skb->cb) < sizeof(struct ipoib_cb));
> + return (struct ipoib_cb *)skb->cb;
> +}
It seems better not to use const for the struct sk_buff * here.
Neither of the uses take a const struct sk_buff *
> diff --git a/drivers/infiniband/ulp/ipoib/ipoib_main.c b/drivers/infiniband/ulp/ipoib/ipoib_main.c
> @@ -716,7 +716,7 @@ static int ipoib_start_xmit(struct sk_buff *skb, struct net_device *dev)
> {
> struct ipoib_dev_priv *priv = netdev_priv(dev);
> struct ipoib_neigh *neigh;
> - struct ipoib_cb *cb = (struct ipoib_cb *) skb->cb;
> + struct ipoib_cb *cb = ipoib_skb_cb(skb);
> struct ipoib_header *header;
> unsigned long flags;
>
> @@ -813,7 +813,7 @@ static int ipoib_hard_header(struct sk_buff *skb,
> const void *daddr, const void *saddr, unsigned len)
> {
> struct ipoib_header *header;
> - struct ipoib_cb *cb = (struct ipoib_cb *) skb->cb;
> + struct ipoib_cb *cb = ipoib_skb_cb(skb);
>
> header = (struct ipoib_header *) skb_push(skb, sizeof *header);
^ permalink raw reply
* Re: [PATCH 1/2 v2 nf-next] net: bridge: don't register netfilter call_iptables hooks by default
From: Pablo Neira Ayuso @ 2014-09-18 18:39 UTC (permalink / raw)
To: Florian Westphal; +Cc: netfilter-devel, netdev
In-Reply-To: <20140918143108.GB8484@breakpoint.cc>
On Thu, Sep 18, 2014 at 04:31:08PM +0200, Florian Westphal wrote:
> > I also guess most distributors will use CONFIG_BRIDGE_NETFILTER=m.
> > Then, users will get a warning message to let them know that they will
> > have to modprobe br_netfilter in the future if they need it, so we can
> > remove the deferred request_module from the br init path.
>
> Hmm, not sure if its safe to do this, e.g. with bridge=y,
> br_netfilter=m, module might not yet be present in such cases?
>
> Also, what about this:
> iptables-restore < rules.txt
> modprobe bridge
> brctl addbr ...
> brctl addif ...
>
> at this point, any packet forwarded by bridge is filtered
> via iptables.
>
> After your patch, this might no longer be the case, if the modprobe
> call is delayed (maybe this is far-fetched and not an issue in practice)?
Agreed, I think we can use your static key approach to drop traffic
from br_handle_frame after the work has called request_module.
> 2nd hypothetical issue:
> modprobe bridge
> sysctl net.bridge.bridge-nf-call-iptables=0
>
> The sysctl could fail when br_nf_core is not yet present.
Indeed.
We can keep those sysctls there in bridge.ko and deprecated them in
favour of br_netlink. I think we can add some IFLA_BRPORT_NF for the
setlink command to replace the existing global proc nf-call-thing.
We'll have to enhance the 'bridge' tool in iproute2 to support this.
I can see this is packaged in testing already by debian at least.
> Then, in two years or so, we would remove the autoload hook
> in the packet procesing path.
Agreed. By that time, we can kill the bridge.ko <-> br_netfilter.ko
dependency and the /proc call-nf-bridge stuff in favour of br_netlink.
Let me know if you still have any concern. Thanks.
^ permalink raw reply
* Re: [PATCHv6 net-next 1/3] sunvnet: upgrade to VIO protocol version 1.6
From: Raghuram Kothakota @ 2014-09-18 18:49 UTC (permalink / raw)
To: David L Stevens; +Cc: David Miller, netdev
In-Reply-To: <541AD838.50700@oracle.com>
On Sep 18, 2014, at 6:03 AM, David L Stevens <david.stevens@oracle.com> wrote:
>
>
> On 09/18/2014 12:09 AM, Raghuram Kothakota wrote:
>
>>> @@ -1048,8 +1116,8 @@ static int vnet_port_alloc_tx_bufs(struct vnet_port *port)
>>> void *dring;
>>>
>>> for (i = 0; i < VNET_TX_RING_SIZE; i++) {
>>> - void *buf = kzalloc(ETH_FRAME_LEN + 8, GFP_KERNEL);
>>> - int map_len = (ETH_FRAME_LEN + 7) & ~7;
>>> + void *buf = kzalloc(VNET_MAXPACKET + 8, GFP_KERNEL);
>>
>>
>> This patch doesn't change the VNET_MAXPACKET to 64k, but the patch 2/3 changes
>> it to 64k+. Allocating buffers of size VNET_MAXPACKET always can consume too much
>> memory for every port/LDC, that would be more than 32MB. You may want to allocate
>> buffers based on the mtu that is negotiated, so that this memory used only when
>> such large packets are accepted by the peer.
>
> I originally had code to dynamically allocate them after the MTU negotiation, but
> that opens up a can of worms regarding stopping and freeing an active ring. I don't
> believe the shutdown code addresses this adequately, either, and I think this is
> worth addressing, but separately.
>
I am probably not as knowledgeable of sunvnet as you may be, but I would assume
the code is capable of handling the a vport removal and should have sufficient method
cleanup as well.
> I convinced myself to do it this way because:
> a) memory is cheap
In the virtualization world, we want resources to be efficiently used and memory is
still very important resource. My concern is mostly because this memory usage of
32+MB is on a per LDC basis. LDoms today supports a max of 128 domains, but
from my experience seen actual deployments of the order of 50 domains. This is
going up as the platforms getting more and more powerful. If there are really
that many peers, then the amount of memory consumed by one vnet instance
is 50 * 32+MB = 1.6GB+. It's fine if this memory is really used, but it seems like this
will be useful only when the peer is another linux guest with this version of vnet and
also the MTU is configured to use 64K. The memory is being wasted for all other
peers that either don't support 64K MTU or not configured to use it and also
the switch port as obviously it doesn't support 64K MTU today.
Note, it's just buffer space that is being consumed here, it is also the LDC shared
memory space. Luckily the SHADOW_MAPPED shared memory space has
very less limitations otherwise it can cause other impact to other virtual devices.
> b) I think most people will want to use large MTUs for performance; enough so
> that perhaps the bring-up MTU should be 64K too
>From my experience in SPARC world, most customers pushed us back for any
proposal to use Jumbo Frames. The customers who configured Jumbo frames,
mostly used 9K for performance of NFS etc.
> c) future (actually current) TSO/GSO work will want large buffers even if the MTU
> is not changed
>
When we implemented TSO support, we evaluated the cost of the buffers vs
performance. We were able to limit TSO support to 8K(actually bit less) and still
achieve high performance, for example we are able to drive line rate on a 10G
and guest-to-guest of the order of 45+Gbps. So, my suggestion would be to
increase the parallelism of the code more than depending on large MTU.
> So, if this is actually too much memory, I was more inclined to reduce the ring
> size rather than either add complicating code to handle active-ring reallocation
> that would typically be run once per boot, or another alternative of adding
> module parameters to specify the buffer size TSO/GSO will need 64K to perform
> well, regardless of the device MTU.
>
Note, my experience shows that reducing ring size can have larger impact on the
standard MTU case. I would assume the sunvnet code to be dynamic in any case
to deal with ports being added and removed and ports going down and up and
this is just one aspect of those operations.
-Raghuram
> +-DLS
^ 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