Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH] Fix IXP4xx coherent allocations
From: Krzysztof Halasa @ 2013-04-01 20:17 UTC (permalink / raw)
  To: Russell King - ARM Linux
  Cc: Ben Hutchings, linux-arm-kernel, netdev, David Miller,
	linux-kernel, c.aeschlimann
In-Reply-To: <20130330153135.GC17995@n2100.arm.linux.org.uk>

Russell King - ARM Linux <linux@arm.linux.org.uk> writes:

> Right, so, the answer is - yes you are talking about platform devices,
> and the reason that these aren't already set is because if you grep for
> ixp4xx_eth or ixp4xx_hss in arch/arm/mach-ixp4xx, you'll notice that
> _none_ of the device declarations set either of the DMA masks at all.
> They don't even set the dev->dma_mask pointer.  That is why the masks
> are zero.  For a device which does DMA, that is wrong.

Well, that's new to me. Please tell me how it should be done.
Should I simply add in platform code (on all platforms):

+++ b/arch/arm/mach-ixp4xx/XXX.c
@@ -380,10 +380,12 @@ static struct platform_device device_eth_tab[] = {
         .name           = "ixp4xx_eth",
         .id         = IXP4XX_ETH_NPEB,
         .dev.platform_data  = eth_plat,
+        .dev.coherent_dma_mask  = DMA_BIT_MASK(32),
     }, {
         .name           = "ixp4xx_eth",
         .id         = IXP4XX_ETH_NPEC,
         .dev.platform_data  = eth_plat + 1,
+        .dev.coherent_dma_mask  = DMA_BIT_MASK(32),
     }
 };

This alone isn't going to work, the problem is coherent DMA mask in
net_dev->dev and not in platform_device. So what do I do in the network
drivers? Copy the masks from platform_device to the actual newly created
net_dev->dev?

Should I use the parent device (net_dev->dev.parent which is the
platform_device) as the argument to the allocator? This would probably
work though I'm not sure of its correctness.

BTW the platform code shouldn't IMHO need to declare the masks as they
aren't platform-specific. They are driver-specific (defined by CPU
design) and setting them in the driver seems clean to me (unlike the
platform code).

Several other device drivers simply set their masks directly. Is it the
recommended way?

> If you look at the PCI code, it pre-initializes the DMA mask to be 4GiB:
> drivers/pci/probe.c:        dev->dev.coherent_dma_mask = 0xffffffffull;
>
> And that is what is missing from the IXP4xx platform code.
>
> However, avoiding the call to dma_set_coherent_mask() from within the
> driver also seems to be questionable as it bypasses the "check if the
> mask is possible" part of the DMA API.

I thought about this for a second but the situation is the mask
is guaranteed to be valid (these are on-chip devices).
-- 
Krzysztof Halasa

^ permalink raw reply

* Re: [PATCH]  net-sysfs: make flags symmetrical
From: Ben Greear @ 2013-04-01 20:16 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: David Miller, netdev
In-Reply-To: <20130401115315.1f8e4213@nehalam.linuxnetplumber.net>

On 04/01/2013 11:53 AM, Stephen Hemminger wrote:
> The flags reported by sysfs are the raw kernel flags, not the version
> exported to user space. This leads to the unsymmetrical behaviour that
> read != write. An example of this is when a device is part of a
> bridge.  The PROMISC flag returned from sysfs will not be the same as
> other API's.
>
> The reason this patch deserves wider discussion is someone might be
> depending on sysfs to read raw kernel flags.

I am depending on this feature.  There is no other way I know
of to determine if an interface is actually currently acting
PROMISC or not.

Please don't 'fix' this.

Thanks,
Ben

-- 
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com

^ permalink raw reply

* Re: [Patch net-next v1 3/4] vxlan: add ipv6 support
From: Stephen Hemminger @ 2013-04-01 20:14 UTC (permalink / raw)
  To: Cong Wang; +Cc: netdev, David S. Miller
In-Reply-To: <1364708625-29063-3-git-send-email-amwang@redhat.com>

On Sun, 31 Mar 2013 13:43:44 +0800
Cong Wang <amwang@redhat.com> wrote:

>  	/* Need to drop RTNL to call multicast leave */
>  	rtnl_unlock();
> -	lock_sock(sk);
> -	err = ip_mc_leave_group(sk, &mreq);
> +	if (vxlan->gaddr.proto == htons(ETH_P_IP)) {
> +		lock_sock(sk);
> +		err = ip_mc_leave_group(sk, &mreq);
> +	} else {
> +#if IS_ENABLED(CONFIG_IPV6)
> +		lock_sock(sk);
> +		err = ipv6_sock_mc_drop(sk, vxlan->link, &vxlan->gaddr.ip6);
> +#endif
> +	}
>  	release_sock(sk);

Since both v4 and v6 need socket locked why not?

	rtnl_unlock();
	lock_sock(sk);
	if (vxlan->gaddr.proto == htons(ETH_P_IP)) 
		err = ip_mc_leave_group(sk, &mreq);
if IS_ENABLED(CONFIG_IPV6)
	else
		err = ipv6_sock_mc_drop(sk, vxlan->link, &vxlan->gaddr.ip6);
#endif

  	release_sock(sk);

^ permalink raw reply

* Re: [Patch net-next v1 3/4] vxlan: add ipv6 support
From: David Miller @ 2013-04-01 20:05 UTC (permalink / raw)
  To: dlstevens; +Cc: amwang, netdev, netdev-owner, stephen
In-Reply-To: <OFCC7D4E76.D0FD3154-ON85257B40.00659778-85257B40.006E30A9@us.ibm.com>

From: David Stevens <dlstevens@us.ibm.com>
Date: Mon, 1 Apr 2013 16:03:38 -0400

> My suggestion doesn't apply to anything at user level, or visible
> to user level through an include file, kernel or glibc.

In that case I rescind my objection.

^ permalink raw reply

* Re: [Patch net-next v1 3/4] vxlan: add ipv6 support
From: David Stevens @ 2013-04-01 20:03 UTC (permalink / raw)
  To: David Miller; +Cc: amwang, netdev, netdev-owner, stephen
In-Reply-To: <20130401.141548.1477224368912481541.davem@davemloft.net>

netdev-owner@vger.kernel.org wrote on 04/01/2013 02:15:48 PM:
Date: Mon, 1 Apr 2013 14:05:37 -0400
> 
> >         I guess, but in this case, I'm not saying it's like a sockaddr 

> > with
> > device-specific requirements. Rather, I'm saying it's exactly a 
sockaddr--
> > it is either a sockaddr_in or a sockaddr_in6 and a family field to say
> > which.
> 
> in6_addr, which is what sockaddr_in6 is composed of, is precisely the
> problematic type that we want to avoid to elide the double definition
> error.
> 
> http://www.spinics.net/linux/fedora/libvir/msg70921.html
> 
> It's really not safe to use at the moment.
> 
> We're not creating more instances of this mess, and that decision
> is final.

Dave,

I think we're talking about two different things. I'm suggesting
we should use sockaddrs within the kernel module code as the type for
what is now a v4-only address but would be one of v4 or v6 after
adding v6 support.

Within the kernel module code, without changing any header files,
I'm suggesting we use the existing kernel sockaddr definitions and the
sa_family to know which address type we have, instead of defining
a new type local to vxlan.c with the same purpose and most of the
fields as a sockaddr for that very same kernel-only private data.

An additional comment is that those fields missing in the private type
and present in the sockaddr types are useful in cases that apply
to these destination addresses, too; "vxlan_ip" ought to have all the
fields a sockaddr_in6 or sockaddr_in does and if it did, then it
ought to use sockaddr/sockaddr_in/sockaddr_in6 for this kernel-only data.

My suggestion doesn't apply to anything at user level, or visible
to user level through an include file, kernel or glibc.

                                                        +-DLS

^ permalink raw reply

* Re: [PATCH] of_net.h: Provide dummy functions if OF_NET is not configured
From: Rob Herring @ 2013-04-01 19:14 UTC (permalink / raw)
  To: Guenter Roeck
  Cc: David Daney, netdev-u79uwXL29TY76Z2rM5mHXA,
	devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Rob Herring, David S. Miller
