* Re: [PATCH net-next] sfc: add support for skb->xmit_more
From: Ben Hutchings @ 2014-10-22 8:58 UTC (permalink / raw)
To: Edward Cree
Cc: Robert Stonehouse, Daniel Borkmann, davem, nikolay, netdev,
Shradha Shah, Jon Cooper, linux-net-drivers
In-Reply-To: <alpine.LFD.2.03.1410171512070.3218@solarflare.com>
[-- Attachment #1: Type: text/plain, Size: 684 bytes --]
On Fri, 2014-10-17 at 15:32 +0100, Edward Cree wrote:
[...]
> @@ -351,7 +343,7 @@ netdev_tx_t efx_enqueue_skb(struct efx_tx_queue *tx_queue, struct sk_buff *skb)
> unsigned short dma_flags;
> int i = 0;
>
> - EFX_BUG_ON_PARANOID(tx_queue->write_count != tx_queue->insert_count);
> + EFX_BUG_ON_PARANOID(tx_queue->write_count > tx_queue->insert_count);
[...]
Doesn't this break after 2^32 descriptors? It seems like you would need
a similar comparison to time_after(); possibly:
EFX_BUG_ON_PARANOID((int)(tx_queue->write_count - tx_queue->insert_count) > 0);
Ben.
--
Ben Hutchings
For every action, there is an equal and opposite criticism. - Harrison
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 811 bytes --]
^ permalink raw reply
* Problem: A divide error 0000 occured when rcv_mss is 0
From: Wang Weidong @ 2014-10-22 9:07 UTC (permalink / raw)
To: David Miller
Cc: Eric Dumazet, rui.xiang, Li Zefan, netdev,
linux-kernel@vger.kernel.org
Hi everyone,
my kernel is based on linux-stable-3.4.87. And when I do some testing,
I got this Bug:
<4>[18042.394823] divide error: 0000 [#1]
<4>[18042.395178] SMP
<4>[18042.734309] CPU 2
...
<4>[18042.734309] RIP: 0010:[<ffffffff81385e44>] [<ffffffff81385e44>] tcp_send_dupack+0x54/0xe0
<4>[18042.734309] RSP: 0018:ffff8801c1845af0 EFLAGS: 00010246
<4>[18042.734309] RAX: 00000000000f4240 RBX: ffff8801604fad00 RCX: 0000000000000000
<4>[18042.734309] RDX: 0000000000000000 RSI: ffff880121cd5400 RDI: ffff8801604fad00
<4>[18042.734309] RBP: ffff8801c1845b00 R08: 00000000355e0308 R09: 0000000008015603
<4>[18042.734309] R10: 000000000000000f R11: ffff88016095e1cc R12: ffff880121cd5400
<4>[18042.734309] R13: 0000000000000000 R14: ffff88014f5d1c62 R15: ffff88014f5d1c62
<4>[18042.734309] FS: 00007f611dc6d700(0000) GS:ffff8801c1840000(0000) knlGS:0000000000000000
<4>[18042.734309] CS: 0010 DS: 0000 ES: 0000 CR0: 000000008005003b
<4>[18042.734309] CR2: ffffffffff600400 CR3: 000000014c83e000 CR4: 00000000001407e0
<4>[18042.734309] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
<4>[18042.734309] DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400
<4>[18042.734309] Process CSD_1 (pid: 28654, threadinfo ffff88019fb92000, task ffff880162555c80)
<4>[18042.734309] Stack:
<4>[18042.734309] 0000000000002135 ffff8801604fad00 ffff8801c1845b40 ffffffff813886b5
<4>[18042.734309] ffff8801c1845b20 ffffffff81441aa2 ffff8801604fad00 ffff880121cd5400
<4>[18042.734309] ffff88014f5d1c62 ffff88016095e180 ffff8801c1845b90 ffffffff8138d0b7
<4>[18042.734309] ffff88014153e0d0 0000000000000001 0000000000000002 ffff8801604fad00
<4>[18042.734309] ffff88016095e180 0000000000000003 ffff88016095e180 ffff88014f5d1c62
<4>[18042.734309] ffff8801c1845bc0 ffffffff81397255 ffff880121cd5400 ffff88016095e180
<4>[18042.734309] ffff8801604fad00 ffff88014f5d1c62 ffff8801c1845c10 ffffffff81395a70
<4>[18042.734309] ffff88010000000f ffffffff811bf0e1 ffff8801c1845c10 ffffffff81356d2f
<4>[18042.734309] ffff880121cd5400 ffffffff81856680 ffff88016095e180 ffff88016095e1d0
<4>[18042.734309] ffff8801c1845c80 ffffffff81396fc8 ffff88010000000f ffff880163d5e000
<4>[18042.734309] ffff8801c1845cb0 ffffffff0000000f ffffffff81856680 0a72a8c000000801
<4>[18042.734309] ffffffff81884600 ffff880121cd5400 ffffffff8164cc80 0000000000000006
<4>[18042.734309] ffffffff81856680 ffffffff81883920 ffff8801c1845cb0 ffffffff813746e9
<4>[18042.734309] ffff880121cd5400 ffff88014f5d1c4e 0000000000000008 ffff880163d5e000
<4>[18042.734309] ffff8801c1845ce0 ffffffff81374a20 ffff880180000000 0000000000000008
<4>[18042.734309] ffff880163d5e000 ffff880121cd5400
<4>[18042.734309] Call Trace:
<4>[18042.734309] <IRQ>
<4>[18042.734309] [<ffffffff813886b5>] tcp_validate_incoming+0x135/0x2c0
<4>[18042.734309] [<ffffffff81441aa2>] ? _raw_spin_unlock_bh+0x12/0x20
<4>[18042.734309] [<ffffffff8138d0b7>] tcp_rcv_state_process+0x47/0xba0
<4>[18042.734309] [<ffffffff81397255>] tcp_child_process+0x45/0xf0
<4>[18042.734309] [<ffffffff81395a70>] tcp_v4_do_rcv+0x1b0/0x290
<4>[18042.734309] [<ffffffff811bf0e1>] ? security_sock_rcv_skb+0x11/0x20
<4>[18042.734309] [<ffffffff81356d2f>] ? sk_filter+0x1f/0xb0
<4>[18042.734309] [<ffffffff81396fc8>] tcp_v4_rcv+0x738/0x810
<4>[18042.734309] [<ffffffff813746e9>] ip_local_deliver_finish+0xb9/0x230
<4>[18042.734309] [<ffffffff81374a20>] ip_local_deliver+0x80/0x90
<4>[18042.734309] [<ffffffff81374389>] ip_rcv_finish+0x69/0x310
<4>[18042.734309] [<ffffffff81374c78>] ip_rcv+0x248/0x320
<4>[18042.734309] [<ffffffff81344dd2>] __netif_receive_skb+0x372/0x580
<4>[18042.734309] [<ffffffff8106f0d8>] ? check_preempt_wakeup+0x158/0x250
<4>[18042.734309] [<ffffffff81345198>] netif_receive_skb+0x28/0x90
<4>[18042.734309] [<ffffffff81064c4e>] ? __wake_up+0x4e/0x70
<4>[18042.734309] [<ffffffff813453dc>] napi_gro_complete+0xcc/0x100
<4>[18042.734309] [<ffffffff8134584f>] napi_complete+0x2f/0x80
<4>[18042.734309] [<ffffffffa1575baf>] napi_rx_handler+0x2f/0x80 [cxgb4]
<4>[18042.734309] [<ffffffff81345957>] net_rx_action+0xb7/0x1a0
<4>[18042.734309] [<ffffffff812259b1>] ? do_raw_spin_lock+0x61/0x110
<4>[18042.734309] [<ffffffff8103e385>] __do_softirq+0xb5/0x1c0
<4>[18042.734309] [<ffffffff81441b59>] ? _raw_spin_lock+0x9/0x10
<4>[18042.734309] [<ffffffff8144a99c>] call_softirq+0x1c/0x30
<4>[18042.734309] [<ffffffff810040fd>] do_softirq+0x6d/0xa0
<4>[18042.734309] [<ffffffff8103e725>] irq_exit+0xa5/0xb0
<4>[18042.734309] [<ffffffff81003cee>] do_IRQ+0x5e/0xd0
<4>[18042.734309] [<ffffffff81441d6a>] common_interrupt+0x6a/0x6a
Is it like the problem which fixed by commit 709e8697af1c86772c1a6fccda6d4b0e2e226547
(tcp: clear xmit timers in tcp_v4_syn_recv_sock())?
I think the divide error happen at:
tcp_send_dupack
-> tcp_enter_quickack_mode
-> tcp_incr_quickack
There is code that:
unsigned quickacks = tcp_sk(sk)->rcv_wnd / (2 * icsk->icsk_ack.rcv_mss);
so the icsk_ack.rcv_mss is 0?
But I am not sure, is it a Bug in the kernel? Or maybe somewhere I do wrong.
Can anybody help me to check is it a Bug or other problems, and how can I to
resolve it.
Regards,
Wang
^ permalink raw reply
* [PATCH 1/2 v2] xfrm: fix a potential use after free in xfrm4_policy.c
From: roy.qing.li @ 2014-10-22 9:09 UTC (permalink / raw)
To: netdev
From: Li RongQing <roy.qing.li@gmail.com>
pskb_may_pull() maybe change skb->data and make xprth pointer oboslete,
so recompute the xprth
Signed-off-by: Li RongQing <roy.qing.li@gmail.com>
---
Diff with v1: NEXT_HEAD add a length parameter, which hide on v1
net/ipv4/xfrm4_policy.c | 21 +++++++++++++--------
1 file changed, 13 insertions(+), 8 deletions(-)
diff --git a/net/ipv4/xfrm4_policy.c b/net/ipv4/xfrm4_policy.c
index 6156f68..d7b33c5 100644
--- a/net/ipv4/xfrm4_policy.c
+++ b/net/ipv4/xfrm4_policy.c
@@ -98,11 +98,14 @@ static int xfrm4_fill_dst(struct xfrm_dst *xdst, struct net_device *dev,
return 0;
}
+#define NEXT_HEAD(skb, length) (skb_network_header(skb) + length)
+
static void
_decode_session4(struct sk_buff *skb, struct flowi *fl, int reverse)
{
const struct iphdr *iph = ip_hdr(skb);
- u8 *xprth = skb_network_header(skb) + iph->ihl * 4;
+ int ihl = iph->ihl * 4;
+ u8 *xprth = NEXT_HEAD(skb, ihl);
struct flowi4 *fl4 = &fl->u.ip4;
int oif = 0;
@@ -122,7 +125,7 @@ _decode_session4(struct sk_buff *skb, struct flowi *fl, int reverse)
case IPPROTO_DCCP:
if (xprth + 4 < skb->data ||
pskb_may_pull(skb, xprth + 4 - skb->data)) {
- __be16 *ports = (__be16 *)xprth;
+ __be16 *ports = (__be16 *)NEXT_HEAD(skb, ihl);
fl4->fl4_sport = ports[!!reverse];
fl4->fl4_dport = ports[!reverse];
@@ -131,7 +134,7 @@ _decode_session4(struct sk_buff *skb, struct flowi *fl, int reverse)
case IPPROTO_ICMP:
if (pskb_may_pull(skb, xprth + 2 - skb->data)) {
- u8 *icmp = xprth;
+ u8 *icmp = NEXT_HEAD(skb, ihl);
fl4->fl4_icmp_type = icmp[0];
fl4->fl4_icmp_code = icmp[1];
@@ -140,7 +143,7 @@ _decode_session4(struct sk_buff *skb, struct flowi *fl, int reverse)
case IPPROTO_ESP:
if (pskb_may_pull(skb, xprth + 4 - skb->data)) {
- __be32 *ehdr = (__be32 *)xprth;
+ __be32 *ehdr = (__be32 *)NEXT_HEAD(skb, ihl);
fl4->fl4_ipsec_spi = ehdr[0];
}
@@ -148,7 +151,7 @@ _decode_session4(struct sk_buff *skb, struct flowi *fl, int reverse)
case IPPROTO_AH:
if (pskb_may_pull(skb, xprth + 8 - skb->data)) {
- __be32 *ah_hdr = (__be32 *)xprth;
+ __be32 *ah_hdr = (__be32 *)NEXT_HEAD(skb, ihl);
fl4->fl4_ipsec_spi = ah_hdr[1];
}
@@ -156,17 +159,19 @@ _decode_session4(struct sk_buff *skb, struct flowi *fl, int reverse)
case IPPROTO_COMP:
if (pskb_may_pull(skb, xprth + 4 - skb->data)) {
- __be16 *ipcomp_hdr = (__be16 *)xprth;
+ __be16 *ipcomp_hdr;
+ ipcomp_hdr = (__be16 *)NEXT_HEAD(skb, ihl);
fl4->fl4_ipsec_spi = htonl(ntohs(ipcomp_hdr[1]));
}
break;
case IPPROTO_GRE:
if (pskb_may_pull(skb, xprth + 12 - skb->data)) {
- __be16 *greflags = (__be16 *)xprth;
- __be32 *gre_hdr = (__be32 *)xprth;
+ __be16 *greflags;
+ __be32 *gre_hdr = (__be32 *)NEXT_HEAD(skb, ihl);
+ greflags = (__be16 *)NEXT_HEAD(skb, ihl);
if (greflags[0] & GRE_KEY) {
if (greflags[0] & GRE_CSUM)
gre_hdr++;
--
1.7.10.4
^ permalink raw reply related
* [PATCH 2/2 v2] xfrm6: fix a potential use after free in xfrm6_policy.c
From: roy.qing.li @ 2014-10-22 9:09 UTC (permalink / raw)
To: netdev
In-Reply-To: <1413968993-13528-1-git-send-email-roy.qing.li@gmail.com>
From: Li RongQing <roy.qing.li@gmail.com>
pskb_may_pull() maybe change skb->data and make nh and exthdr pointer
oboslete, so recompute the nd and exthdr
Signed-off-by: Li RongQing <roy.qing.li@gmail.com>
---
Diff v1: add space between the date type and *, like (__be16 *)
net/ipv6/xfrm6_policy.c | 11 ++++++++---
1 file changed, 8 insertions(+), 3 deletions(-)
diff --git a/net/ipv6/xfrm6_policy.c b/net/ipv6/xfrm6_policy.c
index ac49f84..5f98364 100644
--- a/net/ipv6/xfrm6_policy.c
+++ b/net/ipv6/xfrm6_policy.c
@@ -170,8 +170,10 @@ _decode_session6(struct sk_buff *skb, struct flowi *fl, int reverse)
case IPPROTO_DCCP:
if (!onlyproto && (nh + offset + 4 < skb->data ||
pskb_may_pull(skb, nh + offset + 4 - skb->data))) {
- __be16 *ports = (__be16 *)exthdr;
+ __be16 *ports;
+ nh = skb_network_header(skb);
+ ports = (__be16 *)(nh + offset);
fl6->fl6_sport = ports[!!reverse];
fl6->fl6_dport = ports[!reverse];
}
@@ -180,8 +182,10 @@ _decode_session6(struct sk_buff *skb, struct flowi *fl, int reverse)
case IPPROTO_ICMPV6:
if (!onlyproto && pskb_may_pull(skb, nh + offset + 2 - skb->data)) {
- u8 *icmp = (u8 *)exthdr;
+ u8 *icmp;
+ nh = skb_network_header(skb);
+ icmp = (u8 *)(nh + offset);
fl6->fl6_icmp_type = icmp[0];
fl6->fl6_icmp_code = icmp[1];
}
@@ -192,8 +196,9 @@ _decode_session6(struct sk_buff *skb, struct flowi *fl, int reverse)
case IPPROTO_MH:
if (!onlyproto && pskb_may_pull(skb, nh + offset + 3 - skb->data)) {
struct ip6_mh *mh;
- mh = (struct ip6_mh *)exthdr;
+ nh = skb_network_header(skb);
+ mh = (struct ip6_mh *)(nh + offset);
fl6->fl6_mh_type = mh->ip6mh_type;
}
fl6->flowi6_proto = nexthdr;
--
1.7.10.4
^ permalink raw reply related
* Re: IPv6 UFO for VMs
From: Hannes Frederic Sowa @ 2014-10-22 9:35 UTC (permalink / raw)
To: Ben Hutchings; +Cc: netdev, virtualization
In-Reply-To: <1413935045.5994.2.camel@decadent.org.uk>
On Mi, 2014-10-22 at 00:44 +0100, Ben Hutchings wrote:
> There are several ways that VMs can take advantage of UFO and get the
> host to do fragmentation for them:
>
> drivers/net/macvtap.c: gso_type = SKB_GSO_UDP;
> drivers/net/tun.c: skb_shinfo(skb)->gso_type = SKB_GSO_UDP;
> drivers/net/virtio_net.c: skb_shinfo(skb)->gso_type = SKB_GSO_UDP;
>
> Our implementation of UFO for IPv6 does:
>
> fptr = (struct frag_hdr *)(skb_network_header(skb) + unfrag_ip6hlen);
> fptr->nexthdr = nexthdr;
> fptr->reserved = 0;
> fptr->identification = skb_shinfo(skb)->ip6_frag_id;
>
> which assumes ip6_frag_id has been set. That's only true if the local
> stack constructed the skb; otherwise it appears we get zero.
>
> This seems to be a regression as a result of:
>
> commit 916e4cf46d0204806c062c8c6c4d1f633852c5b6
> Author: Hannes Frederic Sowa <hannes@stressinduktion.org>
> Date: Fri Feb 21 02:55:35 2014 +0100
>
> ipv6: reuse ip6_frag_id from ip6_ufo_append_data
>
> However, that change seems reasonable - we *shouldn't* be choosing IDs
> for any other stack. Any paravirt net driver that can use IPv6 UFO
> needs to have some way of passing a fragmentation ID to put in
> skb_shared_info::ip6_frag_id.
Do we really gain a lot of performance by enabling UFO on those devices
or would it make sense to just drop support? It only helps fragmenting
large UDP packets, so I don't think it is worth it.
Otherwise I agree with Ben, we need to pass a fragmentation id from the
host over to the system segmenting the gso frame. Fragmentation ids must
be generated by the end system.
Hmm...
Bye,
Hannes
^ permalink raw reply
* Re: [RFC PATCH v2 net-next] net: ipv6: Add a sysctl to make optimistic addresses useful candidates
From: Hannes Frederic Sowa @ 2014-10-22 10:12 UTC (permalink / raw)
To: Erik Kline; +Cc: netdev, Ben Hutchings, Lorenzo Colitti
In-Reply-To: <CAAedzxpsaEGt+oO0xJWLxcjapdRYauHP4rAsKkf_skcdzbGEmg@mail.gmail.com>
On Mi, 2014-10-22 at 14:25 +0900, Erik Kline wrote:
> Hello.
>
> On Wed, Oct 22, 2014 at 4:45 AM, Hannes Frederic Sowa
> <hannes@stressinduktion.org> wrote:
> > Hi,
> >
> > On Di, 2014-10-21 at 13:05 +0900, Erik Kline wrote:
> >> Add a sysctl that causes an interface's optimistic addresses
> >> to be considered equivalent to other non-deprecated addresses
> >> for source address selection purposes. Preferred addresses
> >> will still take precedence over optimistic addresses, subject
> >> to other ranking in the source address selection algorithm.
> >>
> >> This is useful where different interfaces are connected to
> >> different networks from different ISPs (e.g., a cell network
> >> and a home wifi network).
> >>
> >> The current behaviour complies with RFC 3484/6724, and it
> >> makes sense if the host has only one interface, or has
> >> multiple interfaces on the same network (same or cooperating
> >> administrative domain(s), but not in the multiple distinct
> >> networks case.
> >>
> >> For example, if a mobile device has an IPv6 address on an LTE
> >> network and then connects to IPv6-enabled wifi, while the wifi
> >> IPv6 address is undergoing DAD, IPv6 connections will try use
> >> the wifi default route with the LTE IPv6 address, and will get
> >> stuck until they time out.
> >>
> >> Also, because optimistic addresses can actually be used, issue
> >> an RTM_NEWADDR as soon as DAD starts. If DAD fails an separate
> >> RTM_DELADDR will be sent.
> >>
> >> Also: add an entry in ip-sysctl.txt for optimistic_dad.
> >>
> >> Signed-off-by: Erik Kline <ek@google.com>
> >> ---
> >> Documentation/networking/ip-sysctl.txt | 13 ++++++++++++
> >> include/linux/ipv6.h | 1 +
> >> include/uapi/linux/ipv6.h | 1 +
> >> net/ipv6/addrconf.c | 36 ++++++++++++++++++++++++++++++++--
> >> 4 files changed, 49 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/Documentation/networking/ip-sysctl.txt b/Documentation/networking/ip-sysctl.txt
> >> index 0307e28..e03cf49 100644
> >> --- a/Documentation/networking/ip-sysctl.txt
> >> +++ b/Documentation/networking/ip-sysctl.txt
> >> @@ -1452,6 +1452,19 @@ suppress_frag_ndisc - INTEGER
> >> 1 - (default) discard fragmented neighbor discovery packets
> >> 0 - allow fragmented neighbor discovery packets
> >>
> >> +optimistic_dad - BOOLEAN
> >> + Whether to perform Optimistic Duplicate Address Detection (RFC 4429).
> >> + 0: disabled (default)
> >> + 1: enabled
> >> +
> >> +use_optimistic - BOOLEAN
> >> + If enabled, do not classify optimistic addresses as deprecated during
> >> + source address selection. Preferred addresses will still be chosen
> >> + before optimistic addresses, subject to other ranking in the source
> >> + address selection algorithm.
> >> + 0: disabled (default)
> >> + 1: enabled
> >> +
> >> icmp/*:
> >> ratelimit - INTEGER
> >> Limit the maximal rates for sending ICMPv6 packets.
> >> diff --git a/include/linux/ipv6.h b/include/linux/ipv6.h
> >> index ff56053..7121a2e 100644
> >> --- a/include/linux/ipv6.h
> >> +++ b/include/linux/ipv6.h
> >> @@ -42,6 +42,7 @@ struct ipv6_devconf {
> >> __s32 accept_ra_from_local;
> >> #ifdef CONFIG_IPV6_OPTIMISTIC_DAD
> >> __s32 optimistic_dad;
> >> + __s32 use_optimistic;
> >> #endif
> >> #ifdef CONFIG_IPV6_MROUTE
> >> __s32 mc_forwarding;
> >> diff --git a/include/uapi/linux/ipv6.h b/include/uapi/linux/ipv6.h
> >> index efa2666..e863d08 100644
> >> --- a/include/uapi/linux/ipv6.h
> >> +++ b/include/uapi/linux/ipv6.h
> >> @@ -164,6 +164,7 @@ enum {
> >> DEVCONF_MLDV2_UNSOLICITED_REPORT_INTERVAL,
> >> DEVCONF_SUPPRESS_FRAG_NDISC,
> >> DEVCONF_ACCEPT_RA_FROM_LOCAL,
> >> + DEVCONF_USE_OPTIMISTIC,
> >> DEVCONF_MAX
> >> };
> >>
> >> diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
> >> index 725c763..c2fddb7 100644
> >> --- a/net/ipv6/addrconf.c
> >> +++ b/net/ipv6/addrconf.c
> >> @@ -1169,6 +1169,9 @@ enum {
> >> IPV6_SADDR_RULE_LABEL,
> >> IPV6_SADDR_RULE_PRIVACY,
> >> IPV6_SADDR_RULE_ORCHID,
> >> +#ifdef CONFIG_IPV6_OPTIMISTIC_DAD
> >> + IPV6_SADDR_RULE_NOT_OPTIMISTIC,
> >> +#endif
> >> IPV6_SADDR_RULE_PREFIX,
> >> IPV6_SADDR_RULE_MAX
> >> };
> >> @@ -1257,10 +1260,17 @@ static int ipv6_get_saddr_eval(struct net *net,
> >> score->scopedist = ret;
> >> break;
> >> case IPV6_SADDR_RULE_PREFERRED:
> >> + {
> >> /* Rule 3: Avoid deprecated and optimistic addresses */
> >> + u8 avoid = IFA_F_DEPRECATED;
> >> +#ifdef CONFIG_IPV6_OPTIMISTIC_DAD
> >> + if (!score->ifa->idev->cnf.use_optimistic)
> >> + avoid |= IFA_F_OPTIMISTIC;
> >> +#endif
> >> ret = ipv6_saddr_preferred(score->addr_type) ||
> >> - !(score->ifa->flags & (IFA_F_DEPRECATED|IFA_F_OPTIMISTIC));
> >> + !(score->ifa->flags & avoid);
> >> break;
> >> + }
> >> #ifdef CONFIG_IPV6_MIP6
> >> case IPV6_SADDR_RULE_HOA:
> >> {
> >> @@ -1299,6 +1309,14 @@ static int ipv6_get_saddr_eval(struct net *net,
> >> ret = !(ipv6_addr_orchid(&score->ifa->addr) ^
> >> ipv6_addr_orchid(dst->addr));
> >> break;
> >> +#ifdef CONFIG_IPV6_OPTIMISTIC_DAD
> >> + case IPV6_SADDR_RULE_NOT_OPTIMISTIC:
> >> + /* Optimistic addresses still have lower precedence than other
> >> + * preferred addresses.
> >> + */
> >> + ret = !(score->ifa->flags & IFA_F_OPTIMISTIC);
> >> + break;
> >> +#endif
> >
> > I wonder a bit why this rule is not directly ordered after
> > IPV6_SADDR_RULE_PREFERRED? This would e.g. matter for privacy addresses.
>
> Privacy addresses ("tempaddrs") that win in earlier checks are
> preferred before optimistic in this code (i.e. a tempaddr on the same
> outgoing interface is preferred before an optimistic address).
>
> Similarly, a non-tentative non-privacy address (same outgoing
> interface, same label, ...) will match before an optimistic address,
> but only until DAD completes and the address is no longer optimistic.
> I think this is in keeping with the spirit of the RFC 3484/6724 rules.
Oh yes, I see. I had the evaluation order messed up. ;)
So the question I should be asking would be. AFAIR optimistic addresses
should be handled like deprecated ones, so I am a bit concerned adding a
non-conditional rule before the RULE_PREFIX check.
Shouldn't we only break the tie *after* longest prefix match then? If
you do that before longest prefix match I would prefer ret being masked
by use_optimisitic flag.
> After there's an RFC 7217 implementation, EUI-64-based SLAAC could be
> disabled by folks.
Ack.
>
> >> case IPV6_SADDR_RULE_PREFIX:
> >> /* Rule 8: Use longest matching prefix */
> >> ret = ipv6_addr_diff(&score->ifa->addr, dst->addr);
> >> @@ -3222,8 +3240,13 @@ static void addrconf_dad_begin(struct inet6_ifaddr *ifp)
> >> * Optimistic nodes can start receiving
> >> * Frames right away
> >> */
> >> - if (ifp->flags & IFA_F_OPTIMISTIC)
> >> + if (ifp->flags & IFA_F_OPTIMISTIC) {
> >> ip6_ins_rt(ifp->rt);
> >> + /* Because optimistic nodes can receive frames, notify
> >> + * listeners. If DAD fails, RTM_DELADDR is sent.
> >> + */
> >> + ipv6_ifa_notify(RTM_NEWADDR, ifp);
> >> + }
> >
> > I wonder if we can now delete the ipv6_ifa_notify(RTM_NEWADDR, ifp) in
> > addrconf_dad_completed.
>
> I don't know what everyone's general preference would be, but mine
> would be to err on the side of notifying on state changes. It seems
> harmless to me to keep it in, and something in userspace might want to
> know if/when DAD completes.
Userspace expects to communicate with an address which gets announced
via RTM_NEWADDR, so I would make the RTM_NEWADDR notifications
conditional on use_optimistic flag in both, the completed and the
dad_begin function. This sounds like the best option to me, no?
Bye && thanks,
Hannes
^ permalink raw reply
* [PATCHv2 net-next] xen-netfront: always keep the Rx ring full of requests
From: David Vrabel @ 2014-10-22 10:17 UTC (permalink / raw)
To: netdev; +Cc: David Vrabel, xen-devel, Konrad Rzeszutek Wilk, Boris Ostrovsky
A full Rx ring only requires 1 MiB of memory. This is not enough
memory that it is useful to dynamically scale the number of Rx
requests in the ring based on traffic rates, because:
a) Even the full 1 MiB is a tiny fraction of a typically modern Linux
VM (for example, the AWS micro instance still has 1 GiB of memory).
b) Netfront would have used up to 1 MiB already even with moderate
data rates (there was no adjustment of target based on memory
pressure).
c) Small VMs are going to typically have one VCPU and hence only one
queue.
Keeping the ring full of Rx requests handles bursty traffic better
than trying to converge on an optimal number of requests to keep
filled.
On a 4 core host, an iperf -P 64 -t 60 run from dom0 to a 4 VCPU guest
improved from 5.1 Gbit/s to 5.6 Gbit/s. Gains with more bursty
traffic are expected to be higher.
Signed-off-by: David Vrabel <david.vrabel@citrix.com>
---
Changes in v2:
- Keep rxbuf_* sysfs files.
---
drivers/net/xen-netfront.c | 255 +++++++++++---------------------------------
1 file changed, 63 insertions(+), 192 deletions(-)
diff --git a/drivers/net/xen-netfront.c b/drivers/net/xen-netfront.c
index cca8713..83e39a1 100644
--- a/drivers/net/xen-netfront.c
+++ b/drivers/net/xen-netfront.c
@@ -77,7 +77,9 @@ struct netfront_cb {
#define NET_TX_RING_SIZE __CONST_RING_SIZE(xen_netif_tx, PAGE_SIZE)
#define NET_RX_RING_SIZE __CONST_RING_SIZE(xen_netif_rx, PAGE_SIZE)
-#define TX_MAX_TARGET min_t(int, NET_TX_RING_SIZE, 256)
+
+/* Minimum number of Rx slots (includes slot for GSO metadata). */
+#define NET_RX_SLOTS_MIN (XEN_NETIF_NR_SLOTS_MIN + 1)
/* Queue name is interface name with "-qNNN" appended */
#define QUEUE_NAME_SIZE (IFNAMSIZ + 6)
@@ -137,13 +139,6 @@ struct netfront_queue {
struct xen_netif_rx_front_ring rx;
int rx_ring_ref;
- /* Receive-ring batched refills. */
-#define RX_MIN_TARGET 8
-#define RX_DFL_MIN_TARGET 64
-#define RX_MAX_TARGET min_t(int, NET_RX_RING_SIZE, 256)
- unsigned rx_min_target, rx_max_target, rx_target;
- struct sk_buff_head rx_batch;
-
struct timer_list rx_refill_timer;
struct sk_buff *rx_skbs[NET_RX_RING_SIZE];
@@ -251,7 +246,7 @@ static void rx_refill_timeout(unsigned long data)
static int netfront_tx_slot_available(struct netfront_queue *queue)
{
return (queue->tx.req_prod_pvt - queue->tx.rsp_cons) <
- (TX_MAX_TARGET - MAX_SKB_FRAGS - 2);
+ (NET_TX_RING_SIZE - MAX_SKB_FRAGS - 2);
}
static void xennet_maybe_wake_tx(struct netfront_queue *queue)
@@ -265,77 +260,55 @@ static void xennet_maybe_wake_tx(struct netfront_queue *queue)
netif_tx_wake_queue(netdev_get_tx_queue(dev, queue->id));
}
-static void xennet_alloc_rx_buffers(struct netfront_queue *queue)
+
+static struct sk_buff *xennet_alloc_one_rx_buffer(struct netfront_queue *queue)
{
- unsigned short id;
struct sk_buff *skb;
struct page *page;
- int i, batch_target, notify;
- RING_IDX req_prod = queue->rx.req_prod_pvt;
- grant_ref_t ref;
- unsigned long pfn;
- void *vaddr;
- struct xen_netif_rx_request *req;
- if (unlikely(!netif_carrier_ok(queue->info->netdev)))
- return;
+ skb = __netdev_alloc_skb(queue->info->netdev,
+ RX_COPY_THRESHOLD + NET_IP_ALIGN,
+ GFP_ATOMIC | __GFP_NOWARN);
+ if (unlikely(!skb))
+ return NULL;
- /*
- * Allocate skbuffs greedily, even though we batch updates to the
- * receive ring. This creates a less bursty demand on the memory
- * allocator, so should reduce the chance of failed allocation requests
- * both for ourself and for other kernel subsystems.
- */
- batch_target = queue->rx_target - (req_prod - queue->rx.rsp_cons);
- for (i = skb_queue_len(&queue->rx_batch); i < batch_target; i++) {
- skb = __netdev_alloc_skb(queue->info->netdev,
- RX_COPY_THRESHOLD + NET_IP_ALIGN,
- GFP_ATOMIC | __GFP_NOWARN);
- if (unlikely(!skb))
- goto no_skb;
-
- /* Align ip header to a 16 bytes boundary */
- skb_reserve(skb, NET_IP_ALIGN);
-
- page = alloc_page(GFP_ATOMIC | __GFP_NOWARN);
- if (!page) {
- kfree_skb(skb);
-no_skb:
- /* Could not allocate any skbuffs. Try again later. */
- mod_timer(&queue->rx_refill_timer,
- jiffies + (HZ/10));
-
- /* Any skbuffs queued for refill? Force them out. */
- if (i != 0)
- goto refill;
- break;
- }
-
- skb_add_rx_frag(skb, 0, page, 0, 0, PAGE_SIZE);
- __skb_queue_tail(&queue->rx_batch, skb);
+ page = alloc_page(GFP_ATOMIC | __GFP_NOWARN);
+ if (!page) {
+ kfree_skb(skb);
+ return NULL;
}
+ skb_add_rx_frag(skb, 0, page, 0, 0, PAGE_SIZE);
- /* Is the batch large enough to be worthwhile? */
- if (i < (queue->rx_target/2)) {
- if (req_prod > queue->rx.sring->req_prod)
- goto push;
+ /* Align ip header to a 16 bytes boundary */
+ skb_reserve(skb, NET_IP_ALIGN);
+ skb->dev = queue->info->netdev;
+
+ return skb;
+}
+
+
+static void xennet_alloc_rx_buffers(struct netfront_queue *queue)
+{
+ RING_IDX req_prod = queue->rx.req_prod_pvt;
+ int notify;
+
+ if (unlikely(!netif_carrier_ok(queue->info->netdev)))
return;
- }
- /* Adjust our fill target if we risked running out of buffers. */
- if (((req_prod - queue->rx.sring->rsp_prod) < (queue->rx_target / 4)) &&
- ((queue->rx_target *= 2) > queue->rx_max_target))
- queue->rx_target = queue->rx_max_target;
+ for (req_prod = queue->rx.req_prod_pvt;
+ req_prod - queue->rx.rsp_cons < NET_RX_RING_SIZE;
+ req_prod++) {
+ struct sk_buff *skb;
+ unsigned short id;
+ grant_ref_t ref;
+ unsigned long pfn;
+ struct xen_netif_rx_request *req;
- refill:
- for (i = 0; ; i++) {
- skb = __skb_dequeue(&queue->rx_batch);
- if (skb == NULL)
+ skb = xennet_alloc_one_rx_buffer(queue);
+ if (!skb)
break;
- skb->dev = queue->info->netdev;
-
- id = xennet_rxidx(req_prod + i);
+ id = xennet_rxidx(req_prod);
BUG_ON(queue->rx_skbs[id]);
queue->rx_skbs[id] = skb;
@@ -345,9 +318,8 @@ no_skb:
queue->grant_rx_ref[id] = ref;
pfn = page_to_pfn(skb_frag_page(&skb_shinfo(skb)->frags[0]));
- vaddr = page_address(skb_frag_page(&skb_shinfo(skb)->frags[0]));
- req = RING_GET_REQUEST(&queue->rx, req_prod + i);
+ req = RING_GET_REQUEST(&queue->rx, req_prod);
gnttab_grant_foreign_access_ref(ref,
queue->info->xbdev->otherend_id,
pfn_to_mfn(pfn),
@@ -357,11 +329,16 @@ no_skb:
req->gref = ref;
}
+ queue->rx.req_prod_pvt = req_prod;
+
+ /* Not enough requests? Try again later. */
+ if (req_prod - queue->rx.rsp_cons < NET_RX_SLOTS_MIN) {
+ mod_timer(&queue->rx_refill_timer, jiffies + (HZ/10));
+ return;
+ }
+
wmb(); /* barrier so backend seens requests */
- /* Above is a suitable barrier to ensure backend will see requests. */
- queue->rx.req_prod_pvt = req_prod + i;
- push:
RING_PUSH_REQUESTS_AND_CHECK_NOTIFY(&queue->rx, notify);
if (notify)
notify_remote_via_irq(queue->rx_irq);
@@ -1070,13 +1047,6 @@ err:
work_done -= handle_incoming_queue(queue, &rxq);
- /* If we get a callback with very few responses, reduce fill target. */
- /* NB. Note exponential increase, linear decrease. */
- if (((queue->rx.req_prod_pvt - queue->rx.sring->rsp_prod) >
- ((3*queue->rx_target) / 4)) &&
- (--queue->rx_target < queue->rx_min_target))
- queue->rx_target = queue->rx_min_target;
-
xennet_alloc_rx_buffers(queue);
if (work_done < budget) {
@@ -1643,11 +1613,6 @@ static int xennet_init_queue(struct netfront_queue *queue)
spin_lock_init(&queue->tx_lock);
spin_lock_init(&queue->rx_lock);
- skb_queue_head_init(&queue->rx_batch);
- queue->rx_target = RX_DFL_MIN_TARGET;
- queue->rx_min_target = RX_DFL_MIN_TARGET;
- queue->rx_max_target = RX_MAX_TARGET;
-
init_timer(&queue->rx_refill_timer);
queue->rx_refill_timer.data = (unsigned long)queue;
queue->rx_refill_timer.function = rx_refill_timeout;
@@ -1670,7 +1635,7 @@ static int xennet_init_queue(struct netfront_queue *queue)
}
/* A grant for every tx ring slot */
- if (gnttab_alloc_grant_references(TX_MAX_TARGET,
+ if (gnttab_alloc_grant_references(NET_TX_RING_SIZE,
&queue->gref_tx_head) < 0) {
pr_alert("can't alloc tx grant refs\n");
err = -ENOMEM;
@@ -1678,7 +1643,7 @@ static int xennet_init_queue(struct netfront_queue *queue)
}
/* A grant for every rx ring slot */
- if (gnttab_alloc_grant_references(RX_MAX_TARGET,
+ if (gnttab_alloc_grant_references(NET_RX_RING_SIZE,
&queue->gref_rx_head) < 0) {
pr_alert("can't alloc rx grant refs\n");
err = -ENOMEM;
@@ -2146,129 +2111,35 @@ static const struct ethtool_ops xennet_ethtool_ops =
};
#ifdef CONFIG_SYSFS
-static ssize_t show_rxbuf_min(struct device *dev,
- struct device_attribute *attr, char *buf)
-{
- struct net_device *netdev = to_net_dev(dev);
- struct netfront_info *info = netdev_priv(netdev);
- unsigned int num_queues = netdev->real_num_tx_queues;
-
- if (num_queues)
- return sprintf(buf, "%u\n", info->queues[0].rx_min_target);
- else
- return sprintf(buf, "%u\n", RX_MIN_TARGET);
-}
-
-static ssize_t store_rxbuf_min(struct device *dev,
- struct device_attribute *attr,
- const char *buf, size_t len)
+static ssize_t show_rxbuf(struct device *dev,
+ struct device_attribute *attr, char *buf)
{
- struct net_device *netdev = to_net_dev(dev);
- struct netfront_info *np = netdev_priv(netdev);
- unsigned int num_queues = netdev->real_num_tx_queues;
- char *endp;
- unsigned long target;
- unsigned int i;
- struct netfront_queue *queue;
-
- if (!capable(CAP_NET_ADMIN))
- return -EPERM;
-
- target = simple_strtoul(buf, &endp, 0);
- if (endp == buf)
- return -EBADMSG;
-
- if (target < RX_MIN_TARGET)
- target = RX_MIN_TARGET;
- if (target > RX_MAX_TARGET)
- target = RX_MAX_TARGET;
-
- for (i = 0; i < num_queues; ++i) {
- queue = &np->queues[i];
- spin_lock_bh(&queue->rx_lock);
- if (target > queue->rx_max_target)
- queue->rx_max_target = target;
- queue->rx_min_target = target;
- if (target > queue->rx_target)
- queue->rx_target = target;
-
- xennet_alloc_rx_buffers(queue);
-
- spin_unlock_bh(&queue->rx_lock);
- }
- return len;
-}
-
-static ssize_t show_rxbuf_max(struct device *dev,
- struct device_attribute *attr, char *buf)
-{
- struct net_device *netdev = to_net_dev(dev);
- struct netfront_info *info = netdev_priv(netdev);
- unsigned int num_queues = netdev->real_num_tx_queues;
-
- if (num_queues)
- return sprintf(buf, "%u\n", info->queues[0].rx_max_target);
- else
- return sprintf(buf, "%u\n", RX_MAX_TARGET);
+ return sprintf(buf, "%lu\n", NET_RX_RING_SIZE);
}
-static ssize_t store_rxbuf_max(struct device *dev,
- struct device_attribute *attr,
- const char *buf, size_t len)
+static ssize_t store_rxbuf(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t len)
{
- struct net_device *netdev = to_net_dev(dev);
- struct netfront_info *np = netdev_priv(netdev);
- unsigned int num_queues = netdev->real_num_tx_queues;
char *endp;
unsigned long target;
- unsigned int i = 0;
- struct netfront_queue *queue = NULL;
if (!capable(CAP_NET_ADMIN))
return -EPERM;
-
+
target = simple_strtoul(buf, &endp, 0);
if (endp == buf)
return -EBADMSG;
- if (target < RX_MIN_TARGET)
- target = RX_MIN_TARGET;
- if (target > RX_MAX_TARGET)
- target = RX_MAX_TARGET;
-
- for (i = 0; i < num_queues; ++i) {
- queue = &np->queues[i];
- spin_lock_bh(&queue->rx_lock);
- if (target < queue->rx_min_target)
- queue->rx_min_target = target;
- queue->rx_max_target = target;
- if (target < queue->rx_target)
- queue->rx_target = target;
-
- xennet_alloc_rx_buffers(queue);
+ /* rxbuf_min and rxbuf_max are no longer configurable. */
- spin_unlock_bh(&queue->rx_lock);
- }
return len;
}
-static ssize_t show_rxbuf_cur(struct device *dev,
- struct device_attribute *attr, char *buf)
-{
- struct net_device *netdev = to_net_dev(dev);
- struct netfront_info *info = netdev_priv(netdev);
- unsigned int num_queues = netdev->real_num_tx_queues;
-
- if (num_queues)
- return sprintf(buf, "%u\n", info->queues[0].rx_target);
- else
- return sprintf(buf, "0\n");
-}
-
static struct device_attribute xennet_attrs[] = {
- __ATTR(rxbuf_min, S_IRUGO|S_IWUSR, show_rxbuf_min, store_rxbuf_min),
- __ATTR(rxbuf_max, S_IRUGO|S_IWUSR, show_rxbuf_max, store_rxbuf_max),
- __ATTR(rxbuf_cur, S_IRUGO, show_rxbuf_cur, NULL),
+ __ATTR(rxbuf_min, S_IRUGO|S_IWUSR, show_rxbuf, store_rxbuf),
+ __ATTR(rxbuf_max, S_IRUGO|S_IWUSR, show_rxbuf, store_rxbuf),
+ __ATTR(rxbuf_cur, S_IRUGO, show_rxbuf, NULL),
};
static int xennet_sysfs_addif(struct net_device *netdev)
--
1.7.10.4
^ permalink raw reply related
* [PATCH 4/6] qeth: s390 ethernet device driver dependency
From: Frank Blaschka @ 2014-10-22 10:18 UTC (permalink / raw)
To: davem; +Cc: netdev, linux-s390, Thomas Richter, Frank Blaschka
In-Reply-To: <1413973087-18740-1-git-send-email-blaschka@linux.vnet.ibm.com>
From: Thomas Richter <tmricht@linux.vnet.ibm.com>
Compile the s390 10GB ethernet device driver only when
ETHERNET has been defined in the kernel configuration file.
Right now the qeth device driver is always built regardless
of which network connectivity is active.
Signed-off-by: Thomas Richter <tmricht@linux.vnet.ibm.com>
Signed-off-by: Frank Blaschka <blaschka@linux.vnet.ibm.com>
---
drivers/s390/net/Kconfig | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/s390/net/Kconfig b/drivers/s390/net/Kconfig
index 8b3f559..f1b5111 100644
--- a/drivers/s390/net/Kconfig
+++ b/drivers/s390/net/Kconfig
@@ -71,7 +71,7 @@ config CLAW
config QETH
def_tristate y
prompt "Gigabit Ethernet device support"
- depends on CCW && NETDEVICES && IP_MULTICAST && QDIO
+ depends on CCW && NETDEVICES && IP_MULTICAST && QDIO && ETHERNET
help
This driver supports the IBM System z OSA Express adapters
in QDIO mode (all media types), HiperSockets interfaces and z/VM
--
1.8.5.5
^ permalink raw reply related
* [PATCH 0/6 resend] s390: network patches for net-next
From: Frank Blaschka @ 2014-10-22 10:18 UTC (permalink / raw)
To: davem; +Cc: netdev, linux-s390, Frank Blaschka
Hi Dave,
looks like there was a problem with my previous posting. Hope this time
it will work. Sorry for any inconvenience. The patches are mostly
cleanups and small enhancements for net-next
Thanks,
Frank
Thomas Richter (6):
qeth: qeth_core_main make local functions static
qeth: fix some trace formating issues
qeth: make local functions static in qeth_l3 module
qeth: s390 ethernet device driver dependency
lcs: replace sscanf by kstrto function
ctcm: replace sscanf by kstrto function
drivers/s390/net/Kconfig | 2 +-
drivers/s390/net/ctcm_sysfs.c | 8 ++++----
drivers/s390/net/lcs.c | 11 ++++++-----
drivers/s390/net/qeth_core.h | 16 ----------------
drivers/s390/net/qeth_core_main.c | 24 ++++++++++++++++++------
drivers/s390/net/qeth_l2_main.c | 2 +-
drivers/s390/net/qeth_l3.h | 4 ----
drivers/s390/net/qeth_l3_main.c | 10 +++++-----
8 files changed, 35 insertions(+), 42 deletions(-)
--
1.8.5.5
^ permalink raw reply
* [PATCH 5/6] lcs: replace sscanf by kstrto function
From: Frank Blaschka @ 2014-10-22 10:18 UTC (permalink / raw)
To: davem; +Cc: netdev, linux-s390, Thomas Richter, Frank Blaschka
In-Reply-To: <1413973087-18740-1-git-send-email-blaschka@linux.vnet.ibm.com>
From: Thomas Richter <tmricht@linux.vnet.ibm.com>
Since a single integer value is read from the supplied buffer
use the kstrto functions instead of sscanf.
Signed-off-by: Thomas Richter <tmricht@linux.vnet.ibm.com>
Signed-off-by: Frank Blaschka <blaschka@linux.vnet.ibm.com>
---
drivers/s390/net/lcs.c | 11 ++++++-----
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/drivers/s390/net/lcs.c b/drivers/s390/net/lcs.c
index 0a7d87c..92190aa 100644
--- a/drivers/s390/net/lcs.c
+++ b/drivers/s390/net/lcs.c
@@ -1943,15 +1943,16 @@ static ssize_t
lcs_portno_store (struct device *dev, struct device_attribute *attr, const char *buf, size_t count)
{
struct lcs_card *card;
- int value, rc;
+ int rc;
+ s16 value;
card = dev_get_drvdata(dev);
if (!card)
return 0;
- rc = sscanf(buf, "%d", &value);
- if (rc != 1)
+ rc = kstrtos16(buf, 0, &value);
+ if (rc)
return -EINVAL;
/* TODO: sanity checks */
card->portno = value;
@@ -2007,8 +2008,8 @@ lcs_timeout_store (struct device *dev, struct device_attribute *attr, const char
if (!card)
return 0;
- rc = sscanf(buf, "%u", &value);
- if (rc != 1)
+ rc = kstrtouint(buf, 0, &value);
+ if (rc)
return -EINVAL;
/* TODO: sanity checks */
card->lancmd_timeout = value;
--
1.8.5.5
^ permalink raw reply related
* [PATCH 2/6] qeth: fix some trace formating issues
From: Frank Blaschka @ 2014-10-22 10:18 UTC (permalink / raw)
To: davem; +Cc: netdev, linux-s390, Thomas Richter, Frank Blaschka
In-Reply-To: <1413973087-18740-1-git-send-email-blaschka@linux.vnet.ibm.com>
From: Thomas Richter <tmricht@linux.vnet.ibm.com>
This patch fixes trace formatting issues using the
QETH_CARD_TEXT_ macro. The total size of each trace entry
is 8 bytes. Some of the sprintf formats exceed these 8
bytes (for example using abcd:%d and the converted value
needs more than 3 bytes). The solution is to shorten the
text prepending the value or use a different format (%x).
Signed-off-by: Thomas Richter <tmricht@linux.vnet.ibm.com>
Signed-off-by: Frank Blaschka <blaschka@linux.vnet.ibm.com>
---
drivers/s390/net/qeth_core_main.c | 6 +++---
drivers/s390/net/qeth_l2_main.c | 2 +-
drivers/s390/net/qeth_l3_main.c | 2 +-
3 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/s390/net/qeth_core_main.c b/drivers/s390/net/qeth_core_main.c
index 098bb0f..f407e37 100644
--- a/drivers/s390/net/qeth_core_main.c
+++ b/drivers/s390/net/qeth_core_main.c
@@ -4134,7 +4134,7 @@ static int qeth_setadp_promisc_mode_cb(struct qeth_card *card,
qeth_default_setadapterparms_cb(card, reply, (unsigned long)cmd);
if (cmd->hdr.return_code) {
- QETH_CARD_TEXT_(card, 4, "prmrc%2.2x", cmd->hdr.return_code);
+ QETH_CARD_TEXT_(card, 4, "prmrc%x", cmd->hdr.return_code);
setparms->data.mode = SET_PROMISC_MODE_OFF;
}
card->info.promisc_mode = setparms->data.mode;
@@ -4501,13 +4501,13 @@ static int qeth_snmp_command_cb(struct qeth_card *card,
snmp = &cmd->data.setadapterparms.data.snmp;
if (cmd->hdr.return_code) {
- QETH_CARD_TEXT_(card, 4, "scer1%i", cmd->hdr.return_code);
+ QETH_CARD_TEXT_(card, 4, "scer1%x", cmd->hdr.return_code);
return 0;
}
if (cmd->data.setadapterparms.hdr.return_code) {
cmd->hdr.return_code =
cmd->data.setadapterparms.hdr.return_code;
- QETH_CARD_TEXT_(card, 4, "scer2%i", cmd->hdr.return_code);
+ QETH_CARD_TEXT_(card, 4, "scer2%x", cmd->hdr.return_code);
return 0;
}
data_len = *((__u16 *)QETH_IPA_PDU_LEN_PDU1(data));
diff --git a/drivers/s390/net/qeth_l2_main.c b/drivers/s390/net/qeth_l2_main.c
index c2679bf..d02cd1a 100644
--- a/drivers/s390/net/qeth_l2_main.c
+++ b/drivers/s390/net/qeth_l2_main.c
@@ -1512,7 +1512,7 @@ static void qeth_bridge_state_change(struct qeth_card *card,
QETH_CARD_TEXT(card, 2, "brstchng");
if (qports->entry_length != sizeof(struct qeth_sbp_port_entry)) {
- QETH_CARD_TEXT_(card, 2, "BPsz%.8d", qports->entry_length);
+ QETH_CARD_TEXT_(card, 2, "BPsz%04x", qports->entry_length);
return;
}
extrasize = sizeof(struct qeth_sbp_port_entry) * qports->num_entries;
diff --git a/drivers/s390/net/qeth_l3_main.c b/drivers/s390/net/qeth_l3_main.c
index afebb97..857a990 100644
--- a/drivers/s390/net/qeth_l3_main.c
+++ b/drivers/s390/net/qeth_l3_main.c
@@ -2502,7 +2502,7 @@ static int qeth_l3_arp_query(struct qeth_card *card, char __user *udata)
rc = -EFAULT;
goto free_and_out;
}
- QETH_CARD_TEXT_(card, 4, "qacts");
+ QETH_CARD_TEXT(card, 4, "qacts");
}
free_and_out:
kfree(qinfo.udata);
--
1.8.5.5
^ permalink raw reply related
* [PATCH 3/6] qeth: make local functions static in qeth_l3 module
From: Frank Blaschka @ 2014-10-22 10:18 UTC (permalink / raw)
To: davem; +Cc: netdev, linux-s390, Thomas Richter, Frank Blaschka
In-Reply-To: <1413973087-18740-1-git-send-email-blaschka@linux.vnet.ibm.com>
From: Thomas Richter <tmricht@linux.vnet.ibm.com>
This patch makes 4 local functions static and removes
the prototypes from the header file.
Signed-off-by: Thomas Richter <tmricht@linux.vnet.ibm.com>
Signed-off-by: Frank Blaschka <blaschka@linux.vnet.ibm.com>
---
drivers/s390/net/qeth_l3.h | 4 ----
drivers/s390/net/qeth_l3_main.c | 8 ++++----
2 files changed, 4 insertions(+), 8 deletions(-)
diff --git a/drivers/s390/net/qeth_l3.h b/drivers/s390/net/qeth_l3.h
index 29c1c00..551a4b4 100644
--- a/drivers/s390/net/qeth_l3.h
+++ b/drivers/s390/net/qeth_l3.h
@@ -42,10 +42,6 @@ struct qeth_ipato_entry {
};
-void qeth_l3_ipaddr4_to_string(const __u8 *, char *);
-int qeth_l3_string_to_ipaddr4(const char *, __u8 *);
-void qeth_l3_ipaddr6_to_string(const __u8 *, char *);
-int qeth_l3_string_to_ipaddr6(const char *, __u8 *);
void qeth_l3_ipaddr_to_string(enum qeth_prot_versions, const __u8 *, char *);
int qeth_l3_string_to_ipaddr(const char *, enum qeth_prot_versions, __u8 *);
int qeth_l3_create_device_attributes(struct device *);
diff --git a/drivers/s390/net/qeth_l3_main.c b/drivers/s390/net/qeth_l3_main.c
index 857a990..625227ad 100644
--- a/drivers/s390/net/qeth_l3_main.c
+++ b/drivers/s390/net/qeth_l3_main.c
@@ -55,12 +55,12 @@ static int qeth_l3_isxdigit(char *buf)
return 1;
}
-void qeth_l3_ipaddr4_to_string(const __u8 *addr, char *buf)
+static void qeth_l3_ipaddr4_to_string(const __u8 *addr, char *buf)
{
sprintf(buf, "%i.%i.%i.%i", addr[0], addr[1], addr[2], addr[3]);
}
-int qeth_l3_string_to_ipaddr4(const char *buf, __u8 *addr)
+static int qeth_l3_string_to_ipaddr4(const char *buf, __u8 *addr)
{
int count = 0, rc = 0;
unsigned int in[4];
@@ -78,12 +78,12 @@ int qeth_l3_string_to_ipaddr4(const char *buf, __u8 *addr)
return 0;
}
-void qeth_l3_ipaddr6_to_string(const __u8 *addr, char *buf)
+static void qeth_l3_ipaddr6_to_string(const __u8 *addr, char *buf)
{
sprintf(buf, "%pI6", addr);
}
-int qeth_l3_string_to_ipaddr6(const char *buf, __u8 *addr)
+static int qeth_l3_string_to_ipaddr6(const char *buf, __u8 *addr)
{
const char *end, *end_tmp, *start;
__u16 *in;
--
1.8.5.5
^ permalink raw reply related
* [PATCH 6/6] ctcm: replace sscanf by kstrto function
From: Frank Blaschka @ 2014-10-22 10:18 UTC (permalink / raw)
To: davem; +Cc: netdev, linux-s390, Thomas Richter, Frank Blaschka
In-Reply-To: <1413973087-18740-1-git-send-email-blaschka@linux.vnet.ibm.com>
From: Thomas Richter <tmricht@linux.vnet.ibm.com>
Since a single integer value is read from the supplied buffer
use the kstrto functions instead of sscanf.
Signed-off-by: Thomas Richter <tmricht@linux.vnet.ibm.com>
Signed-off-by: Frank Blaschka <blaschka@linux.vnet.ibm.com>
---
drivers/s390/net/ctcm_sysfs.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/s390/net/ctcm_sysfs.c b/drivers/s390/net/ctcm_sysfs.c
index 6bcfbbb..47773c4 100644
--- a/drivers/s390/net/ctcm_sysfs.c
+++ b/drivers/s390/net/ctcm_sysfs.c
@@ -44,8 +44,8 @@ static ssize_t ctcm_buffer_write(struct device *dev,
return -ENODEV;
}
- rc = sscanf(buf, "%u", &bs1);
- if (rc != 1)
+ rc = kstrtouint(buf, 0, &bs1);
+ if (rc)
goto einval;
if (bs1 > CTCM_BUFSIZE_LIMIT)
goto einval;
@@ -151,8 +151,8 @@ static ssize_t ctcm_proto_store(struct device *dev,
if (!priv)
return -ENODEV;
- rc = sscanf(buf, "%d", &value);
- if ((rc != 1) ||
+ rc = kstrtoint(buf, 0, &value);
+ if (rc ||
!((value == CTCM_PROTO_S390) ||
(value == CTCM_PROTO_LINUX) ||
(value == CTCM_PROTO_MPC) ||
--
1.8.5.5
^ permalink raw reply related
* [PATCH 1/6] qeth: qeth_core_main make local functions static
From: Frank Blaschka @ 2014-10-22 10:18 UTC (permalink / raw)
To: davem; +Cc: netdev, linux-s390, Thomas Richter, Frank Blaschka
In-Reply-To: <1413973087-18740-1-git-send-email-blaschka@linux.vnet.ibm.com>
From: Thomas Richter <tmricht@linux.vnet.ibm.com>
This patch makes some global functions static and removes
the prototypes from the header file.
Also function qeth_query_card_info is not exported anymore,
there is no external user for it, this function should never
have been exported in the first place.
Signed-off-by: Thomas Richter <tmricht@linux.vnet.ibm.com>
Signed-off-by: Frank Blaschka <blaschka@linux.vnet.ibm.com>
---
drivers/s390/net/qeth_core.h | 16 ----------------
drivers/s390/net/qeth_core_main.c | 18 +++++++++++++++---
2 files changed, 15 insertions(+), 19 deletions(-)
diff --git a/drivers/s390/net/qeth_core.h b/drivers/s390/net/qeth_core.h
index e7646ce..7a8bb9f 100644
--- a/drivers/s390/net/qeth_core.h
+++ b/drivers/s390/net/qeth_core.h
@@ -380,11 +380,6 @@ enum qeth_header_ids {
#define QETH_HDR_EXT_CSUM_TRANSP_REQ 0x20
#define QETH_HDR_EXT_UDP 0x40 /*bit off for TCP*/
-static inline int qeth_is_last_sbale(struct qdio_buffer_element *sbale)
-{
- return (sbale->eflags & SBAL_EFLAGS_LAST_ENTRY);
-}
-
enum qeth_qdio_buffer_states {
/*
* inbound: read out by driver; owned by hardware in order to be filled
@@ -843,13 +838,6 @@ struct qeth_trap_id {
/*some helper functions*/
#define QETH_CARD_IFNAME(card) (((card)->dev)? (card)->dev->name : "")
-static inline struct qeth_card *CARD_FROM_CDEV(struct ccw_device *cdev)
-{
- struct qeth_card *card = dev_get_drvdata(&((struct ccwgroup_device *)
- dev_get_drvdata(&cdev->dev))->dev);
- return card;
-}
-
static inline int qeth_get_micros(void)
{
return (int) (get_tod_clock() >> 12);
@@ -894,7 +882,6 @@ const char *qeth_get_cardname_short(struct qeth_card *);
int qeth_realloc_buffer_pool(struct qeth_card *, int);
int qeth_core_load_discipline(struct qeth_card *, enum qeth_discipline_id);
void qeth_core_free_discipline(struct qeth_card *);
-void qeth_buffer_reclaim_work(struct work_struct *);
/* exports for qeth discipline device drivers */
extern struct qeth_card_list_struct qeth_core_card_list;
@@ -913,7 +900,6 @@ int qeth_core_hardsetup_card(struct qeth_card *);
void qeth_print_status_message(struct qeth_card *);
int qeth_init_qdio_queues(struct qeth_card *);
int qeth_send_startlan(struct qeth_card *);
-int qeth_send_stoplan(struct qeth_card *);
int qeth_send_ipa_cmd(struct qeth_card *, struct qeth_cmd_buffer *,
int (*reply_cb)
(struct qeth_card *, struct qeth_reply *, unsigned long),
@@ -954,8 +940,6 @@ int qeth_snmp_command(struct qeth_card *, char __user *);
int qeth_query_oat_command(struct qeth_card *, char __user *);
int qeth_query_switch_attributes(struct qeth_card *card,
struct qeth_switch_info *sw_info);
-int qeth_query_card_info(struct qeth_card *card,
- struct carrier_info *carrier_info);
int qeth_send_control_data(struct qeth_card *, int, struct qeth_cmd_buffer *,
int (*reply_cb)(struct qeth_card *, struct qeth_reply*, unsigned long),
void *reply_param);
diff --git a/drivers/s390/net/qeth_core_main.c b/drivers/s390/net/qeth_core_main.c
index fd22c81..098bb0f 100644
--- a/drivers/s390/net/qeth_core_main.c
+++ b/drivers/s390/net/qeth_core_main.c
@@ -718,6 +718,13 @@ static int qeth_check_idx_response(struct qeth_card *card,
return 0;
}
+static struct qeth_card *CARD_FROM_CDEV(struct ccw_device *cdev)
+{
+ struct qeth_card *card = dev_get_drvdata(&((struct ccwgroup_device *)
+ dev_get_drvdata(&cdev->dev))->dev);
+ return card;
+}
+
static void qeth_setup_ccw(struct qeth_channel *channel, unsigned char *iob,
__u32 len)
{
@@ -1431,6 +1438,7 @@ static void qeth_start_kernel_thread(struct work_struct *work)
}
}
+static void qeth_buffer_reclaim_work(struct work_struct *);
static int qeth_setup_card(struct qeth_card *card)
{
@@ -3232,7 +3240,7 @@ int qeth_check_qdio_errors(struct qeth_card *card, struct qdio_buffer *buf,
}
EXPORT_SYMBOL_GPL(qeth_check_qdio_errors);
-void qeth_buffer_reclaim_work(struct work_struct *work)
+static void qeth_buffer_reclaim_work(struct work_struct *work)
{
struct qeth_card *card = container_of(work, struct qeth_card,
buffer_reclaim_work.work);
@@ -4717,7 +4725,7 @@ static int qeth_query_card_info_cb(struct qeth_card *card,
return 0;
}
-int qeth_query_card_info(struct qeth_card *card,
+static int qeth_query_card_info(struct qeth_card *card,
struct carrier_info *carrier_info)
{
struct qeth_cmd_buffer *iob;
@@ -4730,7 +4738,6 @@ int qeth_query_card_info(struct qeth_card *card,
return qeth_send_ipa_cmd(card, iob, qeth_query_card_info_cb,
(void *)carrier_info);
}
-EXPORT_SYMBOL_GPL(qeth_query_card_info);
static inline int qeth_get_qdio_q_format(struct qeth_card *card)
{
@@ -5113,6 +5120,11 @@ static inline int qeth_create_skb_frag(struct qeth_qdio_buffer *qethbuffer,
return 0;
}
+static inline int qeth_is_last_sbale(struct qdio_buffer_element *sbale)
+{
+ return (sbale->eflags & SBAL_EFLAGS_LAST_ENTRY);
+}
+
struct sk_buff *qeth_core_get_next_skb(struct qeth_card *card,
struct qeth_qdio_buffer *qethbuffer,
struct qdio_buffer_element **__element, int *__offset,
--
1.8.5.5
^ permalink raw reply related
* Re: [RFC PATCH v2 net-next] net: ipv6: Add a sysctl to make optimistic addresses useful candidates
From: Erik Kline @ 2014-10-22 11:15 UTC (permalink / raw)
To: Hannes Frederic Sowa; +Cc: netdev, Ben Hutchings, Lorenzo Colitti
In-Reply-To: <1413972774.2337.37.camel@localhost>
On Wed, Oct 22, 2014 at 7:12 PM, Hannes Frederic Sowa
<hannes@stressinduktion.org> wrote:
> On Mi, 2014-10-22 at 14:25 +0900, Erik Kline wrote:
>> Hello.
>>
>> On Wed, Oct 22, 2014 at 4:45 AM, Hannes Frederic Sowa
>> <hannes@stressinduktion.org> wrote:
>> > Hi,
>> >
>> > On Di, 2014-10-21 at 13:05 +0900, Erik Kline wrote:
>> >> Add a sysctl that causes an interface's optimistic addresses
>> >> to be considered equivalent to other non-deprecated addresses
>> >> for source address selection purposes. Preferred addresses
>> >> will still take precedence over optimistic addresses, subject
>> >> to other ranking in the source address selection algorithm.
>> >>
>> >> This is useful where different interfaces are connected to
>> >> different networks from different ISPs (e.g., a cell network
>> >> and a home wifi network).
>> >>
>> >> The current behaviour complies with RFC 3484/6724, and it
>> >> makes sense if the host has only one interface, or has
>> >> multiple interfaces on the same network (same or cooperating
>> >> administrative domain(s), but not in the multiple distinct
>> >> networks case.
>> >>
>> >> For example, if a mobile device has an IPv6 address on an LTE
>> >> network and then connects to IPv6-enabled wifi, while the wifi
>> >> IPv6 address is undergoing DAD, IPv6 connections will try use
>> >> the wifi default route with the LTE IPv6 address, and will get
>> >> stuck until they time out.
>> >>
>> >> Also, because optimistic addresses can actually be used, issue
>> >> an RTM_NEWADDR as soon as DAD starts. If DAD fails an separate
>> >> RTM_DELADDR will be sent.
>> >>
>> >> Also: add an entry in ip-sysctl.txt for optimistic_dad.
>> >>
>> >> Signed-off-by: Erik Kline <ek@google.com>
>> >> ---
>> >> Documentation/networking/ip-sysctl.txt | 13 ++++++++++++
>> >> include/linux/ipv6.h | 1 +
>> >> include/uapi/linux/ipv6.h | 1 +
>> >> net/ipv6/addrconf.c | 36 ++++++++++++++++++++++++++++++++--
>> >> 4 files changed, 49 insertions(+), 2 deletions(-)
>> >>
>> >> diff --git a/Documentation/networking/ip-sysctl.txt b/Documentation/networking/ip-sysctl.txt
>> >> index 0307e28..e03cf49 100644
>> >> --- a/Documentation/networking/ip-sysctl.txt
>> >> +++ b/Documentation/networking/ip-sysctl.txt
>> >> @@ -1452,6 +1452,19 @@ suppress_frag_ndisc - INTEGER
>> >> 1 - (default) discard fragmented neighbor discovery packets
>> >> 0 - allow fragmented neighbor discovery packets
>> >>
>> >> +optimistic_dad - BOOLEAN
>> >> + Whether to perform Optimistic Duplicate Address Detection (RFC 4429).
>> >> + 0: disabled (default)
>> >> + 1: enabled
>> >> +
>> >> +use_optimistic - BOOLEAN
>> >> + If enabled, do not classify optimistic addresses as deprecated during
>> >> + source address selection. Preferred addresses will still be chosen
>> >> + before optimistic addresses, subject to other ranking in the source
>> >> + address selection algorithm.
>> >> + 0: disabled (default)
>> >> + 1: enabled
>> >> +
>> >> icmp/*:
>> >> ratelimit - INTEGER
>> >> Limit the maximal rates for sending ICMPv6 packets.
>> >> diff --git a/include/linux/ipv6.h b/include/linux/ipv6.h
>> >> index ff56053..7121a2e 100644
>> >> --- a/include/linux/ipv6.h
>> >> +++ b/include/linux/ipv6.h
>> >> @@ -42,6 +42,7 @@ struct ipv6_devconf {
>> >> __s32 accept_ra_from_local;
>> >> #ifdef CONFIG_IPV6_OPTIMISTIC_DAD
>> >> __s32 optimistic_dad;
>> >> + __s32 use_optimistic;
>> >> #endif
>> >> #ifdef CONFIG_IPV6_MROUTE
>> >> __s32 mc_forwarding;
>> >> diff --git a/include/uapi/linux/ipv6.h b/include/uapi/linux/ipv6.h
>> >> index efa2666..e863d08 100644
>> >> --- a/include/uapi/linux/ipv6.h
>> >> +++ b/include/uapi/linux/ipv6.h
>> >> @@ -164,6 +164,7 @@ enum {
>> >> DEVCONF_MLDV2_UNSOLICITED_REPORT_INTERVAL,
>> >> DEVCONF_SUPPRESS_FRAG_NDISC,
>> >> DEVCONF_ACCEPT_RA_FROM_LOCAL,
>> >> + DEVCONF_USE_OPTIMISTIC,
>> >> DEVCONF_MAX
>> >> };
>> >>
>> >> diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
>> >> index 725c763..c2fddb7 100644
>> >> --- a/net/ipv6/addrconf.c
>> >> +++ b/net/ipv6/addrconf.c
>> >> @@ -1169,6 +1169,9 @@ enum {
>> >> IPV6_SADDR_RULE_LABEL,
>> >> IPV6_SADDR_RULE_PRIVACY,
>> >> IPV6_SADDR_RULE_ORCHID,
>> >> +#ifdef CONFIG_IPV6_OPTIMISTIC_DAD
>> >> + IPV6_SADDR_RULE_NOT_OPTIMISTIC,
>> >> +#endif
>> >> IPV6_SADDR_RULE_PREFIX,
>> >> IPV6_SADDR_RULE_MAX
>> >> };
>> >> @@ -1257,10 +1260,17 @@ static int ipv6_get_saddr_eval(struct net *net,
>> >> score->scopedist = ret;
>> >> break;
>> >> case IPV6_SADDR_RULE_PREFERRED:
>> >> + {
>> >> /* Rule 3: Avoid deprecated and optimistic addresses */
>> >> + u8 avoid = IFA_F_DEPRECATED;
>> >> +#ifdef CONFIG_IPV6_OPTIMISTIC_DAD
>> >> + if (!score->ifa->idev->cnf.use_optimistic)
>> >> + avoid |= IFA_F_OPTIMISTIC;
>> >> +#endif
>> >> ret = ipv6_saddr_preferred(score->addr_type) ||
>> >> - !(score->ifa->flags & (IFA_F_DEPRECATED|IFA_F_OPTIMISTIC));
>> >> + !(score->ifa->flags & avoid);
>> >> break;
>> >> + }
>> >> #ifdef CONFIG_IPV6_MIP6
>> >> case IPV6_SADDR_RULE_HOA:
>> >> {
>> >> @@ -1299,6 +1309,14 @@ static int ipv6_get_saddr_eval(struct net *net,
>> >> ret = !(ipv6_addr_orchid(&score->ifa->addr) ^
>> >> ipv6_addr_orchid(dst->addr));
>> >> break;
>> >> +#ifdef CONFIG_IPV6_OPTIMISTIC_DAD
>> >> + case IPV6_SADDR_RULE_NOT_OPTIMISTIC:
>> >> + /* Optimistic addresses still have lower precedence than other
>> >> + * preferred addresses.
>> >> + */
>> >> + ret = !(score->ifa->flags & IFA_F_OPTIMISTIC);
>> >> + break;
>> >> +#endif
>> >
>> > I wonder a bit why this rule is not directly ordered after
>> > IPV6_SADDR_RULE_PREFERRED? This would e.g. matter for privacy addresses.
>>
>> Privacy addresses ("tempaddrs") that win in earlier checks are
>> preferred before optimistic in this code (i.e. a tempaddr on the same
>> outgoing interface is preferred before an optimistic address).
>>
>> Similarly, a non-tentative non-privacy address (same outgoing
>> interface, same label, ...) will match before an optimistic address,
>> but only until DAD completes and the address is no longer optimistic.
>> I think this is in keeping with the spirit of the RFC 3484/6724 rules.
>
> Oh yes, I see. I had the evaluation order messed up. ;)
>
> So the question I should be asking would be. AFAIR optimistic addresses
> should be handled like deprecated ones, so I am a bit concerned adding a
> non-conditional rule before the RULE_PREFIX check.
>
> Shouldn't we only break the tie *after* longest prefix match then? If
> you do that before longest prefix match I would prefer ret being masked
> by use_optimisitic flag.
There was some text suggesting that the longest prefix could be
ignored (like for DNS load-balancing reasons). But I think in this
particular case it doesn't matter, so I'll move it after. Ack.
>> After there's an RFC 7217 implementation, EUI-64-based SLAAC could be
>> disabled by folks.
>
> Ack.
>
>>
>> >> case IPV6_SADDR_RULE_PREFIX:
>> >> /* Rule 8: Use longest matching prefix */
>> >> ret = ipv6_addr_diff(&score->ifa->addr, dst->addr);
>> >> @@ -3222,8 +3240,13 @@ static void addrconf_dad_begin(struct inet6_ifaddr *ifp)
>> >> * Optimistic nodes can start receiving
>> >> * Frames right away
>> >> */
>> >> - if (ifp->flags & IFA_F_OPTIMISTIC)
>> >> + if (ifp->flags & IFA_F_OPTIMISTIC) {
>> >> ip6_ins_rt(ifp->rt);
>> >> + /* Because optimistic nodes can receive frames, notify
>> >> + * listeners. If DAD fails, RTM_DELADDR is sent.
>> >> + */
>> >> + ipv6_ifa_notify(RTM_NEWADDR, ifp);
>> >> + }
>> >
>> > I wonder if we can now delete the ipv6_ifa_notify(RTM_NEWADDR, ifp) in
>> > addrconf_dad_completed.
>>
>> I don't know what everyone's general preference would be, but mine
>> would be to err on the side of notifying on state changes. It seems
>> harmless to me to keep it in, and something in userspace might want to
>> know if/when DAD completes.
>
> Userspace expects to communicate with an address which gets announced
> via RTM_NEWADDR, so I would make the RTM_NEWADDR notifications
> conditional on use_optimistic flag in both, the completed and the
> dad_begin function. This sounds like the best option to me, no?
That's easy enough to do in addrconf_dad_begin(). Unfortunately, by
the time we get to addrconf_dad_completed() the IFA_F_OPTIMISTIC flag
has already been cleared.
I have a version that attempts to take its best guess as to whether an
RTM_NEWADDR _should_ have already been sent--something like:
#ifdef CONFIG_IPV6_OPTIMISTIC_DAD
// We probably already sent a notification in addrconf_dad_begin().
if (!ifp->idev->cnf.optimistic_dad || !ifp->idev->cnf.use_optimistic)
#endif
ipv6_ifa_notify(RTM_NEWADDR, ifp);
but that doesn't seem that clean to me (apart from the ifdef-y
messiness of it). Do you think this "best guess" approach is
reasonable?
With just the "use_optimistic" check in addrconf_dad_begin(),
userspace can still communicate with addresses it gets via
RTM_NEWADDR, it will just get /two/ such notifications: one when it
can probably use it and one when it definitely can.
Thoughts?
^ permalink raw reply
* Re: [stable request <= 3.11] net/mlx4_en: Fix BlueFlame race
From: Amir Vadai @ 2014-10-22 11:15 UTC (permalink / raw)
To: David S. Miller
Cc: Or Gerlitz, Cong Wang, Vinson Lee, Or Gerlitz, Jack Morgenstein,
Eugenia Emantayev, Matan Barak, netdev, saeedm
In-Reply-To: <CAJ3xEMiOEZ9fDtLd4qqPmqAGb3hZneBXidyaxA9Pv1Ki6cKnAg@mail.gmail.com>
On 10/22/2014 7:17 AM, Or Gerlitz wrote:
> On Wed, Oct 22, 2014 at 2:15 AM, Cong Wang <cwang@twopensource.com> wrote:
>> On Sat, Oct 18, 2014 at 2:14 PM, Vinson Lee <vlee@twopensource.com> wrote:
>>> Hi.
>>>
>>> Please consider backporting upstream commit
>>> 2d4b646613d6b12175b017aca18113945af1faf3 "net/mlx4_en: Fix BlueFlame
>>> race" to stable kernels <= 3.11.
>>>
>>
>> David, could you take care of it if you have time? It fixes a real
>> bug in production. :)
>
> Let out folks here look on that 1st.
>
> Or.
>
Verified patch above [1] on 3.10.y (v3.10.58) as is.
Dave, please pull it to stable 3.10
I don't know about an LTS 3.11 kernel - but if there is such an option,
I verified that the patch also applies cleanly on 3.11.y.
Thanks,
Amir
[1] - 2d4b646 ("net/mlx4_en: Fix BlueFlame race")
^ permalink raw reply
* Re: [RFC PATCH v2 net-next] net: ipv6: Add a sysctl to make optimistic addresses useful candidates
From: Hannes Frederic Sowa @ 2014-10-22 11:36 UTC (permalink / raw)
To: Erik Kline; +Cc: netdev, Ben Hutchings, Lorenzo Colitti
In-Reply-To: <CAAedzxpG1TEVVm_kHgRJcOO7qkFmyyYnfPL0ONN7GcypmtRq1g@mail.gmail.com>
Hi,
On Mi, 2014-10-22 at 20:15 +0900, Erik Kline wrote:
> >> >> case IPV6_SADDR_RULE_PREFIX:
> >> >> /* Rule 8: Use longest matching prefix */
> >> >> ret = ipv6_addr_diff(&score->ifa->addr, dst->addr);
> >> >> @@ -3222,8 +3240,13 @@ static void addrconf_dad_begin(struct inet6_ifaddr *ifp)
> >> >> * Optimistic nodes can start receiving
> >> >> * Frames right away
> >> >> */
> >> >> - if (ifp->flags & IFA_F_OPTIMISTIC)
> >> >> + if (ifp->flags & IFA_F_OPTIMISTIC) {
> >> >> ip6_ins_rt(ifp->rt);
> >> >> + /* Because optimistic nodes can receive frames, notify
> >> >> + * listeners. If DAD fails, RTM_DELADDR is sent.
> >> >> + */
> >> >> + ipv6_ifa_notify(RTM_NEWADDR, ifp);
> >> >> + }
> >> >
> >> > I wonder if we can now delete the ipv6_ifa_notify(RTM_NEWADDR, ifp) in
> >> > addrconf_dad_completed.
> >>
> >> I don't know what everyone's general preference would be, but mine
> >> would be to err on the side of notifying on state changes. It seems
> >> harmless to me to keep it in, and something in userspace might want to
> >> know if/when DAD completes.
> >
> > Userspace expects to communicate with an address which gets announced
> > via RTM_NEWADDR, so I would make the RTM_NEWADDR notifications
> > conditional on use_optimistic flag in both, the completed and the
> > dad_begin function. This sounds like the best option to me, no?
>
> That's easy enough to do in addrconf_dad_begin(). Unfortunately, by
> the time we get to addrconf_dad_completed() the IFA_F_OPTIMISTIC flag
> has already been cleared.
>
> I have a version that attempts to take its best guess as to whether an
> RTM_NEWADDR _should_ have already been sent--something like:
>
> #ifdef CONFIG_IPV6_OPTIMISTIC_DAD
> // We probably already sent a notification in addrconf_dad_begin().
> if (!ifp->idev->cnf.optimistic_dad || !ifp->idev->cnf.use_optimistic)
> #endif
> ipv6_ifa_notify(RTM_NEWADDR, ifp);
>
> but that doesn't seem that clean to me (apart from the ifdef-y
> messiness of it). Do you think this "best guess" approach is
> reasonable?
I would definitely ack a patch which removes the macros and the config
option CONFIG_IPV6_OPTIMISTIC_DAD completely.
You also can split the ifdefed part into a small helper function:
static inline bool ipv6_use_optimistic_addr(struct inet6_dev *idev)
{
#ifdef CONFIG_IPV6_OPTIMISTIC_DAD
return idev->cnf.optimistic_dad && idev->cnf.use_optimistic;
#else
return false;
#endif
}
...
and then in both locations update the RTM_NEWADDR like this:
addrconf_dad_begin:
if (ipv6_use_optimisitic_addr(idev))
notify...
and the dad_completed with a negated check. I think this is the easiest
option.
> With just the "use_optimistic" check in addrconf_dad_begin(),
> userspace can still communicate with addresses it gets via
> RTM_NEWADDR, it will just get /two/ such notifications: one when it
> can probably use it and one when it definitely can.
>
> Thoughts?
In the near future we also must introduce specific DAD events needed for
RFC7217 addresses (to count DAD progress and safe the information per
prefix in user space). I would prefer to keep the logic for RTM_NEWADDR
that the kernel will definitely allow the use of the address but
RTM_NEWADDR should be handled idempotent by user space.
Thanks,
Hannes
^ permalink raw reply
* Re: [PATCH net-next 1/6] ethernet: wiznet: remove unnecessary check
From: Sergei Shtylyov @ 2014-10-22 11:43 UTC (permalink / raw)
To: Varka Bhadram, netdev; +Cc: Varka Bhadram
In-Reply-To: <1413951386-29645-2-git-send-email-varkab@cdac.in>
Hello.
On 10/22/2014 8:16 AM, Varka Bhadram wrote:
> devm_ioremap_resource checks platform_get_resource() return value.
> We can remove the duplicate check here.
> Signed-off-by: Varka Bhadram <varkab@cdac.in>
> ---
> drivers/net/ethernet/wiznet/w5100.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
> diff --git a/drivers/net/ethernet/wiznet/w5100.c b/drivers/net/ethernet/wiznet/w5100.c
> index 0f56b1c..bf195e3 100644
> --- a/drivers/net/ethernet/wiznet/w5100.c
> +++ b/drivers/net/ethernet/wiznet/w5100.c
> @@ -638,8 +638,7 @@ static int w5100_hw_probe(struct platform_device *pdev)
> }
>
> mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> - if (!mem)
> - return -ENXIO;
> +
> mem_size = resource_size(mem);
This would cause a NULL dereference on resource_size() call if 'mem' is
NULL. You can't remove the NULL check because of that.
>
> priv->base = devm_ioremap_resource(&pdev->dev, mem);
>
WBR, Sergei
^ permalink raw reply
* Re: [PATCH net-next 2/6] ethernet: wiznet: remove unnecessary check
From: Sergei Shtylyov @ 2014-10-22 11:44 UTC (permalink / raw)
To: Varka Bhadram, netdev; +Cc: Varka Bhadram
In-Reply-To: <1413951386-29645-3-git-send-email-varkab@cdac.in>
On 10/22/2014 8:16 AM, Varka Bhadram wrote:
> devm_ioremap_resource checks platform_get_resource() return value.
> We can remove the duplicate check here.
> Signed-off-by: Varka Bhadram <varkab@cdac.in>
> ---
> drivers/net/ethernet/wiznet/w5300.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
> diff --git a/drivers/net/ethernet/wiznet/w5300.c b/drivers/net/ethernet/wiznet/w5300.c
> index f961f14..315d090 100644
> --- a/drivers/net/ethernet/wiznet/w5300.c
> +++ b/drivers/net/ethernet/wiznet/w5300.c
> @@ -558,8 +558,7 @@ static int w5300_hw_probe(struct platform_device *pdev)
> }
>
> mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> - if (!mem)
> - return -ENXIO;
> +
> mem_size = resource_size(mem);
Same problem as in the patch #1
>
> priv->base = devm_ioremap_resource(&pdev->dev, mem);
>
WBR, Sergei
^ permalink raw reply
* Re: [PATCH net-next 5/6] ethernet: renesas: remove unnecessary check
From: Sergei Shtylyov @ 2014-10-22 11:47 UTC (permalink / raw)
To: Varka Bhadram, netdev; +Cc: Varka Bhadram
In-Reply-To: <1413951386-29645-6-git-send-email-varkab@cdac.in>
On 10/22/2014 8:16 AM, Varka Bhadram wrote:
> devm_ioremap_resource checks platform_get_resource() return value.
> We can remove the duplicate check here.
> Signed-off-by: Varka Bhadram <varkab@cdac.in>
> ---
> drivers/net/ethernet/renesas/sh_eth.c | 4 ----
> 1 file changed, 4 deletions(-)
> diff --git a/drivers/net/ethernet/renesas/sh_eth.c b/drivers/net/ethernet/renesas/sh_eth.c
> index 60e9c2c..d824ba5 100644
> --- a/drivers/net/ethernet/renesas/sh_eth.c
> +++ b/drivers/net/ethernet/renesas/sh_eth.c
> @@ -2769,10 +2769,6 @@ static int sh_eth_drv_probe(struct platform_device *pdev)
>
> /* get base addr */
> res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> - if (unlikely(res == NULL)) {
> - dev_err(&pdev->dev, "invalid resource\n");
> - return -EINVAL;
> - }
The driver dereferences 'res' further on, so you can't remove this check.
WBR, Sergei
^ permalink raw reply
* Re: [RFC PATCH v2 net-next] net: ipv6: Add a sysctl to make optimistic addresses useful candidates
From: Lorenzo Colitti @ 2014-10-22 11:50 UTC (permalink / raw)
To: Hannes Frederic Sowa; +Cc: Erik Kline, netdev, Ben Hutchings
In-Reply-To: <1413977771.2337.50.camel@localhost>
On Wed, Oct 22, 2014 at 8:36 PM, Hannes Frederic Sowa
<hannes@stressinduktion.org> wrote:
> In the near future we also must introduce specific DAD events needed for
> RFC7217 addresses (to count DAD progress and safe the information per
> prefix in user space). I would prefer to keep the logic for RTM_NEWADDR
> that the kernel will definitely allow the use of the address but
> RTM_NEWADDR should be handled idempotent by user space.
Duplicate RTM_NEWADDR notifications aren't a problem though, right?
The current code send RTM_NEWADDR every time an RA increases the
lifetime of an address, so I think it's reasonable to assume that apps
have to be able to deal with it, because it's pretty frequent.
^ permalink raw reply
* Re: [PATCH] netfilter: log: protect nf_log_register against double registering
From: Pablo Neira Ayuso @ 2014-10-22 12:02 UTC (permalink / raw)
To: Marcelo Ricardo Leitner; +Cc: netfilter-devel, netdev
In-Reply-To: <c7fcb46fa24c1a64f00c9322bb5cddbde977da0a.1413842217.git.mleitner@redhat.com>
On Mon, Oct 20, 2014 at 07:58:03PM -0200, Marcelo Ricardo Leitner wrote:
> Currently, despite the comment right before the function,
> nf_log_register allows registering two loggers on with the same type and
> end up overwriting the previous register.
>
> Not a real issue today as current tree doesn't have two loggers for the
> same type but it's better to get this protected.
>
> Also make sure that all of its callers do error checking.
No major objetions to this sanity check. Some comment below.
> Signed-off-by: Marcelo Ricardo Leitner <mleitner@redhat.com>
> ---
>
> Notes:
> Please let me know if you have any issues with the identation on
> nf_log_register. I just couldn't find a better one.
You can split nf_log_register() in two functions to avoid this.
> Thanks
>
> net/ipv4/netfilter/nf_log_arp.c | 8 +++++++-
> net/ipv4/netfilter/nf_log_ipv4.c | 8 +++++++-
> net/ipv6/netfilter/nf_log_ipv6.c | 8 +++++++-
> net/netfilter/nf_log.c | 13 ++++++++++++-
> 4 files changed, 33 insertions(+), 4 deletions(-)
>
> diff --git a/net/ipv4/netfilter/nf_log_arp.c b/net/ipv4/netfilter/nf_log_arp.c
> index ccfc78db12ee8acae68faf451f2cf6bc5597f2c1..8b39174b7be390397a110ec9d3ed497bf8ce6d26 100644
> --- a/net/ipv4/netfilter/nf_log_arp.c
> +++ b/net/ipv4/netfilter/nf_log_arp.c
> @@ -130,7 +130,13 @@ static int __init nf_log_arp_init(void)
> if (ret < 0)
> return ret;
>
> - nf_log_register(NFPROTO_ARP, &nf_arp_logger);
> + ret = nf_log_register(NFPROTO_ARP, &nf_arp_logger);
> + if (ret < 0) {
> + pr_err("log: failed to register logger\n");
I think you can use pr_fmt to avoid appending log, it's also going to
append useful information to know which logger has indeed failed.
> + unregister_pernet_subsys(&nf_log_arp_net_ops);
> + return ret;
Could you add a goto err1; instead? Just in case we need to extend
this again later on, we'll skip some refactoring.
Thanks.
^ permalink raw reply
* Re: [PATCH nf] netfilter: nf_tables: check for NULL in nf_tables_newchain pcpu stats allocation
From: Pablo Neira Ayuso @ 2014-10-22 12:09 UTC (permalink / raw)
To: Sabrina Dubroca; +Cc: kaber, kadlec, netfilter-devel, coreteam, netdev
In-Reply-To: <20141021090821.GA31730@kria>
On Tue, Oct 21, 2014 at 11:08:21AM +0200, Sabrina Dubroca wrote:
> alloc_percpu returns NULL on failure, not a negative error code.
Good catch. Applied, thanks.
^ permalink raw reply
* Re: [RFC PATCH v2 net-next] net: ipv6: Add a sysctl to make optimistic addresses useful candidates
From: Hannes Frederic Sowa @ 2014-10-22 12:13 UTC (permalink / raw)
To: Lorenzo Colitti; +Cc: Erik Kline, netdev, Ben Hutchings
In-Reply-To: <CAKD1Yr08muZ0Dwwt5Xmi+mEFP9OBBYe9CceswsEzpm5PkMUCMg@mail.gmail.com>
On Mi, 2014-10-22 at 20:50 +0900, Lorenzo Colitti wrote:
> On Wed, Oct 22, 2014 at 8:36 PM, Hannes Frederic Sowa
> <hannes@stressinduktion.org> wrote:
> > In the near future we also must introduce specific DAD events needed for
> > RFC7217 addresses (to count DAD progress and safe the information per
> > prefix in user space). I would prefer to keep the logic for RTM_NEWADDR
> > that the kernel will definitely allow the use of the address but
> > RTM_NEWADDR should be handled idempotent by user space.
>
> Duplicate RTM_NEWADDR notifications aren't a problem though, right?
> The current code send RTM_NEWADDR every time an RA increases the
> lifetime of an address, so I think it's reasonable to assume that apps
> have to be able to deal with it, because it's pretty frequent.
I don't think it is a serious problem (or a problem at all). I would
just try to reduce the number of notifications, that's all.
Bye,
Hannes
^ permalink raw reply
* Re: [PATCH net-next 5/6] ethernet: renesas: remove unnecessary check
From: Varka Bhadram @ 2014-10-22 12:22 UTC (permalink / raw)
To: Sergei Shtylyov, netdev; +Cc: Varka Bhadram
In-Reply-To: <5447995F.5080602@cogentembedded.com>
On 10/22/2014 05:17 PM, Sergei Shtylyov wrote:
> On 10/22/2014 8:16 AM, Varka Bhadram wrote:
>
>> devm_ioremap_resource checks platform_get_resource() return value.
>> We can remove the duplicate check here.
>
>> Signed-off-by: Varka Bhadram <varkab@cdac.in>
>> ---
>> drivers/net/ethernet/renesas/sh_eth.c | 4 ----
>> 1 file changed, 4 deletions(-)
>
>> diff --git a/drivers/net/ethernet/renesas/sh_eth.c
>> b/drivers/net/ethernet/renesas/sh_eth.c
>> index 60e9c2c..d824ba5 100644
>> --- a/drivers/net/ethernet/renesas/sh_eth.c
>> +++ b/drivers/net/ethernet/renesas/sh_eth.c
>> @@ -2769,10 +2769,6 @@ static int sh_eth_drv_probe(struct
>> platform_device *pdev)
>>
>> /* get base addr */
>> res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> - if (unlikely(res == NULL)) {
>> - dev_err(&pdev->dev, "invalid resource\n");
>> - return -EINVAL;
>> - }
>
> The driver dereferences 'res' further on, so you can't remove this
> check.
>
Yes its my mistake . I will fix it . Thankx
> WBR, Sergei
>
--
Regards,
Varka Bhadram.
^ 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