* Re: [PATCHv2] am33xx: cpsw: default to ethernet hwaddr from efuse if not defined in dt
From: Michal Bachraty @ 2013-01-18 8:26 UTC (permalink / raw)
To: Mugunthan V N
Cc: michal.bachraty-6oiIBCxl0MMjD8S081q9vkEOCMrvLtNR,
netdev-u79uwXL29TY76Z2rM5mHXA,
devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
linux-omap-u79uwXL29TY76Z2rM5mHXA,
richardcochran-Re5JQEeQqe8AvxtiuMwx3w
In-Reply-To: <50F8DA3A.3090406-l0cyMroinI0@public.gmane.org>
[-- Attachment #1.1: Type: text/plain, Size: 1641 bytes --]
On Fri, Jan 18, 2013 at 6:14 AM, Mugunthan V N <mugunthanvnm-l0cyMroinI0@public.gmane.org> wrote:
> On 1/18/2013 3:48 AM, Peter Korsgaard wrote:
>
>> When booting with CONFIG_ARM_APPENDED_DTB (either because of using an old
>> U-Boot, not wanting the hassle of 2 files or when using Falcon fast boot
>> mode in U-Boot), nothing updates the ethernet hwaddr specified for the
>> CPSW slaves, causing the driver to use a random hwaddr, which is some
>> times
>> troublesome.
>>
>> The am33xx has unique ethernet hwaddrs programmed in the efuse, so it
>> makes
>> more sense to default to these rather than random ones. Add a fixup step
>> which adds mac-address dt properties using the efuse addresses if the DTB
>> didn't contain valid ones.
>>
>> Signed-off-by: Peter Korsgaard <jacmet-OfajU3CKLf1/SzgSGea1oA@public.gmane.org>
>>
>>
> This implementation looks fine.
> Acked-by: Mugunthan V N <mugunthanvnm-l0cyMroinI0@public.gmane.org>
>
> Regards
> Mugunthan V N
>
Thanks,
My approach for cpsw driver is posted here:
https://patchwork.kernel.org/patch/1966481/
I added DT option mac-address-source because there is still possiblility to
have mac address set by the u-boot in the future. Beaglebone have special
eeprom on board, were you can have stored user MAC and u-boot can read
eeprom
data. Maybe some users will want this feature. Also in cpsw dt binding
documentation, there is mention /* Filled in by U-Boot */. For that
purpose,
code I proposed can easily deal with any driver exteds.
Reading CPU mac address registers with (my approach) or without (your
approach) DT should be more discussed. Also your way seems to be fine.
[-- Attachment #1.2: Type: text/html, Size: 2435 bytes --]
[-- Attachment #2: Type: text/plain, Size: 192 bytes --]
_______________________________________________
devicetree-discuss mailing list
devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
^ permalink raw reply
* [RFC PATCH 3/3] xfrm4: Invalidate all ipv4 routes on IPsec pmtu events
From: Steffen Klassert @ 2013-01-18 8:16 UTC (permalink / raw)
To: Yurij M. Plotnikov; +Cc: Ben Hutchings, netdev, Alexandra N. Kossovsky
In-Reply-To: <20130118081145.GB24987@secunet.com>
On IPsec pmtu events we can't access the transport headers of
the original packet, so we can't find the socket that sent
the packet. The only chance to notify the socket about the
pmtu change is to force a relookup for all routes. This
patch implenents this for the IPsec protocols.
Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
---
net/ipv4/ah4.c | 7 +++++--
net/ipv4/esp4.c | 7 +++++--
net/ipv4/ipcomp.c | 7 +++++--
3 files changed, 15 insertions(+), 6 deletions(-)
diff --git a/net/ipv4/ah4.c b/net/ipv4/ah4.c
index a0d8392..612ecc9 100644
--- a/net/ipv4/ah4.c
+++ b/net/ipv4/ah4.c
@@ -413,9 +413,12 @@ static void ah4_err(struct sk_buff *skb, u32 info)
if (!x)
return;
- if (icmp_hdr(skb)->type == ICMP_DEST_UNREACH)
+ if (icmp_hdr(skb)->type == ICMP_DEST_UNREACH) {
+ atomic_inc(&flow_cache_genid);
+ rt_genid_bump(net);
+
ipv4_update_pmtu(skb, net, info, 0, 0, IPPROTO_AH, 0);
- else
+ } else
ipv4_redirect(skb, net, 0, 0, IPPROTO_AH, 0);
xfrm_state_put(x);
}
diff --git a/net/ipv4/esp4.c b/net/ipv4/esp4.c
index b61e9de..2d32ae5 100644
--- a/net/ipv4/esp4.c
+++ b/net/ipv4/esp4.c
@@ -499,9 +499,12 @@ static void esp4_err(struct sk_buff *skb, u32 info)
if (!x)
return;
- if (icmp_hdr(skb)->type == ICMP_DEST_UNREACH)
+ if (icmp_hdr(skb)->type == ICMP_DEST_UNREACH) {
+ atomic_inc(&flow_cache_genid);
+ rt_genid_bump(net);
+
ipv4_update_pmtu(skb, net, info, 0, 0, IPPROTO_ESP, 0);
- else
+ } else
ipv4_redirect(skb, net, 0, 0, IPPROTO_ESP, 0);
xfrm_state_put(x);
}
diff --git a/net/ipv4/ipcomp.c b/net/ipv4/ipcomp.c
index d3ab47e..9a46dae 100644
--- a/net/ipv4/ipcomp.c
+++ b/net/ipv4/ipcomp.c
@@ -47,9 +47,12 @@ static void ipcomp4_err(struct sk_buff *skb, u32 info)
if (!x)
return;
- if (icmp_hdr(skb)->type == ICMP_DEST_UNREACH)
+ if (icmp_hdr(skb)->type == ICMP_DEST_UNREACH) {
+ atomic_inc(&flow_cache_genid);
+ rt_genid_bump(net);
+
ipv4_update_pmtu(skb, net, info, 0, 0, IPPROTO_COMP, 0);
- else
+ } else
ipv4_redirect(skb, net, 0, 0, IPPROTO_COMP, 0);
xfrm_state_put(x);
}
--
1.7.9.5
^ permalink raw reply related
* [RFC PATCH 2/3] ipv4: Add a socket release callback for datagram sockets
From: Steffen Klassert @ 2013-01-18 8:15 UTC (permalink / raw)
To: Yurij M. Plotnikov; +Cc: Ben Hutchings, netdev, Alexandra N. Kossovsky
In-Reply-To: <20130118081145.GB24987@secunet.com>
This implements a socket release callback function to check
if the socket cached route got invalid during the time
we owned the socket. The function is used from udp, raw
and ping sockets.
Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
---
include/net/ip.h | 2 ++
net/ipv4/datagram.c | 25 +++++++++++++++++++++++++
net/ipv4/ping.c | 1 +
net/ipv4/raw.c | 1 +
net/ipv4/udp.c | 1 +
5 files changed, 30 insertions(+)
diff --git a/include/net/ip.h b/include/net/ip.h
index 0707fb9..a68f838 100644
--- a/include/net/ip.h
+++ b/include/net/ip.h
@@ -143,6 +143,8 @@ static inline struct sk_buff *ip_finish_skb(struct sock *sk, struct flowi4 *fl4)
extern int ip4_datagram_connect(struct sock *sk,
struct sockaddr *uaddr, int addr_len);
+extern void ip4_datagram_release_cb(struct sock *sk);
+
struct ip_reply_arg {
struct kvec iov[1];
int flags;
diff --git a/net/ipv4/datagram.c b/net/ipv4/datagram.c
index 424fafb..b28e863 100644
--- a/net/ipv4/datagram.c
+++ b/net/ipv4/datagram.c
@@ -85,3 +85,28 @@ out:
return err;
}
EXPORT_SYMBOL(ip4_datagram_connect);
+
+void ip4_datagram_release_cb(struct sock *sk)
+{
+ const struct inet_sock *inet = inet_sk(sk);
+ const struct ip_options_rcu *inet_opt;
+ __be32 daddr = inet->inet_daddr;
+ struct flowi4 fl4;
+ struct rtable *rt;
+
+ if (! __sk_dst_get(sk) || __sk_dst_check(sk, 0))
+ return;
+
+ rcu_read_lock();
+ inet_opt = rcu_dereference(inet->inet_opt);
+ if (inet_opt && inet_opt->opt.srr)
+ daddr = inet_opt->opt.faddr;
+ rt = ip_route_output_ports(sock_net(sk), &fl4, sk, daddr,
+ inet->inet_saddr, inet->inet_dport,
+ inet->inet_sport, sk->sk_protocol,
+ RT_CONN_FLAGS(sk), sk->sk_bound_dev_if);
+ if (!IS_ERR(rt))
+ __sk_dst_set(sk, &rt->dst);
+ rcu_read_unlock();
+}
+EXPORT_SYMBOL_GPL(ip4_datagram_release_cb);
diff --git a/net/ipv4/ping.c b/net/ipv4/ping.c
index 8f3d054..6f9c072 100644
--- a/net/ipv4/ping.c
+++ b/net/ipv4/ping.c
@@ -738,6 +738,7 @@ struct proto ping_prot = {
.recvmsg = ping_recvmsg,
.bind = ping_bind,
.backlog_rcv = ping_queue_rcv_skb,
+ .release_cb = ip4_datagram_release_cb,
.hash = ping_v4_hash,
.unhash = ping_v4_unhash,
.get_port = ping_v4_get_port,
diff --git a/net/ipv4/raw.c b/net/ipv4/raw.c
index 73d1e4d..6f08991 100644
--- a/net/ipv4/raw.c
+++ b/net/ipv4/raw.c
@@ -894,6 +894,7 @@ struct proto raw_prot = {
.recvmsg = raw_recvmsg,
.bind = raw_bind,
.backlog_rcv = raw_rcv_skb,
+ .release_cb = ip4_datagram_release_cb,
.hash = raw_hash_sk,
.unhash = raw_unhash_sk,
.obj_size = sizeof(struct raw_sock),
diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
index 79c8dbe..1f4d405 100644
--- a/net/ipv4/udp.c
+++ b/net/ipv4/udp.c
@@ -1952,6 +1952,7 @@ struct proto udp_prot = {
.recvmsg = udp_recvmsg,
.sendpage = udp_sendpage,
.backlog_rcv = __udp_queue_rcv_skb,
+ .release_cb = ip4_datagram_release_cb,
.hash = udp_lib_hash,
.unhash = udp_lib_unhash,
.rehash = udp_v4_rehash,
--
1.7.9.5
^ permalink raw reply related
* [RFC PATCH 1/3] ipv4: Invalidate the socket cached route on pmtu events if possible
From: Steffen Klassert @ 2013-01-18 8:14 UTC (permalink / raw)
To: Yurij M. Plotnikov; +Cc: Ben Hutchings, netdev, Alexandra N. Kossovsky
In-Reply-To: <20130118081145.GB24987@secunet.com>
The route lookup in ipv4_sk_update_pmtu() might return a route
different from the route we cached at the socket. This is because
standart routes are per cpu, so each cpu has it's own struct rtable.
This means that we do not invalidate the socket cached route if the
NET_RX_SOFTIRQ is not served by the same cpu that the sending socket
uses. As a result, the cached route reused until we disconnect.
With this patch we invalidate the socket cached route if possible.
If the socket is owened by the user, we can't update the cached
route directly. A followup patch will implement socket release
callback functions for datagram sockets to handle this case.
Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
---
net/ipv4/route.c | 40 +++++++++++++++++++++++++++++++++++++++-
1 file changed, 39 insertions(+), 1 deletion(-)
diff --git a/net/ipv4/route.c b/net/ipv4/route.c
index 259cbee..e59541b 100644
--- a/net/ipv4/route.c
+++ b/net/ipv4/route.c
@@ -965,7 +965,7 @@ void ipv4_update_pmtu(struct sk_buff *skb, struct net *net, u32 mtu,
}
EXPORT_SYMBOL_GPL(ipv4_update_pmtu);
-void ipv4_sk_update_pmtu(struct sk_buff *skb, struct sock *sk, u32 mtu)
+static void __ipv4_sk_update_pmtu(struct sk_buff *skb, struct sock *sk, u32 mtu)
{
const struct iphdr *iph = (const struct iphdr *) skb->data;
struct flowi4 fl4;
@@ -978,6 +978,44 @@ void ipv4_sk_update_pmtu(struct sk_buff *skb, struct sock *sk, u32 mtu)
ip_rt_put(rt);
}
}
+
+void ipv4_sk_update_pmtu(struct sk_buff *skb, struct sock *sk, u32 mtu)
+{
+ const struct iphdr *iph = (const struct iphdr *) skb->data;
+ struct flowi4 fl4;
+ struct rtable *rt = (struct rtable *) __sk_dst_get(sk);
+ struct dst_entry *dst;
+
+ bh_lock_sock(sk);
+ if (sock_owned_by_user(sk) || !rt) {
+ __ipv4_sk_update_pmtu(skb, sk, mtu);
+ goto out;
+ }
+
+ __build_flow_key(&fl4, sk, iph, 0, 0, 0, 0, 0);
+
+ if (!__sk_dst_check(sk, 0)) {
+ rt = ip_route_output_flow(sock_net(sk), &fl4, sk);
+ if (IS_ERR(rt))
+ goto out;
+ }
+
+ __ip_rt_update_pmtu((struct rtable *) rt->dst.path, &fl4, mtu);
+
+ dst = dst_check(&rt->dst, 0);
+ if (!dst) {
+ rt = ip_route_output_flow(sock_net(sk), &fl4, sk);
+ if (IS_ERR(rt))
+ goto out;
+
+ dst = &rt->dst;
+ }
+
+ __sk_dst_set(sk, dst);
+
+out:
+ bh_unlock_sock(sk);
+}
EXPORT_SYMBOL_GPL(ipv4_sk_update_pmtu);
void ipv4_redirect(struct sk_buff *skb, struct net *net,
--
1.7.9.5
^ permalink raw reply related
* Re: PMTU discovery is broken on kernel 3.7.1 for UDP sockets
From: Steffen Klassert @ 2013-01-18 8:11 UTC (permalink / raw)
To: Yurij M. Plotnikov; +Cc: Ben Hutchings, netdev, Alexandra N. Kossovsky
In-Reply-To: <50F3C153.9030204@oktetlabs.ru>
On Mon, Jan 14, 2013 at 12:26:59PM +0400, Yurij M. Plotnikov wrote:
>
> Could you, please, tell me is there any news about this bug?
>
Ok, I tried to reconstruct your testcase and I can confirm this bug.
It happens only on the first pmtu event on a given route. It works
if I run this test again. Actually it works sometimes even on the
first pmtu event, so it took me quite a while to understand what's ging on.
This is because standart routes (no pmtu or redirect events happened on
that route) are per cpu, so each cpu has it's own struct rtable. This means
that we do not invalidate the socket cached route if the NET_RX_SOFTIRQ
(which handles the pmtu event) is not served by the same cpu that the
sending socket uses. Exceptional routes (routes where a pmtu or redirect
event occured) are not per cpu, so there is no problem.
I'll send three patches in reply to this mail. Applying patch one and two
should fix the bug you reported. If you use IPsec you need patch three too.
The patches are marked as RFC, I'll do proper patch submission if you can
confirm that they fix this bug.
^ permalink raw reply
* Re: [PATCH] net: usb: initialize tmp in dm9601.c to avoid warning
From: David Miller @ 2013-01-18 8:03 UTC (permalink / raw)
To: jacmet; +Cc: sque, netdev, msb
In-Reply-To: <87sj5zj60q.fsf@dell.be.48ers.dk>
From: Peter Korsgaard <jacmet@sunsite.dk>
Date: Fri, 18 Jan 2013 08:37:41 +0100
>>>>>> "David" == David Miller <davem@davemloft.net> writes:
>
> >> What warning and what compiler version are you using?
>
> David> I wouldn't bother asking this, unless it's so that you personally
> David> can reproduce this in the future.
>
> David> The compiler cannot determine with %100 certainty that tmp is
> David> initialized in all paths. Any time there are conditional
> David> statements guarding setting and the use, the compiler is going
> David> to err on the side of caution and say that if any of those
> David> paths lead to a use without a set it's going to warn.
>
> Sure, I just wondered as the code hasn't changed in 5+ years, and the
> function that gets called is defined just above, so the compiler should
> be able to figure it out.
Nothing guarentees that a piece of memory passed into an external
function has it's contents initialized completely by that function.
The compiler can't see what usbnet_read_cmd() does with the memory
behind the 'data' pointer argument. So it makes the only assumption
it can make, which is that 'tmp' might be uninitialized.
^ permalink raw reply
* [PATCH v2] net: fix wrong length of mac address
From: Amos Kong @ 2013-01-18 7:51 UTC (permalink / raw)
To: davem, kys, haiyangz; +Cc: netdev, Amos Kong, devel
Typo, it should be ETH_ALEN.
Signed-off-by: Amos Kong <kongjianjun@gmail.com>
---
v2: update commitlog, fix cc list
---
drivers/net/hyperv/netvsc_drv.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/hyperv/netvsc_drv.c b/drivers/net/hyperv/netvsc_drv.c
index f825a62..8264f0e 100644
--- a/drivers/net/hyperv/netvsc_drv.c
+++ b/drivers/net/hyperv/netvsc_drv.c
@@ -349,7 +349,7 @@ static int netvsc_set_mac_addr(struct net_device *ndev, void *p)
struct net_device_context *ndevctx = netdev_priv(ndev);
struct hv_device *hdev = ndevctx->device_ctx;
struct sockaddr *addr = p;
- char save_adr[14];
+ char save_adr[ETH_ALEN];
unsigned char save_aatype;
int err;
--
1.7.11.7
^ permalink raw reply related
* [patch] enic: change sprintf() to snprintf()
From: Dan Carpenter @ 2013-01-18 7:46 UTC (permalink / raw)
To: Christian Benvenuti
Cc: Roopa Prabhu, Neel Patel, Nishank Trivedi, netdev,
kernel-janitors
These are copying data into 16 char arrays. They all specify that the
first string can't be more than 11 characters but once you add on the
"-rx-" and the NUL character there isn't space for the %d.
The first string is probably never going to be 11 characters, but if it
is then let's truncate the string instead of corrupting memory.
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
diff --git a/drivers/net/ethernet/cisco/enic/enic_main.c b/drivers/net/ethernet/cisco/enic/enic_main.c
index ebccebf..ec1a233 100644
--- a/drivers/net/ethernet/cisco/enic/enic_main.c
+++ b/drivers/net/ethernet/cisco/enic/enic_main.c
@@ -1490,7 +1490,8 @@ static int enic_request_intr(struct enic *enic)
for (i = 0; i < enic->rq_count; i++) {
intr = enic_msix_rq_intr(enic, i);
- sprintf(enic->msix[intr].devname,
+ snprintf(enic->msix[intr].devname,
+ sizeof(enic->msix[intr].devname),
"%.11s-rx-%d", netdev->name, i);
enic->msix[intr].isr = enic_isr_msix_rq;
enic->msix[intr].devid = &enic->napi[i];
@@ -1498,20 +1499,23 @@ static int enic_request_intr(struct enic *enic)
for (i = 0; i < enic->wq_count; i++) {
intr = enic_msix_wq_intr(enic, i);
- sprintf(enic->msix[intr].devname,
+ snprintf(enic->msix[intr].devname,
+ sizeof(enic->msix[intr].devname),
"%.11s-tx-%d", netdev->name, i);
enic->msix[intr].isr = enic_isr_msix_wq;
enic->msix[intr].devid = enic;
}
intr = enic_msix_err_intr(enic);
- sprintf(enic->msix[intr].devname,
+ snprintf(enic->msix[intr].devname,
+ sizeof(enic->msix[intr].devname),
"%.11s-err", netdev->name);
enic->msix[intr].isr = enic_isr_msix_err;
enic->msix[intr].devid = enic;
intr = enic_msix_notify_intr(enic);
- sprintf(enic->msix[intr].devname,
+ snprintf(enic->msix[intr].devname,
+ sizeof(enic->msix[intr].devname),
"%.11s-notify", netdev->name);
enic->msix[intr].isr = enic_isr_msix_notify;
enic->msix[intr].devid = enic;
^ permalink raw reply related
* Re: [PATCH] net: usb: initialize tmp in dm9601.c to avoid warning
From: Peter Korsgaard @ 2013-01-18 7:37 UTC (permalink / raw)
To: David Miller; +Cc: sque, netdev, msb
In-Reply-To: <20130117.175454.200184894667897570.davem@davemloft.net>
>>>>> "David" == David Miller <davem@davemloft.net> writes:
>> What warning and what compiler version are you using?
David> I wouldn't bother asking this, unless it's so that you personally
David> can reproduce this in the future.
David> The compiler cannot determine with %100 certainty that tmp is
David> initialized in all paths. Any time there are conditional
David> statements guarding setting and the use, the compiler is going
David> to err on the side of caution and say that if any of those
David> paths lead to a use without a set it's going to warn.
Sure, I just wondered as the code hasn't changed in 5+ years, and the
function that gets called is defined just above, so the compiler should
be able to figure it out.
--
Bye, Peter Korsgaard
^ permalink raw reply
* [PATCH] libertas sdio: remove CMD_FUNC_INIT call
From: Lubomir Rintel @ 2013-01-18 7:24 UTC (permalink / raw)
To: Harro Haan, Dan Williams
Cc: John W. Linville, libertas-dev, linux-wireless, netdev,
linux-kernel, Lubomir Rintel
It actually times out on a 8688 present in GuruPlug with sd8688.bin
(md5=7233401e9687f8c880da547beed4324e) firmware (that's present in
linux-firmware tree), but the adapter works fine.
For that firmware times out with libertas_uap [1] as well, though it succeeds
with sd8688_ap.bin (md5=52cd8f8296b9a7e1d3835d57416256b2) that was originally
shipped with GuruPlug. That firmware is not in linux-firmware tree and btmrvl
driver might win the race programming the 8688 with sd8688.bin anyway.
[1] https://github.com/lkundrak/linux/tree/libertas_uap
---
drivers/net/wireless/libertas/if_sdio.c | 14 --------------
1 files changed, 0 insertions(+), 14 deletions(-)
diff --git a/drivers/net/wireless/libertas/if_sdio.c b/drivers/net/wireless/libertas/if_sdio.c
index 2ecab49..3c4c555 100644
--- a/drivers/net/wireless/libertas/if_sdio.c
+++ b/drivers/net/wireless/libertas/if_sdio.c
@@ -825,20 +825,6 @@ static void if_sdio_finish_power_on(struct if_sdio_card *card)
sdio_release_host(func);
- /*
- * FUNC_INIT is required for SD8688 WLAN/BT multiple functions
- */
- if (card->model == MODEL_8688) {
- struct cmd_header cmd;
-
- memset(&cmd, 0, sizeof(cmd));
-
- lbs_deb_sdio("send function INIT command\n");
- if (__lbs_cmd(priv, CMD_FUNC_INIT, &cmd, sizeof(cmd),
- lbs_cmd_copyback, (unsigned long) &cmd))
- netdev_alert(priv->dev, "CMD_FUNC_INIT cmd failed\n");
- }
-
priv->fw_ready = 1;
wake_up(&card->pwron_waitq);
--
1.7.1
^ permalink raw reply related
* small iputils patch to remove excessive punctuation
From: Benjamin Kerensa @ 2013-01-18 7:22 UTC (permalink / raw)
To: netdev
Here is a quick iputils patch to remove unneeded punctuation from ping
flood warning in ping_common.c
Signed-off-by: Benjamin Kerensa <bkerensa@ubuntu.com>
---
ping_common.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/ping_common.c b/ping_common.c
index 7f82851..261aafc 100644
--- a/ping_common.c
+++ b/ping_common.c
@@ -608,7 +608,7 @@ void setup(int icmp_sock)
interval = 0;
if (uid && interval < MINUSERINTERVAL) {
- fprintf(stderr, "ping: cannot flood; minimal interval,
allowed for user, is %dms\n", MINUSERINTERVAL);
+ fprintf(stderr, "ping: cannot flood; minimal interval allowed
for user is %dms\n", MINUSERINTERVAL);
exit(2);
}
--
1.8.0
^ permalink raw reply related
* Re: [PATCH] net: fix wrong length of mac address
From: Dan Carpenter @ 2013-01-18 7:14 UTC (permalink / raw)
To: Amos Kong; +Cc: netdev, davem, devel
In-Reply-To: <1358475321-3406-1-git-send-email-kongjianjun@gmail.com>
You should still CC Haiyang Zhang and KY (as well as netdev). Use
./scripts/get_maintainer.pl -f drivers/net/hyperv/netvsc_drv.c
to find out where to send patches.
On Fri, Jan 18, 2013 at 10:15:21AM +0800, Amos Kong wrote:
> Replace hardcode 14 with defined ETN_ALEN.
^
Typo. It should be ETH_ALEN.
regards,
dan carpenter
^ permalink raw reply
* Re: [PATCH V5 1/2] virtio-net: fix the set affinity bug when CPU IDs are not consecutive
From: Jason Wang @ 2013-01-18 6:40 UTC (permalink / raw)
To: gaowanlong
Cc: Michael S. Tsirkin, netdev, linux-kernel, virtualization,
Eric Dumazet
In-Reply-To: <50F8EAD2.3040606@cn.fujitsu.com>
On 01/18/2013 02:25 PM, Wanlong Gao wrote:
> On 01/18/2013 02:18 PM, Jason Wang wrote:
>> On 01/18/2013 01:32 PM, Wanlong Gao wrote:
>>> As Michael mentioned, set affinity and select queue will not work very
>>> well when CPU IDs are not consecutive, this can happen with hot unplug.
>>> Fix this bug by traversal the online CPUs, and create a per cpu variable
>>> to find the mapping from CPU to the preferable virtual-queue.
>>>
>>> Cc: Rusty Russell <rusty@rustcorp.com.au>
>>> Cc: "Michael S. Tsirkin" <mst@redhat.com>
>>> Cc: Jason Wang <jasowang@redhat.com>
>>> Cc: Eric Dumazet <erdnetdev@gmail.com>
>>> Cc: virtualization@lists.linux-foundation.org
>>> Cc: netdev@vger.kernel.org
>>> Signed-off-by: Wanlong Gao <gaowanlong@cn.fujitsu.com>
>>> ---
>>> V4->V5:
>>> Add get/put_online_cpus to avoid CPUs go up and down during operations (Rusty)
>>>
>>> V3->V4:
>>> move vq_index into virtnet_info (Jason)
>>> change the mapping value when not setting affinity (Jason)
>>> address the comments about select_queue (Rusty)
>>>
>>> drivers/net/virtio_net.c | 60 +++++++++++++++++++++++++++++++++++++++---------
>>> 1 file changed, 49 insertions(+), 11 deletions(-)
>>>
>>> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
>>> index a6fcf15..440b0eb 100644
>>> --- a/drivers/net/virtio_net.c
>>> +++ b/drivers/net/virtio_net.c
>>> @@ -123,6 +123,9 @@ struct virtnet_info {
>>>
>>> /* Does the affinity hint is set for virtqueues? */
>>> bool affinity_hint_set;
>>> +
>>> + /* Per-cpu variable to show the mapping from CPU to virtqueue */
>>> + int __percpu *vq_index;
>>> };
>>>
>>> struct skb_vnet_hdr {
>>> @@ -1016,6 +1019,7 @@ static int virtnet_vlan_rx_kill_vid(struct net_device *dev, u16 vid)
>>> static void virtnet_set_affinity(struct virtnet_info *vi, bool set)
>>> {
>>> int i;
>>> + int cpu;
>>>
>>> /* In multiqueue mode, when the number of cpu is equal to the number of
>>> * queue pairs, we let the queue pairs to be private to one cpu by
>>> @@ -1029,16 +1033,29 @@ static void virtnet_set_affinity(struct virtnet_info *vi, bool set)
>>> return;
>>> }
>>>
>>> - for (i = 0; i < vi->max_queue_pairs; i++) {
>>> - int cpu = set ? i : -1;
>>> - virtqueue_set_affinity(vi->rq[i].vq, cpu);
>>> - virtqueue_set_affinity(vi->sq[i].vq, cpu);
>>> - }
>>> + if (set) {
>>> + i = 0;
>>> + for_each_online_cpu(cpu) {
>>> + virtqueue_set_affinity(vi->rq[i].vq, cpu);
>>> + virtqueue_set_affinity(vi->sq[i].vq, cpu);
>>> + *per_cpu_ptr(vi->vq_index, cpu) = i;
>>> + i++;
>>> + }
>>>
>>> - if (set)
>>> vi->affinity_hint_set = true;
>>> - else
>>> + } else {
>>> + for(i = 0; i < vi->max_queue_pairs; i++) {
>>> + virtqueue_set_affinity(vi->rq[i].vq, -1);
>>> + virtqueue_set_affinity(vi->sq[i].vq, -1);
>>> + }
>>> +
>>> + i = 0;
>>> + for_each_online_cpu(cpu)
>>> + *per_cpu_ptr(vi->vq_index, cpu) =
>>> + ++i % vi->curr_queue_pairs;
>>> +
>>> vi->affinity_hint_set = false;
>>> + }
>>> }
>>>
>>> static void virtnet_get_ringparam(struct net_device *dev,
>>> @@ -1087,7 +1104,9 @@ static int virtnet_set_channels(struct net_device *dev,
>>> netif_set_real_num_tx_queues(dev, queue_pairs);
>>> netif_set_real_num_rx_queues(dev, queue_pairs);
>>>
>>> + get_online_cpus();
>>> virtnet_set_affinity(vi, true);
>>> + put_online_cpus();
>>> }
>>>
>>> return err;
>>> @@ -1127,12 +1146,19 @@ static int virtnet_change_mtu(struct net_device *dev, int new_mtu)
>>>
>>> /* To avoid contending a lock hold by a vcpu who would exit to host, select the
>>> * txq based on the processor id.
>>> - * TODO: handle cpu hotplug.
>>> */
>>> static u16 virtnet_select_queue(struct net_device *dev, struct sk_buff *skb)
>>> {
>>> - int txq = skb_rx_queue_recorded(skb) ? skb_get_rx_queue(skb) :
>>> - smp_processor_id();
>>> + int txq;
>>> + struct virtnet_info *vi = netdev_priv(dev);
>>> +
>>> + if (skb_rx_queue_recorded(skb)) {
>>> + txq = skb_get_rx_queue(skb);
>>> + } else {
>>> + txq = *__this_cpu_ptr(vi->vq_index);
>>> + if (txq == -1)
>>> + txq = 0;
>>> + }
>>>
>>> while (unlikely(txq >= dev->real_num_tx_queues))
>>> txq -= dev->real_num_tx_queues;
>>> @@ -1248,7 +1274,9 @@ static void virtnet_del_vqs(struct virtnet_info *vi)
>>> {
>>> struct virtio_device *vdev = vi->vdev;
>>>
>>> + get_online_cpus();
>>> virtnet_set_affinity(vi, false);
>>> + put_online_cpus();
>> I think the {get|put}_online_cpus() is not necessary here when we are
>> cleaning.
> When cleaning, we also set the per-cpu variable, is avoiding hot plug
> or unplug unnecessary?
We del vqs when:
1) fail to probe the device
2) device removal
3) freeze before suspend
In both cases, the device has been stopped. And in 1 and 2 we are about
to free the device, the per-cpu won't be used any more. For 3, we will
re-initialize the virtqueues during restoring which will call
virtnet_set_affinity() which has been protected by
{get|put}_online_cpus() below.
So looks we could safely remove this.
Thanks
>
> Thanks,
> Wanlong Gao
>
>>>
>>> vdev->config->del_vqs(vdev);
>>>
>>> @@ -1371,7 +1399,10 @@ static int init_vqs(struct virtnet_info *vi)
>>> if (ret)
>>> goto err_free;
>>>
>>> + get_online_cpus();
>>> virtnet_set_affinity(vi, true);
>>> + put_online_cpus();
>>> +
>>> return 0;
>>>
>>> err_free:
>>> @@ -1453,6 +1484,10 @@ static int virtnet_probe(struct virtio_device *vdev)
>>> if (vi->stats == NULL)
>>> goto free;
>>>
>>> + vi->vq_index = alloc_percpu(int);
>>> + if (vi->vq_index == NULL)
>>> + goto free_stats;
>>> +
>>> mutex_init(&vi->config_lock);
>>> vi->config_enable = true;
>>> INIT_WORK(&vi->config_work, virtnet_config_changed_work);
>>> @@ -1476,7 +1511,7 @@ static int virtnet_probe(struct virtio_device *vdev)
>>> /* Allocate/initialize the rx/tx queues, and invoke find_vqs */
>>> err = init_vqs(vi);
>>> if (err)
>>> - goto free_stats;
>>> + goto free_index;
>>>
>>> netif_set_real_num_tx_queues(dev, 1);
>>> netif_set_real_num_rx_queues(dev, 1);
>>> @@ -1520,6 +1555,8 @@ free_recv_bufs:
>>> free_vqs:
>>> cancel_delayed_work_sync(&vi->refill);
>>> virtnet_del_vqs(vi);
>>> +free_index:
>>> + free_percpu(vi->vq_index);
>>> free_stats:
>>> free_percpu(vi->stats);
>>> free:
>>> @@ -1554,6 +1591,7 @@ static void virtnet_remove(struct virtio_device *vdev)
>>>
>>> flush_work(&vi->config_work);
>>>
>>> + free_percpu(vi->vq_index);
>>> free_percpu(vi->stats);
>>> free_netdev(vi->dev);
>>> }
>>
^ permalink raw reply
* Re: [net-next 04/14] e1000e: add support for IEEE-1588 PTP
From: Jeff Kirsher @ 2013-01-18 6:27 UTC (permalink / raw)
To: Allan, Bruce W
Cc: Stephen Hemminger, davem@davemloft.net, netdev@vger.kernel.org,
gospo@redhat.com, sassmann@redhat.com, Richard Cochran
In-Reply-To: <804857E1F29AAC47BF68C404FC60A1843ABB7F00@ORSMSX102.amr.corp.intel.com>
[-- Attachment #1: Type: text/plain, Size: 1087 bytes --]
On Thu, 2013-01-17 at 17:13 -0800, Allan, Bruce W wrote:
> > -----Original Message-----
> > From: Stephen Hemminger [mailto:stephen@networkplumber.org]
> > Sent: Thursday, January 17, 2013 7:56 AM
> > To: Kirsher, Jeffrey T
> > Cc: davem@davemloft.net; Allan, Bruce W; netdev@vger.kernel.org;
> > gospo@redhat.com; sassmann@redhat.com; Richard Cochran
> > Subject: Re: [net-next 04/14] e1000e: add support for IEEE-1588 PTP
> >
> > On Thu, 17 Jan 2013 03:35:09 -0800
> > Jeff Kirsher <jeffrey.t.kirsher@intel.com> wrote:
> >
> > > +static struct ptp_clock_info e1000e_ptp_clock_info = {
> >
> > Should be static const since it is immutable and contains function pointers.
>
> Nice catch! Thanks for the review Stephen. I'll submit a follow-on patch to change this
> if that is alright with you. Btw, did you catch that through inspection or did you use a
> static analysis tool?
>
> Bruce.
Bruce-
I have to make some other changes in the series of patches, so we can
just fix up this patch for when I re-submit the series of patches.
Cheers,
Jeff
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [PATCH V5 1/2] virtio-net: fix the set affinity bug when CPU IDs are not consecutive
From: Wanlong Gao @ 2013-01-18 6:25 UTC (permalink / raw)
To: Jason Wang
Cc: Michael S. Tsirkin, netdev, linux-kernel, virtualization,
Eric Dumazet
In-Reply-To: <50F8E93B.4090004@redhat.com>
On 01/18/2013 02:18 PM, Jason Wang wrote:
> On 01/18/2013 01:32 PM, Wanlong Gao wrote:
>> As Michael mentioned, set affinity and select queue will not work very
>> well when CPU IDs are not consecutive, this can happen with hot unplug.
>> Fix this bug by traversal the online CPUs, and create a per cpu variable
>> to find the mapping from CPU to the preferable virtual-queue.
>>
>> Cc: Rusty Russell <rusty@rustcorp.com.au>
>> Cc: "Michael S. Tsirkin" <mst@redhat.com>
>> Cc: Jason Wang <jasowang@redhat.com>
>> Cc: Eric Dumazet <erdnetdev@gmail.com>
>> Cc: virtualization@lists.linux-foundation.org
>> Cc: netdev@vger.kernel.org
>> Signed-off-by: Wanlong Gao <gaowanlong@cn.fujitsu.com>
>> ---
>> V4->V5:
>> Add get/put_online_cpus to avoid CPUs go up and down during operations (Rusty)
>>
>> V3->V4:
>> move vq_index into virtnet_info (Jason)
>> change the mapping value when not setting affinity (Jason)
>> address the comments about select_queue (Rusty)
>>
>> drivers/net/virtio_net.c | 60 +++++++++++++++++++++++++++++++++++++++---------
>> 1 file changed, 49 insertions(+), 11 deletions(-)
>>
>> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
>> index a6fcf15..440b0eb 100644
>> --- a/drivers/net/virtio_net.c
>> +++ b/drivers/net/virtio_net.c
>> @@ -123,6 +123,9 @@ struct virtnet_info {
>>
>> /* Does the affinity hint is set for virtqueues? */
>> bool affinity_hint_set;
>> +
>> + /* Per-cpu variable to show the mapping from CPU to virtqueue */
>> + int __percpu *vq_index;
>> };
>>
>> struct skb_vnet_hdr {
>> @@ -1016,6 +1019,7 @@ static int virtnet_vlan_rx_kill_vid(struct net_device *dev, u16 vid)
>> static void virtnet_set_affinity(struct virtnet_info *vi, bool set)
>> {
>> int i;
>> + int cpu;
>>
>> /* In multiqueue mode, when the number of cpu is equal to the number of
>> * queue pairs, we let the queue pairs to be private to one cpu by
>> @@ -1029,16 +1033,29 @@ static void virtnet_set_affinity(struct virtnet_info *vi, bool set)
>> return;
>> }
>>
>> - for (i = 0; i < vi->max_queue_pairs; i++) {
>> - int cpu = set ? i : -1;
>> - virtqueue_set_affinity(vi->rq[i].vq, cpu);
>> - virtqueue_set_affinity(vi->sq[i].vq, cpu);
>> - }
>> + if (set) {
>> + i = 0;
>> + for_each_online_cpu(cpu) {
>> + virtqueue_set_affinity(vi->rq[i].vq, cpu);
>> + virtqueue_set_affinity(vi->sq[i].vq, cpu);
>> + *per_cpu_ptr(vi->vq_index, cpu) = i;
>> + i++;
>> + }
>>
>> - if (set)
>> vi->affinity_hint_set = true;
>> - else
>> + } else {
>> + for(i = 0; i < vi->max_queue_pairs; i++) {
>> + virtqueue_set_affinity(vi->rq[i].vq, -1);
>> + virtqueue_set_affinity(vi->sq[i].vq, -1);
>> + }
>> +
>> + i = 0;
>> + for_each_online_cpu(cpu)
>> + *per_cpu_ptr(vi->vq_index, cpu) =
>> + ++i % vi->curr_queue_pairs;
>> +
>> vi->affinity_hint_set = false;
>> + }
>> }
>>
>> static void virtnet_get_ringparam(struct net_device *dev,
>> @@ -1087,7 +1104,9 @@ static int virtnet_set_channels(struct net_device *dev,
>> netif_set_real_num_tx_queues(dev, queue_pairs);
>> netif_set_real_num_rx_queues(dev, queue_pairs);
>>
>> + get_online_cpus();
>> virtnet_set_affinity(vi, true);
>> + put_online_cpus();
>> }
>>
>> return err;
>> @@ -1127,12 +1146,19 @@ static int virtnet_change_mtu(struct net_device *dev, int new_mtu)
>>
>> /* To avoid contending a lock hold by a vcpu who would exit to host, select the
>> * txq based on the processor id.
>> - * TODO: handle cpu hotplug.
>> */
>> static u16 virtnet_select_queue(struct net_device *dev, struct sk_buff *skb)
>> {
>> - int txq = skb_rx_queue_recorded(skb) ? skb_get_rx_queue(skb) :
>> - smp_processor_id();
>> + int txq;
>> + struct virtnet_info *vi = netdev_priv(dev);
>> +
>> + if (skb_rx_queue_recorded(skb)) {
>> + txq = skb_get_rx_queue(skb);
>> + } else {
>> + txq = *__this_cpu_ptr(vi->vq_index);
>> + if (txq == -1)
>> + txq = 0;
>> + }
>>
>> while (unlikely(txq >= dev->real_num_tx_queues))
>> txq -= dev->real_num_tx_queues;
>> @@ -1248,7 +1274,9 @@ static void virtnet_del_vqs(struct virtnet_info *vi)
>> {
>> struct virtio_device *vdev = vi->vdev;
>>
>> + get_online_cpus();
>> virtnet_set_affinity(vi, false);
>> + put_online_cpus();
>
> I think the {get|put}_online_cpus() is not necessary here when we are
> cleaning.
When cleaning, we also set the per-cpu variable, is avoiding hot plug
or unplug unnecessary?
Thanks,
Wanlong Gao
>>
>> vdev->config->del_vqs(vdev);
>>
>> @@ -1371,7 +1399,10 @@ static int init_vqs(struct virtnet_info *vi)
>> if (ret)
>> goto err_free;
>>
>> + get_online_cpus();
>> virtnet_set_affinity(vi, true);
>> + put_online_cpus();
>> +
>> return 0;
>>
>> err_free:
>> @@ -1453,6 +1484,10 @@ static int virtnet_probe(struct virtio_device *vdev)
>> if (vi->stats == NULL)
>> goto free;
>>
>> + vi->vq_index = alloc_percpu(int);
>> + if (vi->vq_index == NULL)
>> + goto free_stats;
>> +
>> mutex_init(&vi->config_lock);
>> vi->config_enable = true;
>> INIT_WORK(&vi->config_work, virtnet_config_changed_work);
>> @@ -1476,7 +1511,7 @@ static int virtnet_probe(struct virtio_device *vdev)
>> /* Allocate/initialize the rx/tx queues, and invoke find_vqs */
>> err = init_vqs(vi);
>> if (err)
>> - goto free_stats;
>> + goto free_index;
>>
>> netif_set_real_num_tx_queues(dev, 1);
>> netif_set_real_num_rx_queues(dev, 1);
>> @@ -1520,6 +1555,8 @@ free_recv_bufs:
>> free_vqs:
>> cancel_delayed_work_sync(&vi->refill);
>> virtnet_del_vqs(vi);
>> +free_index:
>> + free_percpu(vi->vq_index);
>> free_stats:
>> free_percpu(vi->stats);
>> free:
>> @@ -1554,6 +1591,7 @@ static void virtnet_remove(struct virtio_device *vdev)
>>
>> flush_work(&vi->config_work);
>>
>> + free_percpu(vi->vq_index);
>> free_percpu(vi->stats);
>> free_netdev(vi->dev);
>> }
>
>
^ permalink raw reply
* Re: [PATCH V5 2/2] virtio-net: reset virtqueue affinity when doing cpu hotplug
From: Jason Wang @ 2013-01-18 6:23 UTC (permalink / raw)
To: Wanlong Gao
Cc: Michael S. Tsirkin, netdev, linux-kernel, virtualization,
Eric Dumazet
In-Reply-To: <1358487121-7956-2-git-send-email-gaowanlong@cn.fujitsu.com>
On 01/18/2013 01:32 PM, Wanlong Gao wrote:
> Add a cpu notifier to virtio-net, so that we can reset the
> virtqueue affinity if the cpu hotplug happens. It improve
> the performance through enabling or disabling the virtqueue
> affinity after doing cpu hotplug.
>
> Cc: Rusty Russell <rusty@rustcorp.com.au>
> Cc: "Michael S. Tsirkin" <mst@redhat.com>
> Cc: Jason Wang <jasowang@redhat.com>
> Cc: Eric Dumazet <erdnetdev@gmail.com>
> Cc: virtualization@lists.linux-foundation.org
> Cc: netdev@vger.kernel.org
> Signed-off-by: Wanlong Gao <gaowanlong@cn.fujitsu.com>
> ---
> V4->V5:
> New method to deal with the cpu hotplug actions (Rusty)
>
> drivers/net/virtio_net.c | 50 ++++++++++++++++++++++++++++++++++++++++++------
> 1 file changed, 44 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index 440b0eb..061f2c5 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -26,6 +26,7 @@
> #include <linux/scatterlist.h>
> #include <linux/if_vlan.h>
> #include <linux/slab.h>
> +#include <linux/cpu.h>
>
> static int napi_weight = 128;
> module_param(napi_weight, int, 0444);
> @@ -126,6 +127,9 @@ struct virtnet_info {
>
> /* Per-cpu variable to show the mapping from CPU to virtqueue */
> int __percpu *vq_index;
> +
> + /* CPU hot plug notifier */
> + struct notifier_block nb;
> };
>
> struct skb_vnet_hdr {
> @@ -1016,7 +1020,7 @@ static int virtnet_vlan_rx_kill_vid(struct net_device *dev, u16 vid)
> return 0;
> }
>
> -static void virtnet_set_affinity(struct virtnet_info *vi, bool set)
> +static void virtnet_set_affinity(struct virtnet_info *vi, bool set, long hcpu)
> {
> int i;
> int cpu;
> @@ -1026,7 +1030,8 @@ static void virtnet_set_affinity(struct virtnet_info *vi, bool set)
> * setting the affinity hint to eliminate the contention.
> */
> if ((vi->curr_queue_pairs == 1 ||
> - vi->max_queue_pairs != num_online_cpus()) && set) {
> + vi->max_queue_pairs != num_online_cpus() - ((hcpu == -1) ? 0 : 1))
> + && set) {
> if (vi->affinity_hint_set)
> set = false;
> else
This make complex the this function. How about introduce another
function that just clean the affinity of cpu during CPU_DOWN_PREPARE.
Then we could get a simpler virtnet_set_affinity() called in CPU_DEAD
like you previous version?
> @@ -1036,6 +1041,8 @@ static void virtnet_set_affinity(struct virtnet_info *vi, bool set)
> if (set) {
> i = 0;
> for_each_online_cpu(cpu) {
> + if (cpu == hcpu)
> + continue;
> virtqueue_set_affinity(vi->rq[i].vq, cpu);
> virtqueue_set_affinity(vi->sq[i].vq, cpu);
> *per_cpu_ptr(vi->vq_index, cpu) = i;
> @@ -1050,14 +1057,36 @@ static void virtnet_set_affinity(struct virtnet_info *vi, bool set)
> }
>
> i = 0;
> - for_each_online_cpu(cpu)
> + for_each_online_cpu(cpu) {
> + if (cpu == hcpu)
> + continue;
> *per_cpu_ptr(vi->vq_index, cpu) =
> ++i % vi->curr_queue_pairs;
> + }
>
> vi->affinity_hint_set = false;
> }
> }
>
> +static int virtnet_cpu_callback(struct notifier_block *nfb,
> + unsigned long action, void *hcpu)
> +{
> + struct virtnet_info *vi = container_of(nfb, struct virtnet_info, nb);
> +
> + switch(action & ~CPU_TASKS_FROZEN) {
> + case CPU_ONLINE:
> + case CPU_DOWN_FAILED:
> + virtnet_set_affinity(vi, true, -1);
> + break;
> + case CPU_DOWN_PREPARE:
> + virtnet_set_affinity(vi, true, (long)hcpu);
> + break;
> + default:
> + break;
> + }
> + return NOTIFY_OK;
> +}
> +
> static void virtnet_get_ringparam(struct net_device *dev,
> struct ethtool_ringparam *ring)
> {
> @@ -1105,7 +1134,7 @@ static int virtnet_set_channels(struct net_device *dev,
> netif_set_real_num_rx_queues(dev, queue_pairs);
>
> get_online_cpus();
> - virtnet_set_affinity(vi, true);
> + virtnet_set_affinity(vi, true, -1);
> put_online_cpus();
> }
>
> @@ -1275,7 +1304,7 @@ static void virtnet_del_vqs(struct virtnet_info *vi)
> struct virtio_device *vdev = vi->vdev;
>
> get_online_cpus();
> - virtnet_set_affinity(vi, false);
> + virtnet_set_affinity(vi, false, -1);
> put_online_cpus();
>
> vdev->config->del_vqs(vdev);
> @@ -1400,7 +1429,7 @@ static int init_vqs(struct virtnet_info *vi)
> goto err_free;
>
> get_online_cpus();
> - virtnet_set_affinity(vi, true);
> + virtnet_set_affinity(vi, true, -1);
> put_online_cpus();
>
> return 0;
> @@ -1534,6 +1563,13 @@ static int virtnet_probe(struct virtio_device *vdev)
> }
> }
>
> + vi->nb.notifier_call = &virtnet_cpu_callback;
> + err = register_hotcpu_notifier(&vi->nb);
> + if (err) {
> + pr_debug("virtio_net: registering cpu notifier failed\n");
> + goto free_recv_bufs;
> + }
> +
> /* Assume link up if device can't report link status,
> otherwise get link status from config. */
> if (virtio_has_feature(vi->vdev, VIRTIO_NET_F_STATUS)) {
> @@ -1580,6 +1616,8 @@ static void virtnet_remove(struct virtio_device *vdev)
> {
> struct virtnet_info *vi = vdev->priv;
>
> + unregister_hotcpu_notifier(&vi->nb);
> +
> /* Prevent config work handler from accessing the device. */
> mutex_lock(&vi->config_lock);
> vi->config_enable = false;
^ permalink raw reply
* Re: [PATCH V5 1/2] virtio-net: fix the set affinity bug when CPU IDs are not consecutive
From: Jason Wang @ 2013-01-18 6:18 UTC (permalink / raw)
To: Wanlong Gao
Cc: Michael S. Tsirkin, netdev, linux-kernel, virtualization,
Eric Dumazet
In-Reply-To: <1358487121-7956-1-git-send-email-gaowanlong@cn.fujitsu.com>
On 01/18/2013 01:32 PM, Wanlong Gao wrote:
> As Michael mentioned, set affinity and select queue will not work very
> well when CPU IDs are not consecutive, this can happen with hot unplug.
> Fix this bug by traversal the online CPUs, and create a per cpu variable
> to find the mapping from CPU to the preferable virtual-queue.
>
> Cc: Rusty Russell <rusty@rustcorp.com.au>
> Cc: "Michael S. Tsirkin" <mst@redhat.com>
> Cc: Jason Wang <jasowang@redhat.com>
> Cc: Eric Dumazet <erdnetdev@gmail.com>
> Cc: virtualization@lists.linux-foundation.org
> Cc: netdev@vger.kernel.org
> Signed-off-by: Wanlong Gao <gaowanlong@cn.fujitsu.com>
> ---
> V4->V5:
> Add get/put_online_cpus to avoid CPUs go up and down during operations (Rusty)
>
> V3->V4:
> move vq_index into virtnet_info (Jason)
> change the mapping value when not setting affinity (Jason)
> address the comments about select_queue (Rusty)
>
> drivers/net/virtio_net.c | 60 +++++++++++++++++++++++++++++++++++++++---------
> 1 file changed, 49 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index a6fcf15..440b0eb 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -123,6 +123,9 @@ struct virtnet_info {
>
> /* Does the affinity hint is set for virtqueues? */
> bool affinity_hint_set;
> +
> + /* Per-cpu variable to show the mapping from CPU to virtqueue */
> + int __percpu *vq_index;
> };
>
> struct skb_vnet_hdr {
> @@ -1016,6 +1019,7 @@ static int virtnet_vlan_rx_kill_vid(struct net_device *dev, u16 vid)
> static void virtnet_set_affinity(struct virtnet_info *vi, bool set)
> {
> int i;
> + int cpu;
>
> /* In multiqueue mode, when the number of cpu is equal to the number of
> * queue pairs, we let the queue pairs to be private to one cpu by
> @@ -1029,16 +1033,29 @@ static void virtnet_set_affinity(struct virtnet_info *vi, bool set)
> return;
> }
>
> - for (i = 0; i < vi->max_queue_pairs; i++) {
> - int cpu = set ? i : -1;
> - virtqueue_set_affinity(vi->rq[i].vq, cpu);
> - virtqueue_set_affinity(vi->sq[i].vq, cpu);
> - }
> + if (set) {
> + i = 0;
> + for_each_online_cpu(cpu) {
> + virtqueue_set_affinity(vi->rq[i].vq, cpu);
> + virtqueue_set_affinity(vi->sq[i].vq, cpu);
> + *per_cpu_ptr(vi->vq_index, cpu) = i;
> + i++;
> + }
>
> - if (set)
> vi->affinity_hint_set = true;
> - else
> + } else {
> + for(i = 0; i < vi->max_queue_pairs; i++) {
> + virtqueue_set_affinity(vi->rq[i].vq, -1);
> + virtqueue_set_affinity(vi->sq[i].vq, -1);
> + }
> +
> + i = 0;
> + for_each_online_cpu(cpu)
> + *per_cpu_ptr(vi->vq_index, cpu) =
> + ++i % vi->curr_queue_pairs;
> +
> vi->affinity_hint_set = false;
> + }
> }
>
> static void virtnet_get_ringparam(struct net_device *dev,
> @@ -1087,7 +1104,9 @@ static int virtnet_set_channels(struct net_device *dev,
> netif_set_real_num_tx_queues(dev, queue_pairs);
> netif_set_real_num_rx_queues(dev, queue_pairs);
>
> + get_online_cpus();
> virtnet_set_affinity(vi, true);
> + put_online_cpus();
> }
>
> return err;
> @@ -1127,12 +1146,19 @@ static int virtnet_change_mtu(struct net_device *dev, int new_mtu)
>
> /* To avoid contending a lock hold by a vcpu who would exit to host, select the
> * txq based on the processor id.
> - * TODO: handle cpu hotplug.
> */
> static u16 virtnet_select_queue(struct net_device *dev, struct sk_buff *skb)
> {
> - int txq = skb_rx_queue_recorded(skb) ? skb_get_rx_queue(skb) :
> - smp_processor_id();
> + int txq;
> + struct virtnet_info *vi = netdev_priv(dev);
> +
> + if (skb_rx_queue_recorded(skb)) {
> + txq = skb_get_rx_queue(skb);
> + } else {
> + txq = *__this_cpu_ptr(vi->vq_index);
> + if (txq == -1)
> + txq = 0;
> + }
>
> while (unlikely(txq >= dev->real_num_tx_queues))
> txq -= dev->real_num_tx_queues;
> @@ -1248,7 +1274,9 @@ static void virtnet_del_vqs(struct virtnet_info *vi)
> {
> struct virtio_device *vdev = vi->vdev;
>
> + get_online_cpus();
> virtnet_set_affinity(vi, false);
> + put_online_cpus();
I think the {get|put}_online_cpus() is not necessary here when we are
cleaning.
>
> vdev->config->del_vqs(vdev);
>
> @@ -1371,7 +1399,10 @@ static int init_vqs(struct virtnet_info *vi)
> if (ret)
> goto err_free;
>
> + get_online_cpus();
> virtnet_set_affinity(vi, true);
> + put_online_cpus();
> +
> return 0;
>
> err_free:
> @@ -1453,6 +1484,10 @@ static int virtnet_probe(struct virtio_device *vdev)
> if (vi->stats == NULL)
> goto free;
>
> + vi->vq_index = alloc_percpu(int);
> + if (vi->vq_index == NULL)
> + goto free_stats;
> +
> mutex_init(&vi->config_lock);
> vi->config_enable = true;
> INIT_WORK(&vi->config_work, virtnet_config_changed_work);
> @@ -1476,7 +1511,7 @@ static int virtnet_probe(struct virtio_device *vdev)
> /* Allocate/initialize the rx/tx queues, and invoke find_vqs */
> err = init_vqs(vi);
> if (err)
> - goto free_stats;
> + goto free_index;
>
> netif_set_real_num_tx_queues(dev, 1);
> netif_set_real_num_rx_queues(dev, 1);
> @@ -1520,6 +1555,8 @@ free_recv_bufs:
> free_vqs:
> cancel_delayed_work_sync(&vi->refill);
> virtnet_del_vqs(vi);
> +free_index:
> + free_percpu(vi->vq_index);
> free_stats:
> free_percpu(vi->stats);
> free:
> @@ -1554,6 +1591,7 @@ static void virtnet_remove(struct virtio_device *vdev)
>
> flush_work(&vi->config_work);
>
> + free_percpu(vi->vq_index);
> free_percpu(vi->stats);
> free_netdev(vi->dev);
> }
^ permalink raw reply
* [PATCH V5 2/2] virtio-net: reset virtqueue affinity when doing cpu hotplug
From: Wanlong Gao @ 2013-01-18 5:32 UTC (permalink / raw)
To: linux-kernel; +Cc: Michael S. Tsirkin, netdev, virtualization, Eric Dumazet
In-Reply-To: <1358487121-7956-1-git-send-email-gaowanlong@cn.fujitsu.com>
Add a cpu notifier to virtio-net, so that we can reset the
virtqueue affinity if the cpu hotplug happens. It improve
the performance through enabling or disabling the virtqueue
affinity after doing cpu hotplug.
Cc: Rusty Russell <rusty@rustcorp.com.au>
Cc: "Michael S. Tsirkin" <mst@redhat.com>
Cc: Jason Wang <jasowang@redhat.com>
Cc: Eric Dumazet <erdnetdev@gmail.com>
Cc: virtualization@lists.linux-foundation.org
Cc: netdev@vger.kernel.org
Signed-off-by: Wanlong Gao <gaowanlong@cn.fujitsu.com>
---
V4->V5:
New method to deal with the cpu hotplug actions (Rusty)
drivers/net/virtio_net.c | 50 ++++++++++++++++++++++++++++++++++++++++++------
1 file changed, 44 insertions(+), 6 deletions(-)
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 440b0eb..061f2c5 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -26,6 +26,7 @@
#include <linux/scatterlist.h>
#include <linux/if_vlan.h>
#include <linux/slab.h>
+#include <linux/cpu.h>
static int napi_weight = 128;
module_param(napi_weight, int, 0444);
@@ -126,6 +127,9 @@ struct virtnet_info {
/* Per-cpu variable to show the mapping from CPU to virtqueue */
int __percpu *vq_index;
+
+ /* CPU hot plug notifier */
+ struct notifier_block nb;
};
struct skb_vnet_hdr {
@@ -1016,7 +1020,7 @@ static int virtnet_vlan_rx_kill_vid(struct net_device *dev, u16 vid)
return 0;
}
-static void virtnet_set_affinity(struct virtnet_info *vi, bool set)
+static void virtnet_set_affinity(struct virtnet_info *vi, bool set, long hcpu)
{
int i;
int cpu;
@@ -1026,7 +1030,8 @@ static void virtnet_set_affinity(struct virtnet_info *vi, bool set)
* setting the affinity hint to eliminate the contention.
*/
if ((vi->curr_queue_pairs == 1 ||
- vi->max_queue_pairs != num_online_cpus()) && set) {
+ vi->max_queue_pairs != num_online_cpus() - ((hcpu == -1) ? 0 : 1))
+ && set) {
if (vi->affinity_hint_set)
set = false;
else
@@ -1036,6 +1041,8 @@ static void virtnet_set_affinity(struct virtnet_info *vi, bool set)
if (set) {
i = 0;
for_each_online_cpu(cpu) {
+ if (cpu == hcpu)
+ continue;
virtqueue_set_affinity(vi->rq[i].vq, cpu);
virtqueue_set_affinity(vi->sq[i].vq, cpu);
*per_cpu_ptr(vi->vq_index, cpu) = i;
@@ -1050,14 +1057,36 @@ static void virtnet_set_affinity(struct virtnet_info *vi, bool set)
}
i = 0;
- for_each_online_cpu(cpu)
+ for_each_online_cpu(cpu) {
+ if (cpu == hcpu)
+ continue;
*per_cpu_ptr(vi->vq_index, cpu) =
++i % vi->curr_queue_pairs;
+ }
vi->affinity_hint_set = false;
}
}
+static int virtnet_cpu_callback(struct notifier_block *nfb,
+ unsigned long action, void *hcpu)
+{
+ struct virtnet_info *vi = container_of(nfb, struct virtnet_info, nb);
+
+ switch(action & ~CPU_TASKS_FROZEN) {
+ case CPU_ONLINE:
+ case CPU_DOWN_FAILED:
+ virtnet_set_affinity(vi, true, -1);
+ break;
+ case CPU_DOWN_PREPARE:
+ virtnet_set_affinity(vi, true, (long)hcpu);
+ break;
+ default:
+ break;
+ }
+ return NOTIFY_OK;
+}
+
static void virtnet_get_ringparam(struct net_device *dev,
struct ethtool_ringparam *ring)
{
@@ -1105,7 +1134,7 @@ static int virtnet_set_channels(struct net_device *dev,
netif_set_real_num_rx_queues(dev, queue_pairs);
get_online_cpus();
- virtnet_set_affinity(vi, true);
+ virtnet_set_affinity(vi, true, -1);
put_online_cpus();
}
@@ -1275,7 +1304,7 @@ static void virtnet_del_vqs(struct virtnet_info *vi)
struct virtio_device *vdev = vi->vdev;
get_online_cpus();
- virtnet_set_affinity(vi, false);
+ virtnet_set_affinity(vi, false, -1);
put_online_cpus();
vdev->config->del_vqs(vdev);
@@ -1400,7 +1429,7 @@ static int init_vqs(struct virtnet_info *vi)
goto err_free;
get_online_cpus();
- virtnet_set_affinity(vi, true);
+ virtnet_set_affinity(vi, true, -1);
put_online_cpus();
return 0;
@@ -1534,6 +1563,13 @@ static int virtnet_probe(struct virtio_device *vdev)
}
}
+ vi->nb.notifier_call = &virtnet_cpu_callback;
+ err = register_hotcpu_notifier(&vi->nb);
+ if (err) {
+ pr_debug("virtio_net: registering cpu notifier failed\n");
+ goto free_recv_bufs;
+ }
+
/* Assume link up if device can't report link status,
otherwise get link status from config. */
if (virtio_has_feature(vi->vdev, VIRTIO_NET_F_STATUS)) {
@@ -1580,6 +1616,8 @@ static void virtnet_remove(struct virtio_device *vdev)
{
struct virtnet_info *vi = vdev->priv;
+ unregister_hotcpu_notifier(&vi->nb);
+
/* Prevent config work handler from accessing the device. */
mutex_lock(&vi->config_lock);
vi->config_enable = false;
--
1.8.1
^ permalink raw reply related
* [PATCH V5 1/2] virtio-net: fix the set affinity bug when CPU IDs are not consecutive
From: Wanlong Gao @ 2013-01-18 5:32 UTC (permalink / raw)
To: linux-kernel; +Cc: Michael S. Tsirkin, netdev, virtualization, Eric Dumazet
As Michael mentioned, set affinity and select queue will not work very
well when CPU IDs are not consecutive, this can happen with hot unplug.
Fix this bug by traversal the online CPUs, and create a per cpu variable
to find the mapping from CPU to the preferable virtual-queue.
Cc: Rusty Russell <rusty@rustcorp.com.au>
Cc: "Michael S. Tsirkin" <mst@redhat.com>
Cc: Jason Wang <jasowang@redhat.com>
Cc: Eric Dumazet <erdnetdev@gmail.com>
Cc: virtualization@lists.linux-foundation.org
Cc: netdev@vger.kernel.org
Signed-off-by: Wanlong Gao <gaowanlong@cn.fujitsu.com>
---
V4->V5:
Add get/put_online_cpus to avoid CPUs go up and down during operations (Rusty)
V3->V4:
move vq_index into virtnet_info (Jason)
change the mapping value when not setting affinity (Jason)
address the comments about select_queue (Rusty)
drivers/net/virtio_net.c | 60 +++++++++++++++++++++++++++++++++++++++---------
1 file changed, 49 insertions(+), 11 deletions(-)
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index a6fcf15..440b0eb 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -123,6 +123,9 @@ struct virtnet_info {
/* Does the affinity hint is set for virtqueues? */
bool affinity_hint_set;
+
+ /* Per-cpu variable to show the mapping from CPU to virtqueue */
+ int __percpu *vq_index;
};
struct skb_vnet_hdr {
@@ -1016,6 +1019,7 @@ static int virtnet_vlan_rx_kill_vid(struct net_device *dev, u16 vid)
static void virtnet_set_affinity(struct virtnet_info *vi, bool set)
{
int i;
+ int cpu;
/* In multiqueue mode, when the number of cpu is equal to the number of
* queue pairs, we let the queue pairs to be private to one cpu by
@@ -1029,16 +1033,29 @@ static void virtnet_set_affinity(struct virtnet_info *vi, bool set)
return;
}
- for (i = 0; i < vi->max_queue_pairs; i++) {
- int cpu = set ? i : -1;
- virtqueue_set_affinity(vi->rq[i].vq, cpu);
- virtqueue_set_affinity(vi->sq[i].vq, cpu);
- }
+ if (set) {
+ i = 0;
+ for_each_online_cpu(cpu) {
+ virtqueue_set_affinity(vi->rq[i].vq, cpu);
+ virtqueue_set_affinity(vi->sq[i].vq, cpu);
+ *per_cpu_ptr(vi->vq_index, cpu) = i;
+ i++;
+ }
- if (set)
vi->affinity_hint_set = true;
- else
+ } else {
+ for(i = 0; i < vi->max_queue_pairs; i++) {
+ virtqueue_set_affinity(vi->rq[i].vq, -1);
+ virtqueue_set_affinity(vi->sq[i].vq, -1);
+ }
+
+ i = 0;
+ for_each_online_cpu(cpu)
+ *per_cpu_ptr(vi->vq_index, cpu) =
+ ++i % vi->curr_queue_pairs;
+
vi->affinity_hint_set = false;
+ }
}
static void virtnet_get_ringparam(struct net_device *dev,
@@ -1087,7 +1104,9 @@ static int virtnet_set_channels(struct net_device *dev,
netif_set_real_num_tx_queues(dev, queue_pairs);
netif_set_real_num_rx_queues(dev, queue_pairs);
+ get_online_cpus();
virtnet_set_affinity(vi, true);
+ put_online_cpus();
}
return err;
@@ -1127,12 +1146,19 @@ static int virtnet_change_mtu(struct net_device *dev, int new_mtu)
/* To avoid contending a lock hold by a vcpu who would exit to host, select the
* txq based on the processor id.
- * TODO: handle cpu hotplug.
*/
static u16 virtnet_select_queue(struct net_device *dev, struct sk_buff *skb)
{
- int txq = skb_rx_queue_recorded(skb) ? skb_get_rx_queue(skb) :
- smp_processor_id();
+ int txq;
+ struct virtnet_info *vi = netdev_priv(dev);
+
+ if (skb_rx_queue_recorded(skb)) {
+ txq = skb_get_rx_queue(skb);
+ } else {
+ txq = *__this_cpu_ptr(vi->vq_index);
+ if (txq == -1)
+ txq = 0;
+ }
while (unlikely(txq >= dev->real_num_tx_queues))
txq -= dev->real_num_tx_queues;
@@ -1248,7 +1274,9 @@ static void virtnet_del_vqs(struct virtnet_info *vi)
{
struct virtio_device *vdev = vi->vdev;
+ get_online_cpus();
virtnet_set_affinity(vi, false);
+ put_online_cpus();
vdev->config->del_vqs(vdev);
@@ -1371,7 +1399,10 @@ static int init_vqs(struct virtnet_info *vi)
if (ret)
goto err_free;
+ get_online_cpus();
virtnet_set_affinity(vi, true);
+ put_online_cpus();
+
return 0;
err_free:
@@ -1453,6 +1484,10 @@ static int virtnet_probe(struct virtio_device *vdev)
if (vi->stats == NULL)
goto free;
+ vi->vq_index = alloc_percpu(int);
+ if (vi->vq_index == NULL)
+ goto free_stats;
+
mutex_init(&vi->config_lock);
vi->config_enable = true;
INIT_WORK(&vi->config_work, virtnet_config_changed_work);
@@ -1476,7 +1511,7 @@ static int virtnet_probe(struct virtio_device *vdev)
/* Allocate/initialize the rx/tx queues, and invoke find_vqs */
err = init_vqs(vi);
if (err)
- goto free_stats;
+ goto free_index;
netif_set_real_num_tx_queues(dev, 1);
netif_set_real_num_rx_queues(dev, 1);
@@ -1520,6 +1555,8 @@ free_recv_bufs:
free_vqs:
cancel_delayed_work_sync(&vi->refill);
virtnet_del_vqs(vi);
+free_index:
+ free_percpu(vi->vq_index);
free_stats:
free_percpu(vi->stats);
free:
@@ -1554,6 +1591,7 @@ static void virtnet_remove(struct virtio_device *vdev)
flush_work(&vi->config_work);
+ free_percpu(vi->vq_index);
free_percpu(vi->stats);
free_netdev(vi->dev);
}
--
1.8.1
^ permalink raw reply related
* Re: Redefinition of struct in6_addr in <netinet/in.h> and <linux/in6.h>
From: Mike Frysinger @ 2013-01-18 5:27 UTC (permalink / raw)
To: David Miller
Cc: libc-alpha, bhutchings, yoshfuji, amwang, tmb, eblake, netdev,
linux-kernel, libvirt-list, tgraf, schwab, carlos
In-Reply-To: <20130117.235524.2010232687612037902.davem@davemloft.net>
[-- Attachment #1: Type: Text/Plain, Size: 1683 bytes --]
On Thursday 17 January 2013 23:55:24 David Miller wrote:
> From: Mike Frysinger <vapier@gentoo.org>
> > the kernel already exports many types with a __kernel_ prefix. i changed
> > the kernel headers in Gentoo for a few releases (2.6.28 - 2.6.34) to do
> > the same thing to pretty much all the networking headers. a few
> > packages broke, but the number was low, and trivial to fix (a sed would
> > do it most of the time).
> >
> > it's also trivial for userland packages to detect that they need to do
> > this sort of thing in a local header by using linux/version.h and a set
> > of defines to redirect the new structure name back to the old one.
> >
> > would be a lot cleaner to just break it and be done.
>
> We are not at liberty to break something that has legitimately
> compiled successfully for two decades.
i doubt there are code bases that have compiled successfully for that long
w/out being changed. in fact, the kernel user headers (until recently) have
been a volatile mess requiring constant care & feeding of user programs to
keep compiling. if anything, this would simply be maintaining the status quo
for a bit longer.
> One thing you certainly don't have to be is happy about this header
> file situation, but you cannot use that dissatisfaction as
> justification to make the situation worse by breaking the build for
> people outside of the world you directly control.
i think you're making wrong assumptions about my position on the topic. i
never said i was against cleaning up the kernel user headers (and in fact, you
can find many commits from me doing exactly that over the years in the kernel).
-mike
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [PATCHv2] am33xx: cpsw: default to ethernet hwaddr from efuse if not defined in dt
From: Mugunthan V N @ 2013-01-18 5:14 UTC (permalink / raw)
To: Peter Korsgaard
Cc: michal.bachraty-Re5JQEeQqe8AvxtiuMwx3w,
michal.bachraty-6oiIBCxl0MMjD8S081q9vkEOCMrvLtNR,
netdev-u79uwXL29TY76Z2rM5mHXA,
devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
linux-omap-u79uwXL29TY76Z2rM5mHXA,
richardcochran-Re5JQEeQqe8AvxtiuMwx3w
In-Reply-To: <1358461134-13452-1-git-send-email-jacmet-OfajU3CKLf1/SzgSGea1oA@public.gmane.org>
On 1/18/2013 3:48 AM, Peter Korsgaard wrote:
> When booting with CONFIG_ARM_APPENDED_DTB (either because of using an old
> U-Boot, not wanting the hassle of 2 files or when using Falcon fast boot
> mode in U-Boot), nothing updates the ethernet hwaddr specified for the
> CPSW slaves, causing the driver to use a random hwaddr, which is some times
> troublesome.
>
> The am33xx has unique ethernet hwaddrs programmed in the efuse, so it makes
> more sense to default to these rather than random ones. Add a fixup step
> which adds mac-address dt properties using the efuse addresses if the DTB
> didn't contain valid ones.
>
> Signed-off-by: Peter Korsgaard <jacmet-OfajU3CKLf1/SzgSGea1oA@public.gmane.org>
>
This implementation looks fine.
Acked-by: Mugunthan V N <mugunthanvnm-l0cyMroinI0@public.gmane.org>
Regards
Mugunthan V N
^ permalink raw reply
* Re: Redefinition of struct in6_addr in <netinet/in.h> and <linux/in6.h>
From: David Miller @ 2013-01-18 4:55 UTC (permalink / raw)
To: vapier
Cc: libc-alpha, bhutchings, yoshfuji, amwang, tmb, eblake, netdev,
linux-kernel, libvirt-list, tgraf, schwab, carlos
In-Reply-To: <201301172314.34193.vapier@gentoo.org>
From: Mike Frysinger <vapier@gentoo.org>
Date: Thu, 17 Jan 2013 23:14:31 -0500
> the kernel already exports many types with a __kernel_ prefix. i changed the
> kernel headers in Gentoo for a few releases (2.6.28 - 2.6.34) to do the same
> thing to pretty much all the networking headers. a few packages broke, but
> the number was low, and trivial to fix (a sed would do it most of the time).
>
> it's also trivial for userland packages to detect that they need to do this
> sort of thing in a local header by using linux/version.h and a set of defines
> to redirect the new structure name back to the old one.
>
> would be a lot cleaner to just break it and be done.
We are not at liberty to break something that has legitimately
compiled successfully for two decades.
Your __kernel_ prefix idea breaks compilation just as equally.
One thing you certainly don't have to be is happy about this header
file situation, but you cannot use that dissatisfaction as
justification to make the situation worse by breaking the build for
people outside of the world you directly control.
^ permalink raw reply
* Re: Redefinition of struct in6_addr in <netinet/in.h> and <linux/in6.h>
From: Mike Frysinger @ 2013-01-18 4:34 UTC (permalink / raw)
To: Carlos O'Donell
Cc: David Miller, libc-alpha, bhutchings, yoshfuji, amwang, tmb,
eblake, netdev, linux-kernel, libvirt-list, tgraf, schwab
In-Reply-To: <CAE2sS1jMhnRcoo1oe7wQ+SP=40u9mkps6vXua8TQQ4Tbf2qKOQ@mail.gmail.com>
[-- Attachment #1: Type: Text/Plain, Size: 1587 bytes --]
On Thursday 17 January 2013 23:22:26 Carlos O'Donell wrote:
> On Thu, Jan 17, 2013 at 11:20 PM, Mike Frysinger <vapier@gentoo.org> wrote:
> > On Wednesday 16 January 2013 22:15:38 David Miller wrote:
> >> From: Carlos O'Donell <carlos@systemhalted.org>
> >> Date: Wed, 16 Jan 2013 21:15:03 -0500
> >>
> >> > +/* If a glibc-based userspace has already included in.h, then we will
> >> > not + * define in6_addr (nor the defines), sockaddr_in6, or ipv6_mreq.
> >> > The + * ABI used by the kernel and by glibc match exactly. Neither the
> >> > kernel + * nor glibc should break this ABI without coordination.
> >> > + */
> >> > +#ifndef _NETINET_IN_H
> >> > +
> >>
> >> I think we should shoot for a non-glibc-centric solution.
> >>
> >> I can't imagine that other libc's won't have the same exact problem
> >> with their netinet/in.h conflicting with the kernel's, redefining
> >> structures like in6_addr, that we'd want to provide a protection
> >> scheme for here as well.
> >
> > yes, the kernel's use of __GLIBC__ in exported headers has already caused
> > problems in the past. fortunately, it's been reduced down to just one
> > case now (stat.h). let's not balloon it back up.
>
> I also see coda.h has grown a __GLIBC__ usage.
that file is just a pile of cruft :). it's something that'd be rejected by
today's kernel standard as it's full of OS shim code. "#ifdef DOS" ? comeon!
fortunately, coda is pretty uncommon, so this issue doesn't bite most people,
and i've never bothered with it. the same cannot be said of stat.h.
-mike
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: Redefinition of struct in6_addr in <netinet/in.h> and <linux/in6.h>
From: Carlos O'Donell @ 2013-01-18 4:22 UTC (permalink / raw)
To: Mike Frysinger
Cc: David Miller, libc-alpha, bhutchings, yoshfuji, amwang, tmb,
eblake, netdev, linux-kernel, libvirt-list, tgraf, schwab
In-Reply-To: <201301172320.19905.vapier@gentoo.org>
On Thu, Jan 17, 2013 at 11:20 PM, Mike Frysinger <vapier@gentoo.org> wrote:
> On Wednesday 16 January 2013 22:15:38 David Miller wrote:
>> From: Carlos O'Donell <carlos@systemhalted.org>
>> Date: Wed, 16 Jan 2013 21:15:03 -0500
>>
>> > +/* If a glibc-based userspace has already included in.h, then we will
>> > not + * define in6_addr (nor the defines), sockaddr_in6, or ipv6_mreq.
>> > The + * ABI used by the kernel and by glibc match exactly. Neither the
>> > kernel + * nor glibc should break this ABI without coordination.
>> > + */
>> > +#ifndef _NETINET_IN_H
>> > +
>>
>> I think we should shoot for a non-glibc-centric solution.
>>
>> I can't imagine that other libc's won't have the same exact problem
>> with their netinet/in.h conflicting with the kernel's, redefining
>> structures like in6_addr, that we'd want to provide a protection
>> scheme for here as well.
>
> yes, the kernel's use of __GLIBC__ in exported headers has already caused
> problems in the past. fortunately, it's been reduced down to just one case
> now (stat.h). let's not balloon it back up.
> -mike
I also see coda.h has grown a __GLIBC__ usage.
In the next revision of the patch I created a single libc-compat.h header
which encompasses the logic for any libc that wants to coordinate with
the kernel headers.
It's simple enough to move all of the __GLIBC__ uses into libc-compat.h,
then you control userspace libc coordination from one file.
Cheers,
Carlos.
^ permalink raw reply
* Re: Redefinition of struct in6_addr in <netinet/in.h> and <linux/in6.h>
From: Mike Frysinger @ 2013-01-18 4:20 UTC (permalink / raw)
To: David Miller
Cc: carlos, libc-alpha, bhutchings, yoshfuji, amwang, tmb, eblake,
netdev, linux-kernel, libvirt-list, tgraf, schwab
In-Reply-To: <20130116.221538.1756411165313441486.davem@davemloft.net>
[-- Attachment #1: Type: Text/Plain, Size: 1012 bytes --]
On Wednesday 16 January 2013 22:15:38 David Miller wrote:
> From: Carlos O'Donell <carlos@systemhalted.org>
> Date: Wed, 16 Jan 2013 21:15:03 -0500
>
> > +/* If a glibc-based userspace has already included in.h, then we will
> > not + * define in6_addr (nor the defines), sockaddr_in6, or ipv6_mreq.
> > The + * ABI used by the kernel and by glibc match exactly. Neither the
> > kernel + * nor glibc should break this ABI without coordination.
> > + */
> > +#ifndef _NETINET_IN_H
> > +
>
> I think we should shoot for a non-glibc-centric solution.
>
> I can't imagine that other libc's won't have the same exact problem
> with their netinet/in.h conflicting with the kernel's, redefining
> structures like in6_addr, that we'd want to provide a protection
> scheme for here as well.
yes, the kernel's use of __GLIBC__ in exported headers has already caused
problems in the past. fortunately, it's been reduced down to just one case
now (stat.h). let's not balloon it back up.
-mike
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ 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