In-Reply-To: <20130401190100.GA1042-0h96xk9xTtrk1uMJSBkQmQ@public.gmane.org>

On 04/01/2013 02:01 PM, Guenter Roeck wrote:
> On Mon, Apr 01, 2013 at 01:44:24PM -0500, Rob Herring wrote:
>> On 04/01/2013 01:19 PM, Guenter Roeck wrote:
>>> of_get_mac_address() and of_get_phy_mode() are only provided if OF_NET
>>> is configured. While most callers check for the define, not all do, and those
>>> who do require #ifdef around the code. For those who don't, the missing check
>>> can result in errors such as
>>
>> How about removing the ifdef from those callers?
>>
> That would be the next step, after/if this one is accepted.
> If not, it doesn't make sense to waste my time.

Assuming that is done:

Acked-by: Rob Herring <rob.herring-bsGFqQB8/DxBDgjK7y7TUQ@public.gmane.org>

Presumably with the follow-on patches, this can go in thru the net tree.

Rob

> Thanks,
> Guenter
> 
>> Rob
>>
>>>
>>> arch/powerpc/sysdev/tsi108_dev.c:107:3: error: implicit declaration of
>>> 	function 'of_get_mac_address' [-Werror=implicit-function-declaration]
>>> arch/powerpc/sysdev/mv64x60_dev.c:253:2: error: implicit declaration of
>>> 	function 'of_get_mac_address' [-Werror=implicit-function-declaration]
>>>
>>> Provide dummy function declarations if OF_NET is not configured. This is safe
>>> because all callers do check the return values. If desired, at least some of
>>> the #ifdefs in the code can subsequently be removed.
>>>
>>> Cc: David Daney <david.daney-YGCgFSpz5w/QT0dZR+AlfA@public.gmane.org>
>>> Signed-off-by: Guenter Roeck <linux-0h96xk9xTtrk1uMJSBkQmQ@public.gmane.org>
>>> ---
>>>  include/linux/of_net.h |   10 ++++++++++
>>>  1 file changed, 10 insertions(+)
>>>
>>> diff --git a/include/linux/of_net.h b/include/linux/of_net.h
>>> index f474641..61bf53b 100644
>>> --- a/include/linux/of_net.h
>>> +++ b/include/linux/of_net.h
>>> @@ -11,6 +11,16 @@
>>>  #include <linux/of.h>
>>>  extern const int of_get_phy_mode(struct device_node *np);
>>>  extern const void *of_get_mac_address(struct device_node *np);
>>> +#else
>>> +static inline const int of_get_phy_mode(struct device_node *np)
>>> +{
>>> +	return -ENODEV;
>>> +}
>>> +
>>> +static inline const void *of_get_mac_address(struct device_node *np)
>>> +{
>>> +	return NULL;
>>> +}
>>>  #endif
>>>  
>>>  #endif /* __LINUX_OF_NET_H */
>>>
>>
>>

^ permalink raw reply

* Re: [PATCH] of_net.h: Provide dummy functions if OF_NET is not configured
From: Guenter Roeck @ 2013-04-01 19:01 UTC (permalink / raw)
  To: Rob Herring
  Cc: linux-kernel, devicetree-discuss, netdev, David S. Miller,
	Rob Herring, David Daney
In-Reply-To: <5159D588.5070603@gmail.com>

On Mon, Apr 01, 2013 at 01:44:24PM -0500, Rob Herring wrote:
> On 04/01/2013 01:19 PM, Guenter Roeck wrote:
> > of_get_mac_address() and of_get_phy_mode() are only provided if OF_NET
> > is configured. While most callers check for the define, not all do, and those
> > who do require #ifdef around the code. For those who don't, the missing check
> > can result in errors such as
> 
> How about removing the ifdef from those callers?
> 
That would be the next step, after/if this one is accepted.
If not, it doesn't make sense to waste my time.

Thanks,
Guenter

> Rob
> 
> > 
> > arch/powerpc/sysdev/tsi108_dev.c:107:3: error: implicit declaration of
> > 	function 'of_get_mac_address' [-Werror=implicit-function-declaration]
> > arch/powerpc/sysdev/mv64x60_dev.c:253:2: error: implicit declaration of
> > 	function 'of_get_mac_address' [-Werror=implicit-function-declaration]
> > 
> > Provide dummy function declarations if OF_NET is not configured. This is safe
> > because all callers do check the return values. If desired, at least some of
> > the #ifdefs in the code can subsequently be removed.
> > 
> > Cc: David Daney <david.daney@cavium.com>
> > Signed-off-by: Guenter Roeck <linux@roeck-us.net>
> > ---
> >  include/linux/of_net.h |   10 ++++++++++
> >  1 file changed, 10 insertions(+)
> > 
> > diff --git a/include/linux/of_net.h b/include/linux/of_net.h
> > index f474641..61bf53b 100644
> > --- a/include/linux/of_net.h
> > +++ b/include/linux/of_net.h
> > @@ -11,6 +11,16 @@
> >  #include <linux/of.h>
> >  extern const int of_get_phy_mode(struct device_node *np);
> >  extern const void *of_get_mac_address(struct device_node *np);
> > +#else
> > +static inline const int of_get_phy_mode(struct device_node *np)
> > +{
> > +	return -ENODEV;
> > +}
> > +
> > +static inline const void *of_get_mac_address(struct device_node *np)
> > +{
> > +	return NULL;
> > +}
> >  #endif
> >  
> >  #endif /* __LINUX_OF_NET_H */
> > 
> 
> 

^ permalink raw reply

* [PATCH]  net-sysfs: make flags symmetrical
From: Stephen Hemminger @ 2013-04-01 18:53 UTC (permalink / raw)
  To: David Miller; +Cc: netdev

The flags reported by sysfs are the raw kernel flags, not the version
exported to user space. This leads to the unsymmetrical behaviour that
read != write. An example of this is when a device is part of a
bridge.  The PROMISC flag returned from sysfs will not be the same as
other API's.

The reason this patch deserves wider discussion is someone might be
depending on sysfs to read raw kernel flags.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>


--- a/net/core/net-sysfs.c	2013-03-21 14:17:08.740354291 -0700
+++ b/net/core/net-sysfs.c	2013-04-01 11:47:46.949728288 -0700
@@ -252,7 +252,19 @@ static ssize_t store_mtu(struct device *
 	return netdev_store(dev, attr, buf, len, change_mtu);
 }
 
-NETDEVICE_SHOW(flags, fmt_hex);
+static ssize_t show_flags(struct device *dev,
+			  struct device_attribute *attr, char *buf)
+{
+	struct net_device *ndev = to_net_dev(dev);
+	ssize_t ret = -EINVAL;
+
+	read_lock(&dev_base_lock);
+	if (dev_isalive(ndev))
+		ret = sprintf(buf, fmt_hex, dev_get_flags(ndev));
+	read_unlock(&dev_base_lock);
+
+	return ret;
+}
 
 static int change_flags(struct net_device *net, unsigned long new_flags)
 {

^ permalink raw reply

* Re: [PATCH] of_net.h: Provide dummy functions if OF_NET is not configured
From: Rob Herring @ 2013-04-01 18:44 UTC (permalink / raw)
  To: Guenter Roeck
  Cc: linux-kernel, devicetree-discuss, netdev, David S. Miller,
	Rob Herring, David Daney
In-Reply-To: <1364840349-10696-1-git-send-email-linux@roeck-us.net>

On 04/01/2013 01:19 PM, Guenter Roeck wrote:
> of_get_mac_address() and of_get_phy_mode() are only provided if OF_NET
> is configured. While most callers check for the define, not all do, and those
> who do require #ifdef around the code. For those who don't, the missing check
> can result in errors such as

How about removing the ifdef from those callers?

Rob

> 
> arch/powerpc/sysdev/tsi108_dev.c:107:3: error: implicit declaration of
> 	function 'of_get_mac_address' [-Werror=implicit-function-declaration]
> arch/powerpc/sysdev/mv64x60_dev.c:253:2: error: implicit declaration of
> 	function 'of_get_mac_address' [-Werror=implicit-function-declaration]
> 
> Provide dummy function declarations if OF_NET is not configured. This is safe
> because all callers do check the return values. If desired, at least some of
> the #ifdefs in the code can subsequently be removed.
> 
> Cc: David Daney <david.daney@cavium.com>
> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
> ---
>  include/linux/of_net.h |   10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/include/linux/of_net.h b/include/linux/of_net.h
> index f474641..61bf53b 100644
> --- a/include/linux/of_net.h
> +++ b/include/linux/of_net.h
> @@ -11,6 +11,16 @@
>  #include <linux/of.h>
>  extern const int of_get_phy_mode(struct device_node *np);
>  extern const void *of_get_mac_address(struct device_node *np);
> +#else
> +static inline const int of_get_phy_mode(struct device_node *np)
> +{
> +	return -ENODEV;
> +}
> +
> +static inline const void *of_get_mac_address(struct device_node *np)
> +{
> +	return NULL;
> +}
>  #endif
>  
>  #endif /* __LINUX_OF_NET_H */
> 

^ permalink raw reply

* [PATCH] VSOCK: Handle changes to the VMCI context ID.
From: Reilly Grant @ 2013-04-01 18:41 UTC (permalink / raw)
  To: netdev; +Cc: davem, linux-kernel, pv-drivers, Reilly Grant

The VMCI context ID of a virtual machine may change at any time. There
is a VMCI event which signals this but datagrams may be processed before
this is handled. It is therefore necessary to be flexible about the
destination context ID of any datagrams received. (It can be assumed to
be correct because it is provided by the hypervisor.) The context ID on
existing sockets should be updated to reflect how the hypervisor is
currently referring to the system.

Signed-off-by: Reilly Grant <grantr@vmware.com>
Acked-by: Andy King <acking@vmware.com>
---
 net/vmw_vsock/af_vsock.c       |  6 +++---
 net/vmw_vsock/vmci_transport.c | 31 ++++++++++++++++++++-----------
 net/vmw_vsock/vsock_addr.c     | 10 ----------
 net/vmw_vsock/vsock_addr.h     |  2 --
 4 files changed, 23 insertions(+), 26 deletions(-)

diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c
index ca511c4..d8079da 100644
--- a/net/vmw_vsock/af_vsock.c
+++ b/net/vmw_vsock/af_vsock.c
@@ -207,7 +207,7 @@ static struct sock *__vsock_find_bound_socket(struct sockaddr_vm *addr)
 	struct vsock_sock *vsk;
 
 	list_for_each_entry(vsk, vsock_bound_sockets(addr), bound_table)
-		if (vsock_addr_equals_addr_any(addr, &vsk->local_addr))
+		if (addr->svm_port == vsk->local_addr.svm_port)
 			return sk_vsock(vsk);
 
 	return NULL;
@@ -220,8 +220,8 @@ static struct sock *__vsock_find_connected_socket(struct sockaddr_vm *src,
 
 	list_for_each_entry(vsk, vsock_connected_sockets(src, dst),
 			    connected_table) {
-		if (vsock_addr_equals_addr(src, &vsk->remote_addr)
-		    && vsock_addr_equals_addr(dst, &vsk->local_addr)) {
+		if (vsock_addr_equals_addr(src, &vsk->remote_addr) &&
+		    dst->svm_port == vsk->local_addr.svm_port) {
 			return sk_vsock(vsk);
 		}
 	}
diff --git a/net/vmw_vsock/vmci_transport.c b/net/vmw_vsock/vmci_transport.c
index faf81b8..9caa91c 100644
--- a/net/vmw_vsock/vmci_transport.c
+++ b/net/vmw_vsock/vmci_transport.c
@@ -472,19 +472,16 @@ static struct sock *vmci_transport_get_pending(
 	struct vsock_sock *vlistener;
 	struct vsock_sock *vpending;
 	struct sock *pending;
+	struct sockaddr_vm src;
+
+	vsock_addr_init(&src, pkt->dg.src.context, pkt->src_port);
 
 	vlistener = vsock_sk(listener);
 
 	list_for_each_entry(vpending, &vlistener->pending_links,
 			    pending_links) {
-		struct sockaddr_vm src;
-		struct sockaddr_vm dst;
-
-		vsock_addr_init(&src, pkt->dg.src.context, pkt->src_port);
-		vsock_addr_init(&dst, pkt->dg.dst.context, pkt->dst_port);
-
 		if (vsock_addr_equals_addr(&src, &vpending->remote_addr) &&
-		    vsock_addr_equals_addr(&dst, &vpending->local_addr)) {
+		    pkt->dst_port == vpending->local_addr.svm_port) {
 			pending = sk_vsock(vpending);
 			sock_hold(pending);
 			goto found;
@@ -749,10 +746,15 @@ static int vmci_transport_recv_stream_cb(void *data, struct vmci_datagram *dg)
 	 */
 	bh_lock_sock(sk);
 
-	if (!sock_owned_by_user(sk) && sk->sk_state == SS_CONNECTED)
-		vmci_trans(vsk)->notify_ops->handle_notify_pkt(
-				sk, pkt, true, &dst, &src,
-				&bh_process_pkt);
+	if (!sock_owned_by_user(sk)) {
+		/* The local context ID may be out of date, update it. */
+		vsk->local_addr.svm_cid = dst.svm_cid;
+
+		if (sk->sk_state == SS_CONNECTED)
+			vmci_trans(vsk)->notify_ops->handle_notify_pkt(
+					sk, pkt, true, &dst, &src,
+					&bh_process_pkt);
+	}
 
 	bh_unlock_sock(sk);
 
@@ -912,6 +914,9 @@ static void vmci_transport_recv_pkt_work(struct work_struct *work)
 
 	lock_sock(sk);
 
+	/* The local context ID may be out of date. */
+	vsock_sk(sk)->local_addr.svm_cid = pkt->dg.dst.context;
+
 	switch (sk->sk_state) {
 	case SS_LISTEN:
 		vmci_transport_recv_listen(sk, pkt);
@@ -968,6 +973,10 @@ static int vmci_transport_recv_listen(struct sock *sk,
 	pending = vmci_transport_get_pending(sk, pkt);
 	if (pending) {
 		lock_sock(pending);
+
+		/* The local context ID may be out of date. */
+		vsock_sk(pending)->local_addr.svm_cid = pkt->dg.dst.context;
+
 		switch (pending->sk_state) {
 		case SS_CONNECTING:
 			err = vmci_transport_recv_connecting_server(sk,
diff --git a/net/vmw_vsock/vsock_addr.c b/net/vmw_vsock/vsock_addr.c
index b7df1ae..ec2611b 100644
--- a/net/vmw_vsock/vsock_addr.c
+++ b/net/vmw_vsock/vsock_addr.c
@@ -64,16 +64,6 @@ bool vsock_addr_equals_addr(const struct sockaddr_vm *addr,
 }
 EXPORT_SYMBOL_GPL(vsock_addr_equals_addr);
 
-bool vsock_addr_equals_addr_any(const struct sockaddr_vm *addr,
-				const struct sockaddr_vm *other)
-{
-	return (addr->svm_cid == VMADDR_CID_ANY ||
-		other->svm_cid == VMADDR_CID_ANY ||
-		addr->svm_cid == other->svm_cid) &&
-	       addr->svm_port == other->svm_port;
-}
-EXPORT_SYMBOL_GPL(vsock_addr_equals_addr_any);
-
 int vsock_addr_cast(const struct sockaddr *addr,
 		    size_t len, struct sockaddr_vm **out_addr)
 {
diff --git a/net/vmw_vsock/vsock_addr.h b/net/vmw_vsock/vsock_addr.h
index cdfbcef..9ccd531 100644
--- a/net/vmw_vsock/vsock_addr.h
+++ b/net/vmw_vsock/vsock_addr.h
@@ -24,8 +24,6 @@ bool vsock_addr_bound(const struct sockaddr_vm *addr);
 void vsock_addr_unbind(struct sockaddr_vm *addr);
 bool vsock_addr_equals_addr(const struct sockaddr_vm *addr,
 			    const struct sockaddr_vm *other);
-bool vsock_addr_equals_addr_any(const struct sockaddr_vm *addr,
-				const struct sockaddr_vm *other);
 int vsock_addr_cast(const struct sockaddr *addr, size_t len,
 		    struct sockaddr_vm **out_addr);
 
-- 
1.8.1.5

^ permalink raw reply related

* Re: [PATCH] core: fix the use of this_cpu_ptr
From: Christoph Lameter @ 2013-04-01 18:15 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: RongQing Li, Shan Wei, netdev
In-Reply-To: <1364833887.5113.161.camel@edumazet-glaptop>

On Mon, 1 Apr 2013, Eric Dumazet wrote:

> On Mon, 2013-04-01 at 15:21 +0000, Christoph Lameter wrote:
> > On Fri, 29 Mar 2013, RongQing Li wrote:
>
> > > flush_tasklet is not a percpu var, it is a member of percpu var.
> >
> > Well then it would be best to use this_cpu_read() instead of this_cpu_ptr.
> > It also will generate better code.
>
> I believe we already had this discussion in the past.
>
> flush_tasklet is a structure, and we need its address, not read its
> content.
>
> You can not use this_cpu_read() to get its address, and following
> code is fine.
>
> tasklet = &this_cpu_ptr(info->cache->percpu)->flush_tasklet;

that is confusing..

tasklet = this_cpu_ptr(&info->cache->percpu->flushtasklet)

this_cpu_ptr performs an address relocation. The address then is the one
of flushtasklet.

> Similar to this code in mm/page_alloc.c
>
> pcp = &this_cpu_ptr(zone->pageset)->pcp;

Yeah thats my (early) code using these features.

	pcp = this_cpu_ptr(&zone->pageset->pcp)

I need to do a writeup on this one.

^ permalink raw reply

* [PATCH] of_net.h: Provide dummy functions if OF_NET is not configured
From: Guenter Roeck @ 2013-04-01 18:19 UTC (permalink / raw)
  To: linux-kernel, devicetree-discuss, netdev
  Cc: Grant Likely, Rob Herring, David S. Miller, Guenter Roeck,
	David Daney

of_get_mac_address() and of_get_phy_mode() are only provided if OF_NET
is configured. While most callers check for the define, not all do, and those
who do require #ifdef around the code. For those who don't, the missing check
can result in errors such as

arch/powerpc/sysdev/tsi108_dev.c:107:3: error: implicit declaration of
	function 'of_get_mac_address' [-Werror=implicit-function-declaration]
arch/powerpc/sysdev/mv64x60_dev.c:253:2: error: implicit declaration of
	function 'of_get_mac_address' [-Werror=implicit-function-declaration]

Provide dummy function declarations if OF_NET is not configured. This is safe
because all callers do check the return values. If desired, at least some of
the #ifdefs in the code can subsequently be removed.

Cc: David Daney <david.daney@cavium.com>
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
 include/linux/of_net.h |   10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/include/linux/of_net.h b/include/linux/of_net.h
index f474641..61bf53b 100644
--- a/include/linux/of_net.h
+++ b/include/linux/of_net.h
@@ -11,6 +11,16 @@
 #include <linux/of.h>
 extern const int of_get_phy_mode(struct device_node *np);
 extern const void *of_get_mac_address(struct device_node *np);
+#else
+static inline const int of_get_phy_mode(struct device_node *np)
+{
+	return -ENODEV;
+}
+
+static inline const void *of_get_mac_address(struct device_node *np)
+{
+	return NULL;
+}
 #endif
 
 #endif /* __LINUX_OF_NET_H */
-- 
1.7.9.7

^ permalink raw reply related

* Re: [Patch net-next v1 3/4] vxlan: add ipv6 support
From: David Miller @ 2013-04-01 18:15 UTC (permalink / raw)
  To: dlstevens; +Cc: amwang, netdev, netdev-owner, stephen
In-Reply-To: <OF1E30BC31.571FBD44-ON85257B40.005F5AF2-85257B40.0063627F@us.ibm.com>

From: David Stevens <dlstevens@us.ibm.com>
Date: Mon, 1 Apr 2013 14:05:37 -0400

>         I guess, but in this case, I'm not saying it's like a sockaddr 
> with
> device-specific requirements. Rather, I'm saying it's exactly a sockaddr--
> it is either a sockaddr_in or a sockaddr_in6 and a family field to say
> which.

in6_addr, which is what sockaddr_in6 is composed of, is precisely the
problematic type that we want to avoid to elide the double definition
error.

http://www.spinics.net/linux/fedora/libvir/msg70921.html

It's really not safe to use at the moment.

We're not creating more instances of this mess, and that decision
is final.

^ permalink raw reply

* Re: [PATCH 3.0-longterm] bonding: ARP packet fix for balance-ALB (mode=6)
From: Ben Hutchings @ 2013-04-01 18:14 UTC (permalink / raw)
  To: Matthew O'Connor; +Cc: netdev
In-Reply-To: <CAK8PzVnNfLSnvxY83z3Em3gtJM4zWM0Xd4P3CWy3xt=7feJgMA@mail.gmail.com>

[-- Attachment #1: Type: text/plain, Size: 1474 bytes --]

On Mon, 2013-04-01 at 12:36 -0400, Matthew O'Connor wrote:
> Do not modify or load balance ARP packets passing through balance-alb
> mode (wherein the ARP did not originate locally, and arrived via a bridge).
> 
> This patch is backported from the upstream commit:
>     From 567b871e503316b0927e54a3d7c86d50b722d955 Mon Sep 17 00:00:00 2001
>     Subject: [PATCH] bonding: rlb mode of bond should not alter ARP
> originating via bridge
> 
> Signed-off-by:  Matthew O'Connor <liquidhorse@gmail.com>
[...]

These patches are tab-damaged and word-wrapped.  See
Documentation/email-clients.txt

You should also use the conventional format for backported patches.
Keep the original commit message, but above it add:

    From: original author <address@domain>

    commit 567b871e503316b0927e54a3d7c86d50b722d955 upstream.

The commit reference could also be written in David Miller's favoured
format:

    [ Upstream commit 567b871e503316b0927e54a3d7c86d50b722d955 ]

If you have made substantial changes then explain them at the end of the
commit message with clear attribution to yourself.  You could add some
explanation below the '---' of why this is important to include in a
stable update.

Lastly, this should be cc'd to all the addresses on the original commit
(From, Cc, and Signed-off-by).

Ben.

-- 
Ben Hutchings
DNRC Motto:  I can please only one person per day.
Today is not your day.  Tomorrow isn't looking good either.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

^ permalink raw reply

* Re: [Patch net-next v1 3/4] vxlan: add ipv6 support
From: David Stevens @ 2013-04-01 18:05 UTC (permalink / raw)
  To: David Miller; +Cc: amwang, netdev, netdev-owner, stephen
In-Reply-To: <20130401.130223.1527520446382659798.davem@davemloft.net>

David Miller <davem@davemloft.net> wrote on 04/01/2013 01:02:23 PM:

> 
> People avoid using sockaddr because it gets defined both in the kernel
> exported userland headers and the native libc ones with no easy
> protection between the two to avoid getting a double definition error.
> 
> Once you start including kernel exported headers for things like these
> network device specific interfaces, you potentially run into that issue.
> 
> Therefore I'd rather the subsystem define their own unique type both
> to avoid this double definition problem and to allow easy extention
> later.

        I guess, but in this case, I'm not saying it's like a sockaddr 
with
device-specific requirements. Rather, I'm saying it's exactly a sockaddr--
it is either a sockaddr_in or a sockaddr_in6 and a family field to say
which.
        As is, any user/kernel include file conflicts (in the "ip" 
command,
presumably) are still present because he's using in6_addr, another 
structure
both in user and kernel space.
        The primary multiple include issue here would be in the "ip" 
command,
presumably, but sockaddrs in particular have to agree between user and
kernel space already and both appear with "ip".
        This patch also has issues due to NOT copying other fields in the
sockaddr_in6 structure (scope_id and port).

        Personally, I don't think it's too difficult to make correct code
using sockaddr/sockaddr_in/sockaddr_in6 here, but even with a new type,
the code within vxlan.c could (and I argue should) use something like:

       union {
               struct sockaddr_in       vip_un_sin;
               struct sockaddr_in6      vip_un_sin6;
               struct sockaddr          vip_un_sa;
       } vip_sun;

#define vip_sa  vip_sun.vip_un_sa
#define vip_sin vip_sun.vip_un_sin
#define vip_sin6        vip_sun.vip_un.sin6

and then code like:
switch (vip->vip_sa.sa_family) {
case AF_INET:
        vip->vip_sin.sin_addr.s_addr = blah blah
        break;
case AF_INET6:
        vip->vip_sin6.sin6_addr = blah blah
        break;
...
}

...or whatever appropriate to the context and family.


                                                        +-DLS

^ permalink raw reply

* attach multiple programs to tun/tap interface
From: Prashant Batra @ 2013-04-01 17:12 UTC (permalink / raw)
  To: netdev, linux-newbie

Hi,

I have a basic query on tun/tap driver design. Is it possible to attach 
multiple user-space programs to the same tun/tap interface. I have not 
set the interface as EXCLUSIVE,
but still on trying to do ioctl with cmd as “TUNSETIFF” from the second 
user space program, I get “Device busy”.

Seeing through the driver code, it looks like it does not allow multiple 
user-programs to attach to the same interface, but is this valid, and 
please let me know why this requirement was taken.

Will you please help me in understanding this?

Thanks,
Prashant

^ permalink raw reply

* Re: [Patch net-next v1 3/4] vxlan: add ipv6 support
From: David Miller @ 2013-04-01 17:02 UTC (permalink / raw)
  To: dlstevens; +Cc: amwang, netdev, netdev-owner, stephen
In-Reply-To: <OF5EB046E4.6295C3A5-ON85257B40.00537077-85257B40.00542577@us.ibm.com>

From: David Stevens <dlstevens@us.ibm.com>
Date: Mon, 1 Apr 2013 11:19:10 -0400

> netdev-owner@vger.kernel.org wrote on 03/31/2013 01:43:44 AM:
>  
>> +struct vxlan_ip {
>> +   union {
>> +      __be32  ip4;
>> +#if IS_ENABLED(CONFIG_IPV6)
>> +      struct in6_addr ip6;
>> +#endif
>> +   };
>> +   __be16          proto;
>> +};
>> +
> 
>         This looks suspiciously like a sockaddr. sockaddr_storage is
> much bigger than you need, but you could just make it a sockaddr_in6
> and cast it to sockaddr_in when needed, make it sockaddr_in6 and use
> V4_MAPPED addresses for v4, or make it a union of sockaddr_in and
> sockaddr_in6, or have a buffer the size of sockaddr_in6 and use it
> as a sockaddr to determine the family, then go from there.
>         I think anything along those lines is better than a new variant
> with the same functionality of sockaddr that isn't an overlay of a
> sockaddr.

People avoid using sockaddr because it gets defined both in the kernel
exported userland headers and the native libc ones with no easy
protection between the two to avoid getting a double definition error.

Once you start including kernel exported headers for things like these
network device specific interfaces, you potentially run into that issue.

Therefore I'd rather the subsystem define their own unique type both
to avoid this double definition problem and to allow easy extention
later.

^ permalink raw reply

* Re: [PATCH 01/34] net: add skb_dst_set_noref_force
From: David Miller @ 2013-04-01 16:57 UTC (permalink / raw)
  To: pablo; +Cc: horms, lvs-devel, netdev, netfilter-devel, wensong, ja
In-Reply-To: <20130401120644.GA4882@localhost>

From: Pablo Neira Ayuso <pablo@netfilter.org>
Date: Mon, 1 Apr 2013 14:06:44 +0200

> Hi Simon,
> 
> On Fri, Mar 29, 2013 at 01:11:18PM +0900, Simon Horman wrote:
>> From: Julian Anastasov <ja@ssi.bg>
>> 
>> Rename skb_dst_set_noref to __skb_dst_set_noref and
>> add force flag as suggested by David Miller. The new wrapper
>> skb_dst_set_noref_force will force dst entries that are not
>> cached to be attached as skb dst without taking reference
>> as long as provided dst is reclaimed after RCU grace period.
>> 
>> Signed-off-by: Julian Anastasov <ja@ssi.bg>
>> Signed-off by: Hans Schillstrom <hans@schillstrom.com>
>> Signed-off-by: Simon Horman <horms@verge.net.au>
> 
> I think was acked by David, right?

It was:

Acked-by: David S. Miller <davem@davemloft.net>

^ permalink raw reply

* Fwd: [PATCH 3.4-longterm] bonding: ARP packet fix for balance-ALB (mode=6)
From: Matthew O'Connor @ 2013-04-01 16:40 UTC (permalink / raw)
  To: netdev

Do not modify or load balance ARP packets passing through balance-alb
mode (wherein the ARP did not originate locally, and arrived via a bridge).

This patch is backported from the upstream commit:
    From 567b871e503316b0927e54a3d7c86d50b722d955 Mon Sep 17 00:00:00 2001
    Subject: [PATCH] bonding: rlb mode of bond should not alter ARP
originating via bridge

Signed-off-by:  Matthew O'Connor <liquidhorse@gmail.com>

---
diff -uprN linux-3.4.28/drivers/net/bonding/bond_alb.c
linux-3.4.28-patched/drivers/net/bonding/bond_alb.c
--- linux-3.4.28/drivers/net/bonding/bond_alb.c    2013-01-27
23:51:45.000000000 -0500
+++ linux-3.4.28-patched/drivers/net/bonding/bond_alb.c    2013-01-30
15:37:25.121708311 -0500
@@ -704,6 +704,12 @@ static struct slave *rlb_arp_xmit(struct
     struct arp_pkt *arp = arp_pkt(skb);
     struct slave *tx_slave = NULL;

+    /* Don't modify or load balance ARPs that do not originate locally
+     * (e.g.,arrive via a bridge).
+     */
+    if (!bond_slave_has_mac(bond, arp->mac_src))
+        return NULL;
+
     if (arp->op_code == htons(ARPOP_REPLY)) {
         /* the arp must be sent on the selected
         * rx channel
diff -uprN linux-3.4.28/drivers/net/bonding/bonding.h
linux-3.4.28-patched/drivers/net/bonding/bonding.h
--- linux-3.4.28/drivers/net/bonding/bonding.h    2013-01-27
23:51:45.000000000 -0500
+++ linux-3.4.28-patched/drivers/net/bonding/bonding.h    2013-01-30
15:37:25.121708311 -0500
@@ -18,6 +18,7 @@
 #include <linux/timer.h>
 #include <linux/proc_fs.h>
 #include <linux/if_bonding.h>
+#include <linux/etherdevice.h>
 #include <linux/cpumask.h>
 #include <linux/in6.h>
 #include <linux/netpoll.h>
@@ -450,6 +451,18 @@ static inline void bond_destroy_proc_dir
 }
 #endif

+static inline struct slave *bond_slave_has_mac(struct bonding *bond,
+                           const u8 *mac)
+{
+    int i = 0;
+    struct slave *tmp;
+
+    bond_for_each_slave(bond, tmp, i)
+        if (ether_addr_equal_64bits(mac, tmp->dev->dev_addr))
+            return tmp;
+
+    return NULL;
+}

 /* exported from bond_main.c */
 extern int bond_net_id;
diff -uprN linux-3.4.28/include/linux/etherdevice.h
linux-3.4.28-patched/include/linux/etherdevice.h
--- linux-3.4.28/include/linux/etherdevice.h    2013-01-27
23:51:45.000000000 -0500
+++ linux-3.4.28-patched/include/linux/etherdevice.h    2013-01-30
15:37:25.121708311 -0500
@@ -277,4 +277,37 @@ static inline unsigned long compare_ethe
 #endif
 }

+/**
+ * ether_addr_equal_64bits - Compare two Ethernet addresses
+ * @addr1: Pointer to an array of 8 bytes
+ * @addr2: Pointer to an other array of 8 bytes
+ *
+ * Compare two Ethernet addresses, returns true if equal, false otherwise.
+ *
+ * The function doesn't need any conditional branches and possibly uses
+ * word memory accesses on CPU allowing cheap unaligned memory reads.
+ * arrays = { byte1, byte2, byte3, byte4, byte5, byte6, pad1, pad2 }
+ *
+ * Please note that alignment of addr1 & addr2 are only guaranteed to
be 16 bits.
+ */
+
+static inline bool ether_addr_equal_64bits(const u8 addr1[6+2],
+                                           const u8 addr2[6+2])
+{
+#ifdef CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS
+        unsigned long fold = ((*(unsigned long *)addr1) ^
+                              (*(unsigned long *)addr2));
+
+        if (sizeof(fold) == 8)
+                return zap_last_2bytes(fold) == 0;
+
+        fold |= zap_last_2bytes((*(unsigned long *)(addr1 + 4)) ^
+                                (*(unsigned long *)(addr2 + 4)));
+        return fold == 0;
+#else
+        return ether_addr_equal(addr1, addr2);
+#endif
+}
+
+
 #endif    /* _LINUX_ETHERDEVICE_H */

^ permalink raw reply

* [PATCH 3.2-longterm] bonding: ARP packet fix for balance-ALB (mode=6)
From: Matthew O'Connor @ 2013-04-01 16:38 UTC (permalink / raw)
  To: netdev

Do not modify or load balance ARP packets passing through balance-alb
mode (wherein the ARP did not originate locally, and arrived via a bridge).

This patch is backported from the upstream commit:
    From 567b871e503316b0927e54a3d7c86d50b722d955 Mon Sep 17 00:00:00 2001
    Subject: [PATCH] bonding: rlb mode of bond should not alter ARP
originating via bridge

Signed-off-by:  Matthew O'Connor <liquidhorse@gmail.com>

---
diff -uprN linux-3.2.37/drivers/net/bonding/bond_alb.c
linux-3.2.37-patched/drivers/net/bonding/bond_alb.c
--- linux-3.2.37/drivers/net/bonding/bond_alb.c    2013-01-15
20:13:30.000000000 -0500
+++ linux-3.2.37-patched/drivers/net/bonding/bond_alb.c    2013-01-30
15:37:12.717485854 -0500
@@ -666,6 +666,12 @@ static struct slave *rlb_arp_xmit(struct
     struct arp_pkt *arp = arp_pkt(skb);
     struct slave *tx_slave = NULL;

+    /* Don't modify or load balance ARPs that do not originate locally
+     * (e.g.,arrive via a bridge).
+     */
+    if (!bond_slave_has_mac(bond, arp->mac_src))
+        return NULL;
+
     if (arp->op_code == htons(ARPOP_REPLY)) {
         /* the arp must be sent on the selected
         * rx channel
diff -uprN linux-3.2.37/drivers/net/bonding/bonding.h
linux-3.2.37-patched/drivers/net/bonding/bonding.h
--- linux-3.2.37/drivers/net/bonding/bonding.h    2013-01-15
20:13:30.000000000 -0500
+++ linux-3.2.37-patched/drivers/net/bonding/bonding.h    2013-01-30
15:37:12.729485790 -0500
@@ -18,6 +18,7 @@
 #include <linux/timer.h>
 #include <linux/proc_fs.h>
 #include <linux/if_bonding.h>
+#include <linux/etherdevice.h>
 #include <linux/cpumask.h>
 #include <linux/in6.h>
 #include <linux/netpoll.h>
@@ -436,6 +437,18 @@ static inline void bond_destroy_proc_dir
 }
 #endif

+static inline struct slave *bond_slave_has_mac(struct bonding *bond,
+                           const u8 *mac)
+{
+    int i = 0;
+    struct slave *tmp;
+
+    bond_for_each_slave(bond, tmp, i)
+        if (ether_addr_equal_64bits(mac, tmp->dev->dev_addr))
+            return tmp;
+
+    return NULL;
+}

 /* exported from bond_main.c */
 extern int bond_net_id;
diff -uprN linux-3.2.37/include/linux/etherdevice.h
linux-3.2.37-patched/include/linux/etherdevice.h
--- linux-3.2.37/include/linux/etherdevice.h    2013-01-15
20:13:30.000000000 -0500
+++ linux-3.2.37-patched/include/linux/etherdevice.h    2013-01-30
15:37:12.729485790 -0500
@@ -275,4 +275,37 @@ static inline unsigned long compare_ethe
 #endif
 }

+/**
+ * ether_addr_equal_64bits - Compare two Ethernet addresses
+ * @addr1: Pointer to an array of 8 bytes
+ * @addr2: Pointer to an other array of 8 bytes
+ *
+ * Compare two Ethernet addresses, returns true if equal, false otherwise.
+ *
+ * The function doesn't need any conditional branches and possibly uses
+ * word memory accesses on CPU allowing cheap unaligned memory reads.
+ * arrays = { byte1, byte2, byte3, byte4, byte5, byte6, pad1, pad2 }
+ *
+ * Please note that alignment of addr1 & addr2 are only guaranteed to
be 16 bits.
+ */
+
+static inline bool ether_addr_equal_64bits(const u8 addr1[6+2],
+                                           const u8 addr2[6+2])
+{
+#ifdef CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS
+        unsigned long fold = ((*(unsigned long *)addr1) ^
+                              (*(unsigned long *)addr2));
+
+        if (sizeof(fold) == 8)
+                return zap_last_2bytes(fold) == 0;
+
+        fold |= zap_last_2bytes((*(unsigned long *)(addr1 + 4)) ^
+                                (*(unsigned long *)(addr2 + 4)));
+        return fold == 0;
+#else
+        return ether_addr_equal(addr1, addr2);
+#endif
+}
+
+
 #endif    /* _LINUX_ETHERDEVICE_H */

^ permalink raw reply

* [PATCH 3.0-longterm] bonding: ARP packet fix for balance-ALB (mode=6)
From: Matthew O'Connor @ 2013-04-01 16:36 UTC (permalink / raw)
  To: netdev

Do not modify or load balance ARP packets passing through balance-alb
mode (wherein the ARP did not originate locally, and arrived via a bridge).

This patch is backported from the upstream commit:
    From 567b871e503316b0927e54a3d7c86d50b722d955 Mon Sep 17 00:00:00 2001
    Subject: [PATCH] bonding: rlb mode of bond should not alter ARP
originating via bridge

Signed-off-by:  Matthew O'Connor <liquidhorse@gmail.com>

---
diff -uNr linux-3.0.0-a/drivers/net/bonding/bond_alb.c
linux-3.0.0-b/drivers/net/bonding/bond_alb.c
--- linux-3.0.0-a/drivers/net/bonding/bond_alb.c    2013-01-10
12:47:53.000000000 -0500
+++ linux-3.0.0-b/drivers/net/bonding/bond_alb.c    2013-01-10
12:50:58.000000000 -0500
@@ -666,6 +666,12 @@
     struct arp_pkt *arp = arp_pkt(skb);
     struct slave *tx_slave = NULL;

+    /* Don't modify or load balance ARPs that do not originate locally
+     * (e.g.,arrive via a bridge).
+     */
+    if (!bond_slave_has_mac(bond, arp->mac_src))
+        return NULL;
+
     if (arp->op_code == htons(ARPOP_REPLY)) {
         /* the arp must be sent on the selected
         * rx channel
diff -uNr linux-3.0.0-a/drivers/net/bonding/bonding.h
linux-3.0.0-b/drivers/net/bonding/bonding.h
--- linux-3.0.0-a/drivers/net/bonding/bonding.h    2011-07-21
22:17:23.000000000 -0400
+++ linux-3.0.0-b/drivers/net/bonding/bonding.h    2013-01-10
12:51:05.000000000 -0500
@@ -18,6 +18,7 @@
 #include <linux/timer.h>
 #include <linux/proc_fs.h>
 #include <linux/if_bonding.h>
+#include <linux/etherdevice.h>
 #include <linux/cpumask.h>
 #include <linux/in6.h>
 #include <linux/netpoll.h>
@@ -431,6 +432,18 @@
 }
 #endif

+static inline struct slave *bond_slave_has_mac(struct bonding *bond,
+                           const u8 *mac)
+{
+    int i = 0;
+    struct slave *tmp;
+
+    bond_for_each_slave(bond, tmp, i)
+        if (ether_addr_equal_64bits(mac, tmp->dev->dev_addr))
+            return tmp;
+
+    return NULL;
+}

 /* exported from bond_main.c */
 extern int bond_net_id;
diff -uNr linux-3.0.0-a/include/linux/etherdevice.h
linux-3.0.0-b/include/linux/etherdevice.h
--- linux-3.0.0-a/include/linux/etherdevice.h    2011-07-21
22:17:23.000000000 -0400
+++ linux-3.0.0-b/include/linux/etherdevice.h    2013-01-10
12:51:16.000000000 -0500
@@ -275,4 +275,37 @@
 #endif
 }

+/**
+ * ether_addr_equal_64bits - Compare two Ethernet addresses
+ * @addr1: Pointer to an array of 8 bytes
+ * @addr2: Pointer to an other array of 8 bytes
+ *
+ * Compare two Ethernet addresses, returns true if equal, false otherwise.
+ *
+ * The function doesn't need any conditional branches and possibly uses
+ * word memory accesses on CPU allowing cheap unaligned memory reads.
+ * arrays = { byte1, byte2, byte3, byte4, byte5, byte6, pad1, pad2 }
+ *
+ * Please note that alignment of addr1 & addr2 are only guaranteed to
be 16 bits.
+ */
+
+static inline bool ether_addr_equal_64bits(const u8 addr1[6+2],
+                                           const u8 addr2[6+2])
+{
+#ifdef CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS
+        unsigned long fold = ((*(unsigned long *)addr1) ^
+                              (*(unsigned long *)addr2));
+
+        if (sizeof(fold) == 8)
+                return zap_last_2bytes(fold) == 0;
+
+        fold |= zap_last_2bytes((*(unsigned long *)(addr1 + 4)) ^
+                                (*(unsigned long *)(addr2 + 4)));
+        return fold == 0;
+#else
+        return ether_addr_equal(addr1, addr2);
+#endif
+}
+
+
 #endif    /* _LINUX_ETHERDEVICE_H */

^ permalink raw reply

* Re: [PATCH] core: fix the use of this_cpu_ptr
From: Eric Dumazet @ 2013-04-01 16:31 UTC (permalink / raw)
  To: Christoph Lameter; +Cc: RongQing Li, Shan Wei, netdev
In-Reply-To: <0000013dc6307f44-940f2bf1-7556-4d9e-92ab-1a84d2a47ca8-000000@email.amazonses.com>

On Mon, 2013-04-01 at 15:21 +0000, Christoph Lameter wrote:
> On Fri, 29 Mar 2013, RongQing Li wrote:

> > flush_tasklet is not a percpu var, it is a member of percpu var.
> 
> Well then it would be best to use this_cpu_read() instead of this_cpu_ptr.
> It also will generate better code.

I believe we already had this discussion in the past.

flush_tasklet is a structure, and we need its address, not read its
content.

You can not use this_cpu_read() to get its address, and following
code is fine.

tasklet = &this_cpu_ptr(info->cache->percpu)->flush_tasklet;

Similar to this code in mm/page_alloc.c

pcp = &this_cpu_ptr(zone->pageset)->pcp;

^ permalink raw reply

* Linux driver for Realtek RTL8723AU devices with USB ID 0bda:1724 such as found in Lenovo IdeaPad Yoga 13
From: Larry Finger @ 2013-04-01 16:26 UTC (permalink / raw)
  To: linux-wireless; +Cc: LKML, netdev, Dan Williams, 'George0505', Joon Ro

After receiving a number of inquiries regarding a Linux driver for the RTL8723AU 
device with USB ID 0bda:1724, I asked my contacts in the USB group at Realtek if 
such a driver were available. They responded that there is one, but not on their 
public FTP site; however, they gave me the necessary info to get it from their 
private site, and also granted permission for me to make it publicly available.

Therefore, I have created a git repository that can be copied using the command:

git clone http://github.com/lwfinger/rtl8723au.git

Once cloned, the command sequence 'make && sudo make install' in the resulting 
source directory will build and install the driver, which is named 8723au. At 
the moment, it does not require external firmware. The driver will build on all 
kernel versions up to, and including 3.9. I have not tested on kernels older 
than 3.7, but the code contains conditional statements testing against kernel 
2.6.18. It should build on any kernel new enough for the base hardware.

My plan is to evolve this driver into a form that is suitable for inclusion in 
the staging tree of the kernel; however, that will take some effort and time, 
which is why I am making the intermediate results available now. As that work 
proceeds, the git version will evolve into code that can be submitted to the 
kernel with only small changes.

Please be aware that I do not have any hardware that uses this driver, thus I 
can only guarantee that the code compiles; however, Joon Ro has tested the 
driver and he assures me that it works. Together we have been eliminating 
spurious log entries.

The 8723a chip also includes Bluetooth hardware. I have not received any 
indication that it is active in the RTL8723AU, but it certainly is in the 
RTL8723AE, the PCIe version. In a separate activity, I am working with Realtek 
to prepare a kernel driver for this portion of the device.

I welcome your comments, bug reports, patches, suggestions for changes, and 
testing. In addition, please spread the word to anyone that has been seeking 
this driver.

Thanks,

Larry

^ permalink raw reply

* Re: [PATCH 1/1] cbq: incorrect processing of high limits
From: Eric Dumazet @ 2013-04-01 16:24 UTC (permalink / raw)
  To: Vasily Averin; +Cc: netdev, Alexey Kuznetsov, David S. Miller, Jamal Hadi Salim
In-Reply-To: <5159852C.8060003@parallels.com>

On Mon, 2013-04-01 at 17:01 +0400, Vasily Averin wrote:
> currently cbq works incorrectly for limits > 10% real link bandwidth,
> and practically does not work for limits > 50% real link bandwidth.
> Below are results of experiments taken on 1 Gbit link
> 
>  In shaper | Actual Result
> -----------+---------------
>   100M     | 108 Mbps
>   200M     | 244 Mbps
>   300M     | 412 Mbps
>   500M     | 893 Mbps
> 
> This happen because of q->now changes incorrectly in cbq_dequeue():
> when it is called before real end of packet transmitting,
> L2T is greater than real time delay, q_now gets an extra boost
> but never compensate it.
> 
> To fix this problem we prevent change of q->now until its synchronization
> with real time.
> 
> Signed-off-by: Vasily Averin <vvs@openvz.org>
> Reviewed-by: Alexey Kuznetsov <kuznet@ms2.inr.ac.ru>
> ---
>  net/sched/sch_cbq.c |    5 ++++-
>  1 files changed, 4 insertions(+), 1 deletions(-)
> 
> diff --git a/net/sched/sch_cbq.c b/net/sched/sch_cbq.c
> index 6aabd77..5b5c83a 100644
> --- a/net/sched/sch_cbq.c
> +++ b/net/sched/sch_cbq.c
> @@ -962,8 +962,11 @@ cbq_dequeue(struct Qdisc *sch)
> 		cbq_update(q);
> 		if ((incr -= incr2) < 0)
> 			incr = 0;
> +		q->now += incr;
> +	} else {
> +		if (now > q->now)
> +			q->now = now;
> 	}
> -	q->now += incr;
> 	q->now_rt = now;
> 
> 	for (;;) {

Acked-by: Eric Dumazet <edumazet@google.com>

Still, after your patch a limit of 500Mbit gives :

 rate 515872Kbit 946pps backlog 0b 2p requeues 0 

HTB is more precise :

 rate 500667Kbit 41336pps backlog 0b 2p requeues 0 

^ permalink raw reply

* Re: [PATCH 1/2 v3] usbnet: allow status interrupt URB to always be active
From: Dan Williams @ 2013-04-01 16:04 UTC (permalink / raw)
  To: Ming Lei
  Cc: Oliver Neukum, Elina Pasheva, Network Development, linux-usb,
	Rory Filer, Phil Sutter
In-Reply-To: <CACVXFVMBAzTYZKiE_uSTqr_yB4f7c5_PSnK=LBP6=oWdWwYHfg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

On Sat, 2013-03-30 at 22:11 +0800, Ming Lei wrote:
> On Fri, Mar 29, 2013 at 12:30 AM, Dan Williams <dcbw-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
> >
> > Some drivers (sierra_net) need the status interrupt URB
> > active even when the device is closed, because they receive
> > custom indications from firmware.  Add functions to refcount
> > the status interrupt URB submit/kill operation so that
> > sub-drivers and the generic driver don't fight over whether
> > the status interrupt URB is active or not.
> >
> > A sub-driver can call usbnet_status_start() at any time, but
> > the URB is only submitted the first time the function is
> > called.  Likewise, when the sub-driver is done with the URB,
> > it calls usbnet_status_stop() but the URB is only killed when
> > all users have stopped it.  The URB is still killed and
> > re-submitted for suspend/resume, as before, with the same
> > refcount it had at suspend.
> >
> > Signed-off-by: Dan Williams <dcbw-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> > ---
> > diff --git a/drivers/net/usb/usbnet.c b/drivers/net/usb/usbnet.c
> > index 51f3192..6431a03 100644
> > --- a/drivers/net/usb/usbnet.c
> > +++ b/drivers/net/usb/usbnet.c
> > @@ -252,6 +252,43 @@ static int init_status (struct usbnet *dev, struct
> usb_interface *intf)
> >         return 0;
> >  }
> >
> > +/* Submit the interrupt URB if it hasn't been submitted yet */
> > +int usbnet_status_start(struct usbnet *dev, gfp_t mem_flags)
> > +{
> > +       int ret = 0;
> > +
> > +       /* Only drivers that implement a status hook should call this */
> > +       BUG_ON(dev->interrupt == NULL);
> 
> It isn't worth BUG_ON() and returning 0 simply should be OK.

The code is attempting to ensure that modules never, ever call
usbnet_status_start() unless they implement the "status" subdriver hook
which actually handles the interrupt URB messages.  If they don't
implement the hook, that means the driver is doing nothing special with
the interrupt URB, and thus it shouldn't be calling these functions.

> > +
> > +       if (test_bit (EVENT_DEV_ASLEEP, &dev->flags))
> > +               return -EINVAL;
> > +
> > +       mutex_lock (&dev->interrupt_mutex);
> > +       if (++dev->interrupt_count == 1)
> > +               ret = usb_submit_urb (dev->interrupt, mem_flags);
> > +       dev_dbg(&dev->udev->dev, "incremented interrupt URB count to
> %d\n",
> > +               dev->interrupt_count);
> > +       mutex_unlock (&dev->interrupt_mutex);
> > +       return ret;
> > +}
> > +EXPORT_SYMBOL_GPL(usbnet_status_start);
> > +
> > +/* Kill the interrupt URB if all submitters want it killed */
> > +void usbnet_status_stop(struct usbnet *dev)
> > +{
> > +       if (dev->interrupt) {
> > +               mutex_lock (&dev->interrupt_mutex);
> > +               BUG_ON(dev->interrupt_count == 0);
> > +               if (dev->interrupt_count && --dev->interrupt_count == 0)
> 
> Check on dev->interrupt_count isn't necessary.

Fixed.

> > +                       usb_kill_urb(dev->interrupt);
> > +               dev_dbg(&dev->udev->dev,
> > +                       "decremented interrupt URB count to %d\n",
> > +                       dev->interrupt_count);
> > +               mutex_unlock (&dev->interrupt_mutex);
> > +       }
> > +}
> > +EXPORT_SYMBOL_GPL(usbnet_status_stop);
> > +
> >  /* Passes this packet up the stack, updating its accounting.
> >   * Some link protocols batch packets, so their rx_fixup paths
> >   * can return clones as well as just modify the original skb.
> > @@ -725,7 +762,7 @@ int usbnet_stop (struct net_device *net)
> >         if (!(info->flags & FLAG_AVOID_UNLINK_URBS))
> >                 usbnet_terminate_urbs(dev);
> >
> > -       usb_kill_urb(dev->interrupt);
> > +       usbnet_status_stop(dev);
> >
> >         usbnet_purge_paused_rxq(dev);
> >
> > @@ -787,7 +824,7 @@ int usbnet_open (struct net_device *net)
> >
> >         /* start any status interrupt transfer */
> >         if (dev->interrupt) {
> > -               retval = usb_submit_urb (dev->interrupt, GFP_KERNEL);
> > +               retval = usbnet_status_start(dev, GFP_KERNEL);
> >                 if (retval < 0) {
> >                         netif_err(dev, ifup, dev->net,
> >                                   "intr submit %d\n", retval);
> > @@ -1430,6 +1467,8 @@ usbnet_probe (struct usb_interface *udev, const
> struct usb_device_id *prod)
> >         dev->delay.data = (unsigned long) dev;
> >         init_timer (&dev->delay);
> >         mutex_init (&dev->phy_mutex);
> > +       mutex_init (&dev->interrupt_mutex);
> > +       dev->interrupt_count = 0;
> >
> >         dev->net = net;
> >         strcpy (net->name, "usb%d");
> > @@ -1585,9 +1624,13 @@ int usbnet_resume (struct usb_interface *intf)
> >         int                     retval;
> >
> >         if (!--dev->suspend_count) {
> > -               /* resume interrupt URBs */
> > -               if (dev->interrupt && test_bit(EVENT_DEV_OPEN,
> &dev->flags))
> > -                       usb_submit_urb(dev->interrupt, GFP_NOIO);
> > +               /* resume interrupt URBs if they were submitted at
> suspend */
> > +               if (dev->interrupt) {
> > +                       mutex_lock (&dev->interrupt_mutex);
> > +                       if (dev->interrupt_count)
> > +                               usb_submit_urb(dev->interrupt, GFP_NOIO);
> > +                       mutex_unlock (&dev->interrupt_mutex);
> > +               }
> 
> Given the introduced usbnet_status_start/usbnet_status_sop, it is
> better to apply them with one extra 'force' parameter in suspend()
> and resume() too.

Except that I'd rather the 'force' parameter never leak out of usbnet.c,
since that's the only place it should ever be used.  Instead, I'll
create an internal _usbnet_status_start()/_usbnet_status_stop() function
that has these parameters, while the public ones just call these with
'false'.  Sound OK?

Thanks,
Dan

> >
> >                 spin_lock_irq(&dev->txq.lock);
> >                 while ((res = usb_get_from_anchor(&dev->deferred))) {
> > diff --git a/include/linux/usb/usbnet.h b/include/linux/usb/usbnet.h
> > index 0e5ac93..d71f44c 100644
> > --- a/include/linux/usb/usbnet.h
> > +++ b/include/linux/usb/usbnet.h
> > @@ -56,6 +56,8 @@ struct usbnet {
> >         struct sk_buff_head     done;
> >         struct sk_buff_head     rxq_pause;
> >         struct urb              *interrupt;
> > +       unsigned                interrupt_count;
> > +       struct mutex            interrupt_mutex;
> >         struct usb_anchor       deferred;
> >         struct tasklet_struct   bh;
> >
> > @@ -246,4 +248,7 @@ extern int usbnet_nway_reset(struct net_device *net);
> >
> >  extern int usbnet_manage_power(struct usbnet *, int);
> >
> > +int usbnet_status_start(struct usbnet *dev, gfp_t mem_flags);
> > +void usbnet_status_stop(struct usbnet *dev);
> > +
> >  #endif /* __LINUX_USB_USBNET_H */
> >
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-usb" in
> > the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> Thanks,
> --
> Ming Lei


--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox