Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH v2] macvlan: validate flags
From: John Fastabend @ 2013-08-01 19:34 UTC (permalink / raw)
  To: Michael S. Tsirkin; +Cc: linux-kernel, David S. Miller, Patrick McHardy, netdev
In-Reply-To: <20130801191957.GA19580@redhat.com>

On 8/1/2013 12:19 PM, Michael S. Tsirkin wrote:
> On Thu, Aug 01, 2013 at 10:24:19AM -0700, John Fastabend wrote:
>> On 8/1/2013 9:09 AM, Michael S. Tsirkin wrote:
>>> commit df8ef8f3aaa6692970a436204c4429210addb23a
>>>      macvlan: add FDB bridge ops and macvlan flags
>>> added a flags field to macvlan, which can be
>>> controlled from userspace.
>>> The idea is to make the interface future-proof
>>> so we can add flags and not new fields.
>>>
>>> However, flags value isn't validated, as a result,
>>> userspace can't detect which flags are supported.
>>>
>>> Cc: "David S. Miller" <davem@davemloft.net>
>>> Cc: John Fastabend <john.r.fastabend@intel.com>
>>> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
>>> ---
>>>
>>> Changes from v1:
>>> 	tweaked commit message
>>> 	no code changes
>>>
>>> Please consider this patch for -stable.
>>>
>>> The idea is by the time we add more flags,
>>> everyone has updated to a kernel that
>>> detects errors, so userspace will be able
>>> to detect supported flags cleanly.
>>>
>>
>> Agreed and because we haven't added more flags yet this shouldn't
>> break uapi. Thanks for catching this.
>>
>>>
>>>   drivers/net/macvlan.c | 7 +++++++
>>>   1 file changed, 7 insertions(+)
>>>
>>
>> By the same logic should we also add the check to macvlan_changelink()?
>
> I'm not sure what do you mean "By the same logic" -
> macvlan_changelink is static unlike macvlan_common_newlink
> which is exported to modules.

"By the same logic" I only meant to allow userspace to cleanly detect
supported flags even in the changelink case.

> So why isn't macvlan_validate sufficient for macvlan_changelink?

It is you are correct.

>
>>> diff --git a/drivers/net/macvlan.c b/drivers/net/macvlan.c
>>> index 18373b6..8445a94 100644
>>> --- a/drivers/net/macvlan.c
>>> +++ b/drivers/net/macvlan.c
>>> @@ -736,6 +736,10 @@ static int macvlan_validate(struct nlattr *tb[], struct nlattr *data[])
>>>   			return -EADDRNOTAVAIL;
>>>   	}
>>>
>>> +	if (data && data[IFLA_MACVLAN_FLAGS] &&
>>> +	    nla_get_u16(data[IFLA_MACVLAN_FLAGS]) & ~MACVLAN_FLAG_NOPROMISC)
>>> +		return -EINVAL;
>>> +
>>>   	if (data && data[IFLA_MACVLAN_MODE]) {
>>>   		switch (nla_get_u32(data[IFLA_MACVLAN_MODE])) {
>>>   		case MACVLAN_MODE_PRIVATE:
>>> @@ -809,6 +813,9 @@ int macvlan_common_newlink(struct net *src_net, struct net_device *dev,
>>>   	if (data && data[IFLA_MACVLAN_FLAGS])
>>>   		vlan->flags = nla_get_u16(data[IFLA_MACVLAN_FLAGS]);
>>>
>>> +	if (vlan->flags & ~MACVLAN_FLAG_NOPROMISC)
>>> +		return -EINVAL;
>>> +
>>
>> Is there really a case where newlink is called without first calling
>> validate? I don't think there is so the snippet here in newlink could
>> be dropped.
>>
>> Thanks,
>> John
>
> It seems so - macvtap_newlink calls macvlan_common_newlink.
> macvtap does not seem to have .validate.
>

but it calls macvlan_link_register() from macvtap_init which sets
up the validate ops,

int macvlan_link_register(struct rtnl_link_ops *ops)
{
         /* common fields */
         ops->priv_size          = sizeof(struct macvlan_dev);
         ops->validate           = macvlan_validate

^ permalink raw reply

* Re: [PATCH v2] macvlan: validate flags
From: John Fastabend @ 2013-08-01 19:34 UTC (permalink / raw)
  To: Michael S. Tsirkin; +Cc: linux-kernel, David S. Miller, Patrick McHardy, netdev
In-Reply-To: <20130801191957.GA19580@redhat.com>

On 8/1/2013 12:19 PM, Michael S. Tsirkin wrote:
> On Thu, Aug 01, 2013 at 10:24:19AM -0700, John Fastabend wrote:
>> On 8/1/2013 9:09 AM, Michael S. Tsirkin wrote:
>>> commit df8ef8f3aaa6692970a436204c4429210addb23a
>>>      macvlan: add FDB bridge ops and macvlan flags
>>> added a flags field to macvlan, which can be
>>> controlled from userspace.
>>> The idea is to make the interface future-proof
>>> so we can add flags and not new fields.
>>>
>>> However, flags value isn't validated, as a result,
>>> userspace can't detect which flags are supported.
>>>
>>> Cc: "David S. Miller" <davem@davemloft.net>
>>> Cc: John Fastabend <john.r.fastabend@intel.com>
>>> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
>>> ---
>>>
>>> Changes from v1:
>>> 	tweaked commit message
>>> 	no code changes
>>>
>>> Please consider this patch for -stable.
>>>
>>> The idea is by the time we add more flags,
>>> everyone has updated to a kernel that
>>> detects errors, so userspace will be able
>>> to detect supported flags cleanly.
>>>
>>
>> Agreed and because we haven't added more flags yet this shouldn't
>> break uapi. Thanks for catching this.
>>
>>>
>>>   drivers/net/macvlan.c | 7 +++++++
>>>   1 file changed, 7 insertions(+)
>>>
>>
>> By the same logic should we also add the check to macvlan_changelink()?
>
> I'm not sure what do you mean "By the same logic" -
> macvlan_changelink is static unlike macvlan_common_newlink
> which is exported to modules.

"By the same logic" I only meant to allow userspace to cleanly detect
supported flags even in the changelink case.

> So why isn't macvlan_validate sufficient for macvlan_changelink?

It is you are correct.

>
>>> diff --git a/drivers/net/macvlan.c b/drivers/net/macvlan.c
>>> index 18373b6..8445a94 100644
>>> --- a/drivers/net/macvlan.c
>>> +++ b/drivers/net/macvlan.c
>>> @@ -736,6 +736,10 @@ static int macvlan_validate(struct nlattr *tb[], struct nlattr *data[])
>>>   			return -EADDRNOTAVAIL;
>>>   	}
>>>
>>> +	if (data && data[IFLA_MACVLAN_FLAGS] &&
>>> +	    nla_get_u16(data[IFLA_MACVLAN_FLAGS]) & ~MACVLAN_FLAG_NOPROMISC)
>>> +		return -EINVAL;
>>> +
>>>   	if (data && data[IFLA_MACVLAN_MODE]) {
>>>   		switch (nla_get_u32(data[IFLA_MACVLAN_MODE])) {
>>>   		case MACVLAN_MODE_PRIVATE:
>>> @@ -809,6 +813,9 @@ int macvlan_common_newlink(struct net *src_net, struct net_device *dev,
>>>   	if (data && data[IFLA_MACVLAN_FLAGS])
>>>   		vlan->flags = nla_get_u16(data[IFLA_MACVLAN_FLAGS]);
>>>
>>> +	if (vlan->flags & ~MACVLAN_FLAG_NOPROMISC)
>>> +		return -EINVAL;
>>> +
>>
>> Is there really a case where newlink is called without first calling
>> validate? I don't think there is so the snippet here in newlink could
>> be dropped.
>>
>> Thanks,
>> John
>
> It seems so - macvtap_newlink calls macvlan_common_newlink.
> macvtap does not seem to have .validate.
>

but it calls macvlan_link_register() from macvtap_init which sets
up the validate ops,

int macvlan_link_register(struct rtnl_link_ops *ops)
{
         /* common fields */
         ops->priv_size          = sizeof(struct macvlan_dev);
         ops->validate           = macvlan_validate

^ permalink raw reply

* [PATCH v3 06/13] tile: update dev->stats directly in tilegx network driver
From: Chris Metcalf @ 2013-08-01 15:36 UTC (permalink / raw)
  To: linux-kernel, netdev
In-Reply-To: <cover.1375385119.git.cmetcalf@tilera.com>

Signed-off-by: Chris Metcalf <cmetcalf@tilera.com>
---
 drivers/net/ethernet/tile/tilegx.c | 29 ++++++++---------------------
 1 file changed, 8 insertions(+), 21 deletions(-)

diff --git a/drivers/net/ethernet/tile/tilegx.c b/drivers/net/ethernet/tile/tilegx.c
index 39c1e9e..e9eba2b 100644
--- a/drivers/net/ethernet/tile/tilegx.c
+++ b/drivers/net/ethernet/tile/tilegx.c
@@ -178,8 +178,6 @@ struct tile_net_priv {
 	int loopify_channel;
 	/* The egress channel (channel or loopify_channel). */
 	int echannel;
-	/* Total stats. */
-	struct net_device_stats stats;
 };
 
 /* Egress info, indexed by "priv->echannel" (lazily created as needed). */
@@ -414,7 +412,6 @@ static void tile_net_receive_skb(struct net_device *dev, struct sk_buff *skb,
 				 gxio_mpipe_idesc_t *idesc, unsigned long len)
 {
 	struct tile_net_info *info = &__get_cpu_var(per_cpu_info);
-	struct tile_net_priv *priv = netdev_priv(dev);
 
 	/* Encode the actual packet length. */
 	skb_put(skb, len);
@@ -428,8 +425,8 @@ static void tile_net_receive_skb(struct net_device *dev, struct sk_buff *skb,
 	netif_receive_skb(skb);
 
 	/* Update stats. */
-	tile_net_stats_add(1, &priv->stats.rx_packets);
-	tile_net_stats_add(len, &priv->stats.rx_bytes);
+	tile_net_stats_add(1, &dev->stats.rx_packets);
+	tile_net_stats_add(len, &dev->stats.rx_bytes);
 
 	/* Need a new buffer. */
 	if (idesc->size == buffer_size_enums[0])
@@ -445,7 +442,6 @@ static bool tile_net_handle_packet(gxio_mpipe_idesc_t *idesc)
 {
 	struct tile_net_info *info = &__get_cpu_var(per_cpu_info);
 	struct net_device *dev = tile_net_devs_for_channel[idesc->channel];
-	struct tile_net_priv *priv = netdev_priv(dev);
 	uint8_t l2_offset;
 	void *va;
 	void *buf;
@@ -459,7 +455,7 @@ static bool tile_net_handle_packet(gxio_mpipe_idesc_t *idesc)
 	 */
 	if (idesc->be || idesc->me || idesc->tr || idesc->ce) {
 		if (dev)
-			tile_net_stats_add(1, &priv->stats.rx_errors);
+			tile_net_stats_add(1, &dev->stats.rx_errors);
 		goto drop;
 	}
 
@@ -479,7 +475,7 @@ static bool tile_net_handle_packet(gxio_mpipe_idesc_t *idesc)
 	filter = filter_packet(dev, buf);
 	if (filter) {
 		if (dev)
-			tile_net_stats_add(1, &priv->stats.rx_dropped);
+			tile_net_stats_add(1, &dev->stats.rx_dropped);
 drop:
 		gxio_mpipe_iqueue_drop(&info->iqueue, idesc);
 	} else {
@@ -1502,7 +1498,6 @@ static void tso_headers_prepare(struct sk_buff *skb, unsigned char *headers,
 static void tso_egress(struct net_device *dev, gxio_mpipe_equeue_t *equeue,
 		       struct sk_buff *skb, unsigned char *headers, s64 slot)
 {
-	struct tile_net_priv *priv = netdev_priv(dev);
 	struct skb_shared_info *sh = skb_shinfo(skb);
 	unsigned int sh_len = skb_transport_offset(skb) + tcp_hdrlen(skb);
 	unsigned int data_len = skb->len - sh_len;
@@ -1580,8 +1575,8 @@ static void tso_egress(struct net_device *dev, gxio_mpipe_equeue_t *equeue,
 	}
 
 	/* Update stats. */
-	tile_net_stats_add(tx_packets, &priv->stats.tx_packets);
-	tile_net_stats_add(tx_bytes, &priv->stats.tx_bytes);
+	tile_net_stats_add(tx_packets, &dev->stats.tx_packets);
+	tile_net_stats_add(tx_bytes, &dev->stats.tx_bytes);
 }
 
 /* Do "TSO" handling for egress.
@@ -1724,9 +1719,9 @@ static int tile_net_tx(struct sk_buff *skb, struct net_device *dev)
 	add_comp(equeue, comps, slot - 1, skb);
 
 	/* NOTE: Use ETH_ZLEN for short packets (e.g. 42 < 60). */
-	tile_net_stats_add(1, &priv->stats.tx_packets);
+	tile_net_stats_add(1, &dev->stats.tx_packets);
 	tile_net_stats_add(max_t(unsigned int, len, ETH_ZLEN),
-			   &priv->stats.tx_bytes);
+			   &dev->stats.tx_bytes);
 
 	local_irq_restore(irqflags);
 
@@ -1757,13 +1752,6 @@ static int tile_net_ioctl(struct net_device *dev, struct ifreq *rq, int cmd)
 	return -EOPNOTSUPP;
 }
 
-/* Get system network statistics for device. */
-static struct net_device_stats *tile_net_get_stats(struct net_device *dev)
-{
-	struct tile_net_priv *priv = netdev_priv(dev);
-	return &priv->stats;
-}
-
 /* Change the MTU. */
 static int tile_net_change_mtu(struct net_device *dev, int new_mtu)
 {
@@ -1813,7 +1801,6 @@ static const struct net_device_ops tile_net_ops = {
 	.ndo_start_xmit = tile_net_tx,
 	.ndo_select_queue = tile_net_select_queue,
 	.ndo_do_ioctl = tile_net_ioctl,
-	.ndo_get_stats = tile_net_get_stats,
 	.ndo_change_mtu = tile_net_change_mtu,
 	.ndo_tx_timeout = tile_net_tx_timeout,
 	.ndo_set_mac_address = tile_net_set_mac_address,
-- 
1.8.3.1

^ permalink raw reply related

* [PATCH v3 11/13] tile: make "tile_net.custom" a proper bool module parameter
From: Chris Metcalf @ 2013-08-01 15:36 UTC (permalink / raw)
  To: linux-kernel, netdev
In-Reply-To: <cover.1375385119.git.cmetcalf@tilera.com>

Signed-off-by: Chris Metcalf <cmetcalf@tilera.com>
---
 drivers/net/ethernet/tile/tilegx.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/tile/tilegx.c b/drivers/net/ethernet/tile/tilegx.c
index 6d94d58..7302e84 100644
--- a/drivers/net/ethernet/tile/tilegx.c
+++ b/drivers/net/ethernet/tile/tilegx.c
@@ -256,11 +256,11 @@ static char *network_cpus_string;
 /* The actual cpus in "network_cpus". */
 static struct cpumask network_cpus_map;
 
-/* If "loopify=LINK" was specified, this is "LINK". */
+/* If "tile_net.loopify=LINK" was specified, this is "LINK". */
 static char *loopify_link_name;
 
-/* If "tile_net.custom" was specified, this is non-NULL. */
-static char *custom_str;
+/* If "tile_net.custom" was specified, this is true. */
+static bool custom_flag;
 
 /* If "tile_net.jumbo=NUM" was specified, this is "NUM". */
 static uint jumbo_num;
@@ -323,7 +323,7 @@ MODULE_PARM_DESC(loopify, "name the device to use loop0/1 for ingress/egress");
 /* The "tile_net.custom" argument causes us to ignore the "conventional"
  * classifier metadata, in particular, the "l2_offset".
  */
-module_param_named(custom, custom_str, charp, 0444);
+module_param_named(custom, custom_flag, bool, 0444);
 MODULE_PARM_DESC(custom, "indicates a (heavily) customized classifier");
 
 /* The "tile_net.jumbo" argument causes us to support "jumbo" packets,
@@ -501,7 +501,7 @@ static bool tile_net_handle_packet(int instance, gxio_mpipe_idesc_t *idesc)
 	}
 
 	/* Get the "l2_offset", if allowed. */
-	l2_offset = custom_str ? 0 : gxio_mpipe_idesc_get_l2_offset(idesc);
+	l2_offset = custom_flag ? 0 : gxio_mpipe_idesc_get_l2_offset(idesc);
 
 	/* Get the VA (including NET_IP_ALIGN bytes of "headroom"). */
 	va = tile_io_addr_to_va((unsigned long)idesc->va);
-- 
1.8.3.1

^ permalink raw reply related

* Re: [PATCH v2] macvlan: validate flags
From: Michael S. Tsirkin @ 2013-08-01 19:43 UTC (permalink / raw)
  To: John Fastabend; +Cc: linux-kernel, David S. Miller, Patrick McHardy, netdev
In-Reply-To: <51FAB840.6000705@intel.com>

On Thu, Aug 01, 2013 at 12:34:24PM -0700, John Fastabend wrote:
> On 8/1/2013 12:19 PM, Michael S. Tsirkin wrote:
> >On Thu, Aug 01, 2013 at 10:24:19AM -0700, John Fastabend wrote:
> >>On 8/1/2013 9:09 AM, Michael S. Tsirkin wrote:
> >>>commit df8ef8f3aaa6692970a436204c4429210addb23a
> >>>     macvlan: add FDB bridge ops and macvlan flags
> >>>added a flags field to macvlan, which can be
> >>>controlled from userspace.
> >>>The idea is to make the interface future-proof
> >>>so we can add flags and not new fields.
> >>>
> >>>However, flags value isn't validated, as a result,
> >>>userspace can't detect which flags are supported.
> >>>
> >>>Cc: "David S. Miller" <davem@davemloft.net>
> >>>Cc: John Fastabend <john.r.fastabend@intel.com>
> >>>Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> >>>---
> >>>
> >>>Changes from v1:
> >>>	tweaked commit message
> >>>	no code changes
> >>>
> >>>Please consider this patch for -stable.
> >>>
> >>>The idea is by the time we add more flags,
> >>>everyone has updated to a kernel that
> >>>detects errors, so userspace will be able
> >>>to detect supported flags cleanly.
> >>>
> >>
> >>Agreed and because we haven't added more flags yet this shouldn't
> >>break uapi. Thanks for catching this.
> >>
> >>>
> >>>  drivers/net/macvlan.c | 7 +++++++
> >>>  1 file changed, 7 insertions(+)
> >>>
> >>
> >>By the same logic should we also add the check to macvlan_changelink()?
> >
> >I'm not sure what do you mean "By the same logic" -
> >macvlan_changelink is static unlike macvlan_common_newlink
> >which is exported to modules.
> 
> "By the same logic" I only meant to allow userspace to cleanly detect
> supported flags even in the changelink case.
> 
> >So why isn't macvlan_validate sufficient for macvlan_changelink?
> 
> It is you are correct.
> 
> >
> >>>diff --git a/drivers/net/macvlan.c b/drivers/net/macvlan.c
> >>>index 18373b6..8445a94 100644
> >>>--- a/drivers/net/macvlan.c
> >>>+++ b/drivers/net/macvlan.c
> >>>@@ -736,6 +736,10 @@ static int macvlan_validate(struct nlattr *tb[], struct nlattr *data[])
> >>>  			return -EADDRNOTAVAIL;
> >>>  	}
> >>>
> >>>+	if (data && data[IFLA_MACVLAN_FLAGS] &&
> >>>+	    nla_get_u16(data[IFLA_MACVLAN_FLAGS]) & ~MACVLAN_FLAG_NOPROMISC)
> >>>+		return -EINVAL;
> >>>+
> >>>  	if (data && data[IFLA_MACVLAN_MODE]) {
> >>>  		switch (nla_get_u32(data[IFLA_MACVLAN_MODE])) {
> >>>  		case MACVLAN_MODE_PRIVATE:
> >>>@@ -809,6 +813,9 @@ int macvlan_common_newlink(struct net *src_net, struct net_device *dev,
> >>>  	if (data && data[IFLA_MACVLAN_FLAGS])
> >>>  		vlan->flags = nla_get_u16(data[IFLA_MACVLAN_FLAGS]);
> >>>
> >>>+	if (vlan->flags & ~MACVLAN_FLAG_NOPROMISC)
> >>>+		return -EINVAL;
> >>>+
> >>
> >>Is there really a case where newlink is called without first calling
> >>validate? I don't think there is so the snippet here in newlink could
> >>be dropped.
> >>
> >>Thanks,
> >>John
> >
> >It seems so - macvtap_newlink calls macvlan_common_newlink.
> >macvtap does not seem to have .validate.
> >
> 
> but it calls macvlan_link_register() from macvtap_init which sets
> up the validate ops,
> 
> int macvlan_link_register(struct rtnl_link_ops *ops)
> {
>         /* common fields */
>         ops->priv_size          = sizeof(struct macvlan_dev);
>         ops->validate           = macvlan_validate

I see. I'll drop this second chunk in the patch,
thanks for catching this.

-- 
MST

^ permalink raw reply

* Re: [PATCH net-next] net: add a temporary sanity check in skb_orphan()
From: David Miller @ 2013-08-01 19:49 UTC (permalink / raw)
  To: eric.dumazet; +Cc: netdev
In-Reply-To: <1375382588.10515.181.camel@edumazet-glaptop>

From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Thu, 01 Aug 2013 11:43:08 -0700

> From: Eric Dumazet <edumazet@google.com>
> 
> David suggested to add a BUG_ON() to catch if some layer
> sets skb->sk pointer without a corresponding destructor.
> 
> As skb can sit in a queue, it's mandatory to make sure the
> socket cannot disappear, and it's usually done by taking a
> reference on the socket, then releasing it from the skb
> destructor.
> 
> This patch is a follow-up to commit c34a761231b5
> ("net: skb_orphan() changes") and will be reverted after
> catching all possible offenders if any.
> 
> Suggested-by: David Miller <davem@davemloft.net>
> Signed-off-by: Eric Dumazet <edumazet@google.com>

Applied, thanks Eric.

^ permalink raw reply

* Re: pull request: wireless 2013-08-01
From: David Miller @ 2013-08-01 19:58 UTC (permalink / raw)
  To: linville; +Cc: linux-wireless, netdev, linux-kernel
In-Reply-To: <20130801190144.GC13922@tuxdriver.com>

From: "John W. Linville" <linville@tuxdriver.com>
Date: Thu, 1 Aug 2013 15:01:44 -0400

> I'll remind my feeder maintainers to slowdown the patchflow.

Yes, please do :-)

>   git://git.kernel.org/pub/scm/linux/kernel/git/linville/wireless.git for-davem

Pulled, thanks.

^ permalink raw reply

* Re: [Patch net-next 1/2] net: fix a compile error when CONFIG_NET_LL_RX_POLL is not set
From: David Miller @ 2013-08-01 20:03 UTC (permalink / raw)
  To: amwang; +Cc: eliezer.tamir, netdev
In-Reply-To: <1375344487.7780.14.camel@cr0>

From: Cong Wang <amwang@redhat.com>
Date: Thu, 01 Aug 2013 16:08:07 +0800

> Thinking about it again, what is the point of your commit commit
> 89bf1b5a683df497c572c4d3bd3f9c9aa919d773 (net: remove NET_LL_RX_POLL
> config menue)? After that commit, CONFIG_NET_LL_RX_POLL is not visible
> by user and no other configs select it, also since it defaults to y, it
> will be _always_ enabled. If this is really what you want, we can simply
> remove all !CONFIG_NET_LL_RX_POLL code.

Yes we could do that, it has no dependencies which is the usual reason
to have a hidden Kconfig var like this.

^ permalink raw reply

* Re: [PATCH] vlan: cleanup the usage of *vlan in vlan_dev_hard_header
From: David Miller @ 2013-08-01 20:12 UTC (permalink / raw)
  To: shhuiw; +Cc: kaber, netdev
In-Reply-To: <51F9FBCF.9060405@gmail.com>

From: Wang Sheng-Hui <shhuiw@gmail.com>
Date: Thu, 01 Aug 2013 14:10:23 +0800

> We can use vlan directly after we get the pointer at the beginning
>     *vlan = vlan_dev_priv(dev)
> and do not use vlan_dev_priv(dev) to access its fields afterwards.
> 
> Signed-off-by: Wang Sheng-Hui <shhuiw@gmail.com>

This patch was corrupted by your email client.

Please read Documentation/email-clients.txt to learn how to
fix this, send a test patch to yourself, and only when you
can successfully apply the patch you receive in that test email
should you repost your patch again here.

Thanks.

^ permalink raw reply

* [PATCH V2 0/3] networking: Use ETH_ALEN where appropriate
From: Joe Perches @ 2013-08-01 20:14 UTC (permalink / raw)
  To: netdev
  Cc: linux-kernel, linux-arm-kernel, linuxppc-dev, linux-usb,
	linux-media, netfilter-devel, virtualization, wimax

Convert the uses mac addresses to ETH_ALEN so
it's easier to find and verify where mac addresses
need to be __aligned(2)

Change from initial submission:
- Remove include/acpi/actbl2.h conversion
  It's a file copied from outside ACPI sources

Joe Perches (3):
  uapi: Convert some uses of 6 to ETH_ALEN
  include: Convert ethernet mac address declarations to use ETH_ALEN
  ethernet: Convert mac address uses of 6 to ETH_ALEN

 drivers/net/ethernet/8390/ax88796.c                |  4 +-
 drivers/net/ethernet/amd/pcnet32.c                 |  6 +--
 drivers/net/ethernet/broadcom/cnic_if.h            |  6 +--
 drivers/net/ethernet/dec/tulip/tulip_core.c        |  8 +--
 drivers/net/ethernet/i825xx/sun3_82586.h           |  4 +-
 drivers/net/ethernet/myricom/myri10ge/myri10ge.c   |  2 +-
 drivers/net/ethernet/nuvoton/w90p910_ether.c       |  4 +-
 drivers/net/ethernet/pasemi/pasemi_mac.c           | 13 ++---
 drivers/net/ethernet/pasemi/pasemi_mac.h           |  4 +-
 drivers/net/ethernet/qlogic/netxen/netxen_nic_hw.c |  4 +-
 drivers/net/ethernet/qlogic/qlge/qlge.h            |  2 +-
 include/linux/dm9000.h                             |  4 +-
 include/linux/fs_enet_pd.h                         |  3 +-
 include/linux/ieee80211.h                          | 59 +++++++++++-----------
 include/linux/mlx4/device.h                        | 11 ++--
 include/linux/mlx4/qp.h                            |  5 +-
 include/linux/mv643xx_eth.h                        |  3 +-
 include/linux/sh_eth.h                             |  3 +-
 include/linux/smsc911x.h                           |  3 +-
 include/linux/uwb/spec.h                           |  5 +-
 include/media/tveeprom.h                           |  4 +-
 include/net/irda/irlan_common.h                    |  3 +-
 include/uapi/linux/dn.h                            |  3 +-
 include/uapi/linux/if_bridge.h                     |  3 +-
 include/uapi/linux/netfilter_bridge/ebt_802_3.h    |  5 +-
 include/uapi/linux/netfilter_ipv4/ipt_CLUSTERIP.h  |  3 +-
 include/uapi/linux/virtio_net.h                    |  2 +-
 include/uapi/linux/wimax/i2400m.h                  |  4 +-
 28 files changed, 100 insertions(+), 80 deletions(-)

-- 
1.8.1.2.459.gbcd45b4.dirty

^ permalink raw reply

* [PATCH V2 1/3] uapi: Convert some uses of 6 to ETH_ALEN
From: Joe Perches @ 2013-08-01 20:14 UTC (permalink / raw)
  To: netdev
  Cc: wimax, Michael S. Tsirkin, linux-kernel, Inaky Perez-Gonzalez,
	linux-wimax, Bart De Schuymer, netfilter-devel, virtualization
In-Reply-To: <cover.1375387593.git.joe@perches.com>

Use the #define where appropriate.

Add #include <linux/if_ether.h>
where appropriate too.

Signed-off-by: Joe Perches <joe@perches.com>
---
 include/uapi/linux/dn.h                           | 3 ++-
 include/uapi/linux/if_bridge.h                    | 3 ++-
 include/uapi/linux/netfilter_bridge/ebt_802_3.h   | 5 +++--
 include/uapi/linux/netfilter_ipv4/ipt_CLUSTERIP.h | 3 ++-
 include/uapi/linux/virtio_net.h                   | 2 +-
 include/uapi/linux/wimax/i2400m.h                 | 4 ++--
 6 files changed, 12 insertions(+), 8 deletions(-)

diff --git a/include/uapi/linux/dn.h b/include/uapi/linux/dn.h
index 9c50445..5fbdd3d 100644
--- a/include/uapi/linux/dn.h
+++ b/include/uapi/linux/dn.h
@@ -2,6 +2,7 @@
 #define _LINUX_DN_H
 
 #include <linux/types.h>
+#include <linux/if_ether.h>
 
 /*
 
@@ -120,7 +121,7 @@ struct linkinfo_dn {
  * Ethernet address format (for DECnet)
  */
 union etheraddress {
-        __u8 dne_addr[6];             /* Full ethernet address */
+        __u8 dne_addr[ETH_ALEN];      /* Full ethernet address */
   struct {
                 __u8 dne_hiord[4];    /* DECnet HIORD prefix   */
                 __u8 dne_nodeaddr[2]; /* DECnet node address   */
diff --git a/include/uapi/linux/if_bridge.h b/include/uapi/linux/if_bridge.h
index 2d70d79..39f621a 100644
--- a/include/uapi/linux/if_bridge.h
+++ b/include/uapi/linux/if_bridge.h
@@ -14,6 +14,7 @@
 #define _UAPI_LINUX_IF_BRIDGE_H
 
 #include <linux/types.h>
+#include <linux/if_ether.h>
 
 #define SYSFS_BRIDGE_ATTR	"bridge"
 #define SYSFS_BRIDGE_FDB	"brforward"
@@ -88,7 +89,7 @@ struct __port_info {
 };
 
 struct __fdb_entry {
-	__u8 mac_addr[6];
+	__u8 mac_addr[ETH_ALEN];
 	__u8 port_no;
 	__u8 is_local;
 	__u32 ageing_timer_value;
diff --git a/include/uapi/linux/netfilter_bridge/ebt_802_3.h b/include/uapi/linux/netfilter_bridge/ebt_802_3.h
index 5bf8491..f37522a 100644
--- a/include/uapi/linux/netfilter_bridge/ebt_802_3.h
+++ b/include/uapi/linux/netfilter_bridge/ebt_802_3.h
@@ -2,6 +2,7 @@
 #define _UAPI__LINUX_BRIDGE_EBT_802_3_H
 
 #include <linux/types.h>
+#include <linux/if_ether.h>
 
 #define EBT_802_3_SAP 0x01
 #define EBT_802_3_TYPE 0x02
@@ -42,8 +43,8 @@ struct hdr_ni {
 };
 
 struct ebt_802_3_hdr {
-	__u8  daddr[6];
-	__u8  saddr[6];
+	__u8  daddr[ETH_ALEN];
+	__u8  saddr[ETH_ALEN];
 	__be16 len;
 	union {
 		struct hdr_ui ui;
diff --git a/include/uapi/linux/netfilter_ipv4/ipt_CLUSTERIP.h b/include/uapi/linux/netfilter_ipv4/ipt_CLUSTERIP.h
index c6a204c..eac0f65 100644
--- a/include/uapi/linux/netfilter_ipv4/ipt_CLUSTERIP.h
+++ b/include/uapi/linux/netfilter_ipv4/ipt_CLUSTERIP.h
@@ -2,6 +2,7 @@
 #define _IPT_CLUSTERIP_H_target
 
 #include <linux/types.h>
+#include <linux/if_ether.h>
 
 enum clusterip_hashmode {
     CLUSTERIP_HASHMODE_SIP = 0,
@@ -22,7 +23,7 @@ struct ipt_clusterip_tgt_info {
 	__u32 flags;
 
 	/* only relevant for new ones */
-	__u8 clustermac[6];
+	__u8 clustermac[ETH_ALEN];
 	__u16 num_total_nodes;
 	__u16 num_local_nodes;
 	__u16 local_nodes[CLUSTERIP_MAX_NODES];
diff --git a/include/uapi/linux/virtio_net.h b/include/uapi/linux/virtio_net.h
index 227d4ce..172a7f0 100644
--- a/include/uapi/linux/virtio_net.h
+++ b/include/uapi/linux/virtio_net.h
@@ -60,7 +60,7 @@
 
 struct virtio_net_config {
 	/* The config defining mac address (if VIRTIO_NET_F_MAC) */
-	__u8 mac[6];
+	__u8 mac[ETH_ALEN];
 	/* See VIRTIO_NET_F_STATUS and VIRTIO_NET_S_* above */
 	__u16 status;
 	/* Maximum number of each of transmit and receive queues;
diff --git a/include/uapi/linux/wimax/i2400m.h b/include/uapi/linux/wimax/i2400m.h
index 62d3561..fd198bc 100644
--- a/include/uapi/linux/wimax/i2400m.h
+++ b/include/uapi/linux/wimax/i2400m.h
@@ -122,7 +122,7 @@
 #define __LINUX__WIMAX__I2400M_H__
 
 #include <linux/types.h>
-
+#include <linux/if_ether.h>
 
 /*
  * Host Device Interface (HDI) common to all busses
@@ -487,7 +487,7 @@ struct i2400m_tlv_l4_message_versions {
 struct i2400m_tlv_detailed_device_info {
 	struct i2400m_tlv_hdr hdr;
 	__u8 reserved1[400];
-	__u8 mac_address[6];
+	__u8 mac_address[ETH_ALEN];
 	__u8 reserved2[2];
 } __attribute__((packed));
 
-- 
1.8.1.2.459.gbcd45b4.dirty

^ permalink raw reply related

* [PATCH V2 2/3] include: Convert ethernet mac address declarations to use ETH_ALEN
From: Joe Perches @ 2013-08-01 20:14 UTC (permalink / raw)
  To: netdev
  Cc: Steve Glendinning, Samuel Ortiz, linux-usb, linux-kernel,
	Vitaly Bordug, linux-media, linuxppc-dev, David S. Miller,
	Mauro Carvalho Chehab
In-Reply-To: <cover.1375387593.git.joe@perches.com>

It's convenient to have ethernet mac addresses use
ETH_ALEN to be able to grep for them a bit easier and
also to ensure that the addresses are __aligned(2).

Add #include <linux/if_ether.h> as necessary.

Signed-off-by: Joe Perches <joe@perches.com>
Acked-by: Mauro Carvalho Chehab <m.chehab@samsung.com>
---
 include/linux/dm9000.h          |  4 ++-
 include/linux/fs_enet_pd.h      |  3 ++-
 include/linux/ieee80211.h       | 59 +++++++++++++++++++++--------------------
 include/linux/mlx4/device.h     | 11 ++++----
 include/linux/mlx4/qp.h         |  5 ++--
 include/linux/mv643xx_eth.h     |  3 ++-
 include/linux/sh_eth.h          |  3 ++-
 include/linux/smsc911x.h        |  3 ++-
 include/linux/uwb/spec.h        |  5 ++--
 include/media/tveeprom.h        |  4 ++-
 include/net/irda/irlan_common.h |  3 ++-
 11 files changed, 58 insertions(+), 45 deletions(-)

diff --git a/include/linux/dm9000.h b/include/linux/dm9000.h
index 96e8769..841925f 100644
--- a/include/linux/dm9000.h
+++ b/include/linux/dm9000.h
@@ -14,6 +14,8 @@
 #ifndef __DM9000_PLATFORM_DATA
 #define __DM9000_PLATFORM_DATA __FILE__
 
+#include <linux/if_ether.h>
+
 /* IO control flags */
 
 #define DM9000_PLATF_8BITONLY	(0x0001)
@@ -27,7 +29,7 @@
 
 struct dm9000_plat_data {
 	unsigned int	flags;
-	unsigned char	dev_addr[6];
+	unsigned char	dev_addr[ETH_ALEN];
 
 	/* allow replacement IO routines */
 
diff --git a/include/linux/fs_enet_pd.h b/include/linux/fs_enet_pd.h
index 51b7934..343d82a 100644
--- a/include/linux/fs_enet_pd.h
+++ b/include/linux/fs_enet_pd.h
@@ -18,6 +18,7 @@
 
 #include <linux/string.h>
 #include <linux/of_mdio.h>
+#include <linux/if_ether.h>
 #include <asm/types.h>
 
 #define FS_ENET_NAME	"fs_enet"
@@ -135,7 +136,7 @@ struct fs_platform_info {
 	const struct fs_mii_bus_info *bus_info;
 
 	int rx_ring, tx_ring;	/* number of buffers on rx     */
-	__u8 macaddr[6];	/* mac address                 */
+	__u8 macaddr[ETH_ALEN];	/* mac address                 */
 	int rx_copybreak;	/* limit we copy small frames  */
 	int use_napi;		/* use NAPI                    */
 	int napi_weight;	/* NAPI weight                 */
diff --git a/include/linux/ieee80211.h b/include/linux/ieee80211.h
index b0dc87a..4e101af 100644
--- a/include/linux/ieee80211.h
+++ b/include/linux/ieee80211.h
@@ -16,6 +16,7 @@
 #define LINUX_IEEE80211_H
 
 #include <linux/types.h>
+#include <linux/if_ether.h>
 #include <asm/byteorder.h>
 
 /*
@@ -209,28 +210,28 @@ static inline u16 ieee80211_sn_sub(u16 sn1, u16 sn2)
 struct ieee80211_hdr {
 	__le16 frame_control;
 	__le16 duration_id;
-	u8 addr1[6];
-	u8 addr2[6];
-	u8 addr3[6];
+	u8 addr1[ETH_ALEN];
+	u8 addr2[ETH_ALEN];
+	u8 addr3[ETH_ALEN];
 	__le16 seq_ctrl;
-	u8 addr4[6];
+	u8 addr4[ETH_ALEN];
 } __packed __aligned(2);
 
 struct ieee80211_hdr_3addr {
 	__le16 frame_control;
 	__le16 duration_id;
-	u8 addr1[6];
-	u8 addr2[6];
-	u8 addr3[6];
+	u8 addr1[ETH_ALEN];
+	u8 addr2[ETH_ALEN];
+	u8 addr3[ETH_ALEN];
 	__le16 seq_ctrl;
 } __packed __aligned(2);
 
 struct ieee80211_qos_hdr {
 	__le16 frame_control;
 	__le16 duration_id;
-	u8 addr1[6];
-	u8 addr2[6];
-	u8 addr3[6];
+	u8 addr1[ETH_ALEN];
+	u8 addr2[ETH_ALEN];
+	u8 addr3[ETH_ALEN];
 	__le16 seq_ctrl;
 	__le16 qos_ctrl;
 } __packed __aligned(2);
@@ -608,8 +609,8 @@ struct ieee80211s_hdr {
 	u8 flags;
 	u8 ttl;
 	__le32 seqnum;
-	u8 eaddr1[6];
-	u8 eaddr2[6];
+	u8 eaddr1[ETH_ALEN];
+	u8 eaddr2[ETH_ALEN];
 } __packed __aligned(2);
 
 /* Mesh flags */
@@ -758,7 +759,7 @@ struct ieee80211_rann_ie {
 	u8 rann_flags;
 	u8 rann_hopcount;
 	u8 rann_ttl;
-	u8 rann_addr[6];
+	u8 rann_addr[ETH_ALEN];
 	__le32 rann_seq;
 	__le32 rann_interval;
 	__le32 rann_metric;
@@ -802,9 +803,9 @@ enum ieee80211_vht_opmode_bits {
 struct ieee80211_mgmt {
 	__le16 frame_control;
 	__le16 duration;
-	u8 da[6];
-	u8 sa[6];
-	u8 bssid[6];
+	u8 da[ETH_ALEN];
+	u8 sa[ETH_ALEN];
+	u8 bssid[ETH_ALEN];
 	__le16 seq_ctrl;
 	union {
 		struct {
@@ -833,7 +834,7 @@ struct ieee80211_mgmt {
 		struct {
 			__le16 capab_info;
 			__le16 listen_interval;
-			u8 current_ap[6];
+			u8 current_ap[ETH_ALEN];
 			/* followed by SSID and Supported rates */
 			u8 variable[0];
 		} __packed reassoc_req;
@@ -966,21 +967,21 @@ struct ieee80211_vendor_ie {
 struct ieee80211_rts {
 	__le16 frame_control;
 	__le16 duration;
-	u8 ra[6];
-	u8 ta[6];
+	u8 ra[ETH_ALEN];
+	u8 ta[ETH_ALEN];
 } __packed __aligned(2);
 
 struct ieee80211_cts {
 	__le16 frame_control;
 	__le16 duration;
-	u8 ra[6];
+	u8 ra[ETH_ALEN];
 } __packed __aligned(2);
 
 struct ieee80211_pspoll {
 	__le16 frame_control;
 	__le16 aid;
-	u8 bssid[6];
-	u8 ta[6];
+	u8 bssid[ETH_ALEN];
+	u8 ta[ETH_ALEN];
 } __packed __aligned(2);
 
 /* TDLS */
@@ -989,14 +990,14 @@ struct ieee80211_pspoll {
 struct ieee80211_tdls_lnkie {
 	u8 ie_type; /* Link Identifier IE */
 	u8 ie_len;
-	u8 bssid[6];
-	u8 init_sta[6];
-	u8 resp_sta[6];
+	u8 bssid[ETH_ALEN];
+	u8 init_sta[ETH_ALEN];
+	u8 resp_sta[ETH_ALEN];
 } __packed;
 
 struct ieee80211_tdls_data {
-	u8 da[6];
-	u8 sa[6];
+	u8 da[ETH_ALEN];
+	u8 sa[ETH_ALEN];
 	__be16 ether_type;
 	u8 payload_type;
 	u8 category;
@@ -1090,8 +1091,8 @@ struct ieee80211_p2p_noa_attr {
 struct ieee80211_bar {
 	__le16 frame_control;
 	__le16 duration;
-	__u8 ra[6];
-	__u8 ta[6];
+	__u8 ra[ETH_ALEN];
+	__u8 ta[ETH_ALEN];
 	__le16 control;
 	__le16 start_seq_num;
 } __packed;
diff --git a/include/linux/mlx4/device.h b/include/linux/mlx4/device.h
index 6aebdfe..09ef2f4 100644
--- a/include/linux/mlx4/device.h
+++ b/include/linux/mlx4/device.h
@@ -33,6 +33,7 @@
 #ifndef MLX4_DEVICE_H
 #define MLX4_DEVICE_H
 
+#include <linux/if_ether.h>
 #include <linux/pci.h>
 #include <linux/completion.h>
 #include <linux/radix-tree.h>
@@ -620,7 +621,7 @@ struct mlx4_eth_av {
 	u8		dgid[16];
 	u32		reserved4[2];
 	__be16		vlan;
-	u8		mac[6];
+	u8		mac[ETH_ALEN];
 };
 
 union mlx4_ext_av {
@@ -914,10 +915,10 @@ enum mlx4_net_trans_promisc_mode {
 };
 
 struct mlx4_spec_eth {
-	u8	dst_mac[6];
-	u8	dst_mac_msk[6];
-	u8	src_mac[6];
-	u8	src_mac_msk[6];
+	u8	dst_mac[ETH_ALEN];
+	u8	dst_mac_msk[ETH_ALEN];
+	u8	src_mac[ETH_ALEN];
+	u8	src_mac_msk[ETH_ALEN];
 	u8	ether_type_enable;
 	__be16	ether_type;
 	__be16	vlan_id_msk;
diff --git a/include/linux/mlx4/qp.h b/include/linux/mlx4/qp.h
index 262deac..6d35147 100644
--- a/include/linux/mlx4/qp.h
+++ b/include/linux/mlx4/qp.h
@@ -34,6 +34,7 @@
 #define MLX4_QP_H
 
 #include <linux/types.h>
+#include <linux/if_ether.h>
 
 #include <linux/mlx4/device.h>
 
@@ -143,7 +144,7 @@ struct mlx4_qp_path {
 	u8			feup;
 	u8			fvl_rx;
 	u8			reserved4[2];
-	u8			dmac[6];
+	u8			dmac[ETH_ALEN];
 };
 
 enum { /* fl */
@@ -318,7 +319,7 @@ struct mlx4_wqe_datagram_seg {
 	__be32			dqpn;
 	__be32			qkey;
 	__be16			vlan;
-	u8			mac[6];
+	u8			mac[ETH_ALEN];
 };
 
 struct mlx4_wqe_lso_seg {
diff --git a/include/linux/mv643xx_eth.h b/include/linux/mv643xx_eth.h
index 6e8215b..61a0da3 100644
--- a/include/linux/mv643xx_eth.h
+++ b/include/linux/mv643xx_eth.h
@@ -6,6 +6,7 @@
 #define __LINUX_MV643XX_ETH_H
 
 #include <linux/mbus.h>
+#include <linux/if_ether.h>
 
 #define MV643XX_ETH_SHARED_NAME		"mv643xx_eth"
 #define MV643XX_ETH_NAME		"mv643xx_eth_port"
@@ -48,7 +49,7 @@ struct mv643xx_eth_platform_data {
 	 * Use this MAC address if it is valid, overriding the
 	 * address that is already in the hardware.
 	 */
-	u8			mac_addr[6];
+	u8			mac_addr[ETH_ALEN];
 
 	/*
 	 * If speed is 0, autonegotiation is enabled.
diff --git a/include/linux/sh_eth.h b/include/linux/sh_eth.h
index fc30571..6205eeb 100644
--- a/include/linux/sh_eth.h
+++ b/include/linux/sh_eth.h
@@ -2,6 +2,7 @@
 #define __ASM_SH_ETH_H__
 
 #include <linux/phy.h>
+#include <linux/if_ether.h>
 
 enum {EDMAC_LITTLE_ENDIAN, EDMAC_BIG_ENDIAN};
 enum {
@@ -18,7 +19,7 @@ struct sh_eth_plat_data {
 	phy_interface_t phy_interface;
 	void (*set_mdio_gate)(void *addr);
 
-	unsigned char mac_addr[6];
+	unsigned char mac_addr[ETH_ALEN];
 	unsigned no_ether_link:1;
 	unsigned ether_link_active_low:1;
 	unsigned needs_init:1;
diff --git a/include/linux/smsc911x.h b/include/linux/smsc911x.h
index 4dde70e..eec3efd 100644
--- a/include/linux/smsc911x.h
+++ b/include/linux/smsc911x.h
@@ -22,6 +22,7 @@
 #define __LINUX_SMSC911X_H__
 
 #include <linux/phy.h>
+#include <linux/if_ether.h>
 
 /* platform_device configuration data, should be assigned to
  * the platform_device's dev.platform_data */
@@ -31,7 +32,7 @@ struct smsc911x_platform_config {
 	unsigned int flags;
 	unsigned int shift;
 	phy_interface_t phy_interface;
-	unsigned char mac[6];
+	unsigned char mac[ETH_ALEN];
 };
 
 /* Constants for platform_device irq polarity configuration */
diff --git a/include/linux/uwb/spec.h b/include/linux/uwb/spec.h
index b52e44f..0df24bf 100644
--- a/include/linux/uwb/spec.h
+++ b/include/linux/uwb/spec.h
@@ -32,6 +32,7 @@
 
 #include <linux/types.h>
 #include <linux/bitmap.h>
+#include <linux/if_ether.h>
 
 #define i1480_FW 0x00000303
 /* #define i1480_FW 0x00000302 */
@@ -130,7 +131,7 @@ enum { UWB_DRP_BACKOFF_WIN_MAX = 16 };
  * it is also used to define headers sent down and up the wire/radio).
  */
 struct uwb_mac_addr {
-	u8 data[6];
+	u8 data[ETH_ALEN];
 } __attribute__((packed));
 
 
@@ -568,7 +569,7 @@ struct uwb_rc_evt_confirm {
 /* Device Address Management event. [WHCI] section 3.1.3.2. */
 struct uwb_rc_evt_dev_addr_mgmt {
 	struct uwb_rceb rceb;
-	u8 baAddr[6];
+	u8 baAddr[ETH_ALEN];
 	u8 bResultCode;
 } __attribute__((packed));
 
diff --git a/include/media/tveeprom.h b/include/media/tveeprom.h
index 4a1191a..f7119ee 100644
--- a/include/media/tveeprom.h
+++ b/include/media/tveeprom.h
@@ -12,6 +12,8 @@ enum tveeprom_audio_processor {
 	TVEEPROM_AUDPROC_OTHER,
 };
 
+#include <linux/if_ether.h>
+
 struct tveeprom {
 	u32 has_radio;
 	/* If has_ir == 0, then it is unknown what the IR capabilities are,
@@ -40,7 +42,7 @@ struct tveeprom {
 	u32 revision;
 	u32 serial_number;
 	char rev_str[5];
-	u8 MAC_address[6];
+	u8 MAC_address[ETH_ALEN];
 };
 
 void tveeprom_hauppauge_analog(struct i2c_client *c, struct tveeprom *tvee,
diff --git a/include/net/irda/irlan_common.h b/include/net/irda/irlan_common.h
index 0af8b8d..550c2d6 100644
--- a/include/net/irda/irlan_common.h
+++ b/include/net/irda/irlan_common.h
@@ -32,6 +32,7 @@
 #include <linux/types.h>
 #include <linux/skbuff.h>
 #include <linux/netdevice.h>
+#include <linux/if_ether.h>
 
 #include <net/irda/irttp.h>
 
@@ -161,7 +162,7 @@ struct irlan_provider_cb {
 	int access_type;     /* Access type */
 	__u16 send_arb_val;
 
-	__u8 mac_address[6]; /* Generated MAC address for peer device */
+	__u8 mac_address[ETH_ALEN]; /* Generated MAC address for peer device */
 };
 
 /*
-- 
1.8.1.2.459.gbcd45b4.dirty

^ permalink raw reply related

* [PATCH V2 3/3] ethernet: Convert mac address uses of 6 to ETH_ALEN
From: Joe Perches @ 2013-08-01 20:14 UTC (permalink / raw)
  To: netdev
  Cc: Sam Creasey, Grant Grundler, Wan ZongShun, Rajesh Borundia,
	Sony Chacko, linux-kernel, Manish Chopra, linux-driver, Don Fry,
	Ron Mercer, Andrew Gallatin, Olof Johansson, Jitendra Kalsaria,
	linux-arm-kernel, Shahed Shaikh
In-Reply-To: <cover.1375387593.git.joe@perches.com>

Use the normal #define to help grep find mac addresses
and ensure that addresses are aligned.

Move one address in pasemi_mac.h for alignment.

Signed-off-by: Joe Perches <joe@perches.com>
---
 drivers/net/ethernet/8390/ax88796.c                |  4 ++--
 drivers/net/ethernet/amd/pcnet32.c                 |  6 +++---
 drivers/net/ethernet/broadcom/cnic_if.h            |  6 +++---
 drivers/net/ethernet/dec/tulip/tulip_core.c        |  8 +++++---
 drivers/net/ethernet/i825xx/sun3_82586.h           |  4 ++--
 drivers/net/ethernet/myricom/myri10ge/myri10ge.c   |  2 +-
 drivers/net/ethernet/nuvoton/w90p910_ether.c       |  4 ++--
 drivers/net/ethernet/pasemi/pasemi_mac.c           | 13 +++++++------
 drivers/net/ethernet/pasemi/pasemi_mac.h           |  4 ++--
 drivers/net/ethernet/qlogic/netxen/netxen_nic_hw.c |  4 ++--
 drivers/net/ethernet/qlogic/qlge/qlge.h            |  2 +-
 11 files changed, 30 insertions(+), 27 deletions(-)

diff --git a/drivers/net/ethernet/8390/ax88796.c b/drivers/net/ethernet/8390/ax88796.c
index e1d2643..b7232a9 100644
--- a/drivers/net/ethernet/8390/ax88796.c
+++ b/drivers/net/ethernet/8390/ax88796.c
@@ -707,7 +707,7 @@ static int ax_init_dev(struct net_device *dev)
 
 #ifdef CONFIG_AX88796_93CX6
 	if (ax->plat->flags & AXFLG_HAS_93CX6) {
-		unsigned char mac_addr[6];
+		unsigned char mac_addr[ETH_ALEN];
 		struct eeprom_93cx6 eeprom;
 
 		eeprom.data = ei_local;
@@ -719,7 +719,7 @@ static int ax_init_dev(struct net_device *dev)
 				       (__le16 __force *)mac_addr,
 				       sizeof(mac_addr) >> 1);
 
-		memcpy(dev->dev_addr, mac_addr, 6);
+		memcpy(dev->dev_addr, mac_addr, ETH_ALEN);
 	}
 #endif
 	if (ax->plat->wordlength == 2) {
diff --git a/drivers/net/ethernet/amd/pcnet32.c b/drivers/net/ethernet/amd/pcnet32.c
index ed21307..2d8e288 100644
--- a/drivers/net/ethernet/amd/pcnet32.c
+++ b/drivers/net/ethernet/amd/pcnet32.c
@@ -1521,7 +1521,7 @@ pcnet32_probe1(unsigned long ioaddr, int shared, struct pci_dev *pdev)
 	char *chipname;
 	struct net_device *dev;
 	const struct pcnet32_access *a = NULL;
-	u8 promaddr[6];
+	u8 promaddr[ETH_ALEN];
 	int ret = -ENODEV;
 
 	/* reset the chip */
@@ -1665,10 +1665,10 @@ pcnet32_probe1(unsigned long ioaddr, int shared, struct pci_dev *pdev)
 	}
 
 	/* read PROM address and compare with CSR address */
-	for (i = 0; i < 6; i++)
+	for (i = 0; i < ETH_ALEN; i++)
 		promaddr[i] = inb(ioaddr + i);
 
-	if (memcmp(promaddr, dev->dev_addr, 6) ||
+	if (memcmp(promaddr, dev->dev_addr, ETH_ALEN) ||
 	    !is_valid_ether_addr(dev->dev_addr)) {
 		if (is_valid_ether_addr(promaddr)) {
 			if (pcnet32_debug & NETIF_MSG_PROBE) {
diff --git a/drivers/net/ethernet/broadcom/cnic_if.h b/drivers/net/ethernet/broadcom/cnic_if.h
index 82a0312..95aff76 100644
--- a/drivers/net/ethernet/broadcom/cnic_if.h
+++ b/drivers/net/ethernet/broadcom/cnic_if.h
@@ -238,8 +238,8 @@ struct cnic_sock {
 	u16	src_port;
 	u16	dst_port;
 	u16	vlan_id;
-	unsigned char old_ha[6];
-	unsigned char ha[6];
+	unsigned char old_ha[ETH_ALEN];
+	unsigned char ha[ETH_ALEN];
 	u32	mtu;
 	u32	cid;
 	u32	l5_cid;
@@ -308,7 +308,7 @@ struct cnic_dev {
 #define CNIC_F_BNX2_CLASS	3
 #define CNIC_F_BNX2X_CLASS	4
 	atomic_t	ref_count;
-	u8		mac_addr[6];
+	u8		mac_addr[ETH_ALEN];
 
 	int		max_iscsi_conn;
 	int		max_fcoe_conn;
diff --git a/drivers/net/ethernet/dec/tulip/tulip_core.c b/drivers/net/ethernet/dec/tulip/tulip_core.c
index c94152f..4e8cfa2 100644
--- a/drivers/net/ethernet/dec/tulip/tulip_core.c
+++ b/drivers/net/ethernet/dec/tulip/tulip_core.c
@@ -1304,7 +1304,9 @@ static int tulip_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
 {
 	struct tulip_private *tp;
 	/* See note below on the multiport cards. */
-	static unsigned char last_phys_addr[6] = {0x00, 'L', 'i', 'n', 'u', 'x'};
+	static unsigned char last_phys_addr[ETH_ALEN] = {
+		0x00, 'L', 'i', 'n', 'u', 'x'
+	};
 	static int last_irq;
 	static int multiport_cnt;	/* For four-port boards w/one EEPROM */
 	int i, irq;
@@ -1627,8 +1629,8 @@ static int tulip_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
 		dev->dev_addr[i] = last_phys_addr[i] + 1;
 #if defined(CONFIG_SPARC)
 		addr = of_get_property(dp, "local-mac-address", &len);
-		if (addr && len == 6)
-			memcpy(dev->dev_addr, addr, 6);
+		if (addr && len == ETH_ALEN)
+			memcpy(dev->dev_addr, addr, ETH_ALEN);
 #endif
 #if defined(__i386__) || defined(__x86_64__)	/* Patch up x86 BIOS bug. */
 		if (last_irq)
diff --git a/drivers/net/ethernet/i825xx/sun3_82586.h b/drivers/net/ethernet/i825xx/sun3_82586.h
index 93346f0..79aef68 100644
--- a/drivers/net/ethernet/i825xx/sun3_82586.h
+++ b/drivers/net/ethernet/i825xx/sun3_82586.h
@@ -133,8 +133,8 @@ struct rfd_struct
   unsigned char  last;		/* Bit15,Last Frame on List / Bit14,suspend */
   unsigned short next;		/* linkoffset to next RFD */
   unsigned short rbd_offset;	/* pointeroffset to RBD-buffer */
-  unsigned char  dest[6];	/* ethernet-address, destination */
-  unsigned char  source[6];	/* ethernet-address, source */
+  unsigned char  dest[ETH_ALEN];	/* ethernet-address, destination */
+  unsigned char  source[ETH_ALEN];	/* ethernet-address, source */
   unsigned short length;	/* 802.3 frame-length */
   unsigned short zero_dummy;	/* dummy */
 };
diff --git a/drivers/net/ethernet/myricom/myri10ge/myri10ge.c b/drivers/net/ethernet/myricom/myri10ge/myri10ge.c
index 967bae8..d4cdf4d 100644
--- a/drivers/net/ethernet/myricom/myri10ge/myri10ge.c
+++ b/drivers/net/ethernet/myricom/myri10ge/myri10ge.c
@@ -244,7 +244,7 @@ struct myri10ge_priv {
 	int fw_ver_minor;
 	int fw_ver_tiny;
 	int adopted_rx_filter_bug;
-	u8 mac_addr[6];		/* eeprom mac address */
+	u8 mac_addr[ETH_ALEN];		/* eeprom mac address */
 	unsigned long serial_number;
 	int vendor_specific_offset;
 	int fw_multicast_support;
diff --git a/drivers/net/ethernet/nuvoton/w90p910_ether.c b/drivers/net/ethernet/nuvoton/w90p910_ether.c
index e88bdb1..dcfe58f 100644
--- a/drivers/net/ethernet/nuvoton/w90p910_ether.c
+++ b/drivers/net/ethernet/nuvoton/w90p910_ether.c
@@ -922,7 +922,7 @@ static void __init get_mac_address(struct net_device *dev)
 {
 	struct w90p910_ether *ether = netdev_priv(dev);
 	struct platform_device *pdev;
-	char addr[6];
+	char addr[ETH_ALEN];
 
 	pdev = ether->pdev;
 
@@ -934,7 +934,7 @@ static void __init get_mac_address(struct net_device *dev)
 	addr[5] = 0xa8;
 
 	if (is_valid_ether_addr(addr))
-		memcpy(dev->dev_addr, &addr, 0x06);
+		memcpy(dev->dev_addr, &addr, ETH_ALEN);
 	else
 		dev_err(&pdev->dev, "invalid mac address\n");
 }
diff --git a/drivers/net/ethernet/pasemi/pasemi_mac.c b/drivers/net/ethernet/pasemi/pasemi_mac.c
index a5f0b5d..f21ae7b 100644
--- a/drivers/net/ethernet/pasemi/pasemi_mac.c
+++ b/drivers/net/ethernet/pasemi/pasemi_mac.c
@@ -191,7 +191,7 @@ static int pasemi_get_mac_addr(struct pasemi_mac *mac)
 	struct device_node *dn = pci_device_to_OF_node(pdev);
 	int len;
 	const u8 *maddr;
-	u8 addr[6];
+	u8 addr[ETH_ALEN];
 
 	if (!dn) {
 		dev_dbg(&pdev->dev,
@@ -201,8 +201,8 @@ static int pasemi_get_mac_addr(struct pasemi_mac *mac)
 
 	maddr = of_get_property(dn, "local-mac-address", &len);
 
-	if (maddr && len == 6) {
-		memcpy(mac->mac_addr, maddr, 6);
+	if (maddr && len == ETH_ALEN) {
+		memcpy(mac->mac_addr, maddr, ETH_ALEN);
 		return 0;
 	}
 
@@ -219,14 +219,15 @@ static int pasemi_get_mac_addr(struct pasemi_mac *mac)
 		return -ENOENT;
 	}
 
-	if (sscanf(maddr, "%hhx:%hhx:%hhx:%hhx:%hhx:%hhx", &addr[0],
-		   &addr[1], &addr[2], &addr[3], &addr[4], &addr[5]) != 6) {
+	if (sscanf(maddr, "%hhx:%hhx:%hhx:%hhx:%hhx:%hhx",
+		   &addr[0], &addr[1], &addr[2], &addr[3], &addr[4], &addr[5])
+	    != ETH_ALEN) {
 		dev_warn(&pdev->dev,
 			 "can't parse mac address, not configuring\n");
 		return -EINVAL;
 	}
 
-	memcpy(mac->mac_addr, addr, 6);
+	memcpy(mac->mac_addr, addr, ETH_ALEN);
 
 	return 0;
 }
diff --git a/drivers/net/ethernet/pasemi/pasemi_mac.h b/drivers/net/ethernet/pasemi/pasemi_mac.h
index e2f4efa..649fdb4 100644
--- a/drivers/net/ethernet/pasemi/pasemi_mac.h
+++ b/drivers/net/ethernet/pasemi/pasemi_mac.h
@@ -79,11 +79,11 @@ struct pasemi_mac {
 	int		last_cs;
 	int		num_cs;
 	u32		dma_if;
-	u8		type;
 #define MAC_TYPE_GMAC	1
 #define MAC_TYPE_XAUI	2
 
-	u8		mac_addr[6];
+	u8		mac_addr[ETH_ALEN];
+	u8		type;
 
 	struct net_lro_mgr	lro_mgr;
 	struct net_lro_desc	lro_desc[MAX_LRO_DESCRIPTORS];
diff --git a/drivers/net/ethernet/qlogic/netxen/netxen_nic_hw.c b/drivers/net/ethernet/qlogic/netxen/netxen_nic_hw.c
index 9fbb1cd..8375cbd 100644
--- a/drivers/net/ethernet/qlogic/netxen/netxen_nic_hw.c
+++ b/drivers/net/ethernet/qlogic/netxen/netxen_nic_hw.c
@@ -536,10 +536,10 @@ static void netxen_p2_nic_set_multi(struct net_device *netdev)
 {
 	struct netxen_adapter *adapter = netdev_priv(netdev);
 	struct netdev_hw_addr *ha;
-	u8 null_addr[6];
+	u8 null_addr[ETH_ALEN];
 	int i;
 
-	memset(null_addr, 0, 6);
+	memset(null_addr, 0, ETH_ALEN);
 
 	if (netdev->flags & IFF_PROMISC) {
 
diff --git a/drivers/net/ethernet/qlogic/qlge/qlge.h b/drivers/net/ethernet/qlogic/qlge/qlge.h
index 7e8d682..8994337 100644
--- a/drivers/net/ethernet/qlogic/qlge/qlge.h
+++ b/drivers/net/ethernet/qlogic/qlge/qlge.h
@@ -2149,7 +2149,7 @@ struct ql_adapter {
 	struct timer_list timer;
 	atomic_t lb_count;
 	/* Keep local copy of current mac address. */
-	char current_mac_addr[6];
+	char current_mac_addr[ETH_ALEN];
 };
 
 /*
-- 
1.8.1.2.459.gbcd45b4.dirty

^ permalink raw reply related

* Re: [PATCH v2] sis900: Fix the tx queue timeout issue
From: David Miller @ 2013-08-01 20:15 UTC (permalink / raw)
  To: B38611; +Cc: kda, venza, netdev
In-Reply-To: <9848F2DB572E5649BA045B288BE08FBE015E5576@039-SN2MPN1-023.039d.mgd.msft.net>

From: Duan Fugang-B38611 <B38611@freescale.com>
Date: Thu, 1 Aug 2013 15:50:51 +0000

> On Thursday, August 01, 2013 at 3:01 PM, Denis Kirjanov wrote:
> 
>> @@ -1613,9 +1613,12 @@ sis900_start_xmit(struct sk_buff *skb, struct net_device *net_dev)
>> 	unsigned int  count_dirty_tx;
>> 
>> 	/* Don't transmit data before the complete of auto-negotiation */
>> -	if(!sis_priv->autong_complete){
>> -		netif_stop_queue(net_dev);
>> -		return NETDEV_TX_BUSY;
>> +	if (unlikely(!sis_priv->autong_complete)) {
>> +		if (netif_msg_tx_err(sis_priv) && net_ratelimit())
>> +			netdev_dbg(net_dev, "Auto negotiation in progress\n");
>> +		net_dev->stats.tx_dropped++;
>> +		dev_kfree_skb(skb);
>> +		return NETDEV_TX_OK;
>>  	}
> 
> I always insist on that tx queue timeout is caused by un-correctly phy link handler flow.
> The link up handler flow: Auto-negotiation done-> netif_carrier_on()
>  
> The issue cannot be processed in xmit callback, which must be handled in link adjust process.

I completely agree, if sis_priv->autonet_complete is false this driver _MUST_
keep netif_carrier_off() until it becomes true.

This patch is not the correct fix for this problem, please fix it correctly.

^ permalink raw reply

* Re: [PATCH][net-next] gianfar: Remove unused field grp_id from gfar_priv_grp
From: David Miller @ 2013-08-01 20:16 UTC (permalink / raw)
  To: claudiu.manoil; +Cc: netdev
In-Reply-To: <1375340825-6524-1-git-send-email-claudiu.manoil@freescale.com>

From: Claudiu Manoil <claudiu.manoil@freescale.com>
Date: Thu, 1 Aug 2013 10:07:05 +0300

> grp->grp_id is obsolete. It has no use in the current driver.
> Remove it from gfar_priv_grp and put the 'rstat' member
> in its place, in the 2nd cache line, as rstat needs fast access.
> 
> Signed-off-by: Claudiu Manoil <claudiu.manoil@freescale.com>

Looks good, applied, thanks.

^ permalink raw reply

* Re: bonding + arp monitoring fails if interface is a vlan
From: Veaceslav Falico @ 2013-08-01 20:21 UTC (permalink / raw)
  To: Santiago Garcia Mantinan; +Cc: netdev
In-Reply-To: <20130801121142.GA444@www.manty.net>

On Thu, Aug 1, 2013 at 2:11 PM, Santiago Garcia Mantinan
<manty@manty.net> wrote:
> Hi!
...snip...
>
> This is the setup on the bonding interface.
>
> auto bond0
> iface bond0 inet static
>         address 192.168.1.2
>         netmask 255.255.255.0
>         bond-slaves eth0.1002
>         bond-mode active-backup
>         bond-arp_validate 0

Could you please try with arp_validate=1?

^ permalink raw reply

* Re: [PATCH V2 3/3] ethernet: Convert mac address uses of 6 to ETH_ALEN
From: Olof Johansson @ 2013-08-01 20:31 UTC (permalink / raw)
  To: Joe Perches
  Cc: Network Development, Don Fry, Grant Grundler, Sam Creasey,
	Andrew Gallatin, Wan ZongShun, Manish Chopra, Sony Chacko,
	Rajesh Borundia, Shahed Shaikh, Jitendra Kalsaria, Ron Mercer,
	linux-driver, linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org
In-Reply-To: <4170130535e4ea14c66f8510042577be21941f4d.1375387593.git.joe@perches.com>

Hi,

On Thu, Aug 1, 2013 at 1:14 PM, Joe Perches <joe@perches.com> wrote:

> diff --git a/drivers/net/ethernet/pasemi/pasemi_mac.h b/drivers/net/ethernet/pasemi/pasemi_mac.h
> index e2f4efa..649fdb4 100644
> --- a/drivers/net/ethernet/pasemi/pasemi_mac.h
> +++ b/drivers/net/ethernet/pasemi/pasemi_mac.h
> @@ -79,11 +79,11 @@ struct pasemi_mac {
>         int             last_cs;
>         int             num_cs;
>         u32             dma_if;
> -       u8              type;
>  #define MAC_TYPE_GMAC  1
>  #define MAC_TYPE_XAUI  2
>
> -       u8              mac_addr[6];
> +       u8              mac_addr[ETH_ALEN];
> +       u8              type;

Just promote 'type' to u32 instead, saves you from moving the #defines
down, etc, etc.


-Olof

^ permalink raw reply

* Re: [PATCH V2 3/3] ethernet: Convert mac address uses of 6 to ETH_ALEN
From: Russell King - ARM Linux @ 2013-08-01 20:33 UTC (permalink / raw)
  To: Olof Johansson
  Cc: Joe Perches, Sam Creasey, Grant Grundler, Wan ZongShun,
	Rajesh Borundia, Network Development, Sony Chacko,
	linux-kernel@vger.kernel.org, Manish Chopra, Don Fry, Ron Mercer,
	Andrew Gallatin, linux-driver, Jitendra Kalsaria,
	linux-arm-kernel@lists.infradead.org, Shahed Shaikh
In-Reply-To: <CAOesGMhmxJTDjcPeD4b1FvRsbP8df369F01SQBK6dk4nXmRGjA@mail.gmail.com>

On Thu, Aug 01, 2013 at 01:31:09PM -0700, Olof Johansson wrote:
> Hi,
> 
> On Thu, Aug 1, 2013 at 1:14 PM, Joe Perches <joe@perches.com> wrote:
> 
> > diff --git a/drivers/net/ethernet/pasemi/pasemi_mac.h b/drivers/net/ethernet/pasemi/pasemi_mac.h
> > index e2f4efa..649fdb4 100644
> > --- a/drivers/net/ethernet/pasemi/pasemi_mac.h
> > +++ b/drivers/net/ethernet/pasemi/pasemi_mac.h
> > @@ -79,11 +79,11 @@ struct pasemi_mac {
> >         int             last_cs;
> >         int             num_cs;
> >         u32             dma_if;
> > -       u8              type;
> >  #define MAC_TYPE_GMAC  1
> >  #define MAC_TYPE_XAUI  2
> >
> > -       u8              mac_addr[6];
> > +       u8              mac_addr[ETH_ALEN];
> > +       u8              type;
> 
> Just promote 'type' to u32 instead, saves you from moving the #defines
> down, etc, etc.

There's a more fundamental question which has to be asked: what is the
point of moving that in the first place?

^ permalink raw reply

* Re: [PATCH 2/4] USB: XHCI: mark no_sg_limit
From: Sarah Sharp @ 2013-08-01 20:47 UTC (permalink / raw)
  To: Oliver Neukum
  Cc: Greg Kroah-Hartman, Ming Lei, David S. Miller, Freddy Xin,
	Eric Dumazet, Ben Hutchings, Grant Grundler, netdev, linux-usb
In-Reply-To: <1375344951.1518.0.camel@linux-fkkt.site>

On Thu, Aug 01, 2013 at 10:15:51AM +0200, Oliver Neukum wrote:
> On Thu, 2013-08-01 at 15:30 +0800, Greg Kroah-Hartman wrote:
> > On Wed, Jul 31, 2013 at 09:40:26AM -0700, Sarah Sharp wrote:
> > > On Wed, Jul 31, 2013 at 06:51:47PM +0800, Ming Lei wrote:
> > > > This patch marks all xHCI controllers as no_sg_limit since
> > > > xHCI supports building packet from discontinuous buffers.
> > > > 
> > > > Cc: Sarah Sharp <sarah.a.sharp@linux.intel.com>
> > > > Signed-off-by: Ming Lei <ming.lei@canonical.com>
> > > 
> > > Acked-by: Sarah Sharp <sarah.a.sharp@linux.intel.com>
> > 
> > Is it a requirement that all xhci controllers support sg?  I know we are
> > starting to see other controllers (the platform code?) so would they
> > need to support this as well?
> 
> The way XHCI describes transfers allows them to be arbitrary.

Yes, Oliver is right.  The xHCI host can handle arbitrary length buffers
chained together into one transfer, and now that we have the xHCI driver
ring expansion code in place, we should be able to make them as long as
the USB core will allow.

Sarah Sharp

^ permalink raw reply

* Re: [PATCH V2 3/3] ethernet: Convert mac address uses of 6 to ETH_ALEN
From: Joe Perches @ 2013-08-01 20:55 UTC (permalink / raw)
  To: Russell King - ARM Linux
  Cc: Olof Johansson, Sam Creasey, Grant Grundler, Wan ZongShun,
	Rajesh Borundia, Network Development, Sony Chacko,
	linux-kernel@vger.kernel.org, Manish Chopra, Don Fry, Ron Mercer,
	Andrew Gallatin, linux-driver, Jitendra Kalsaria,
	linux-arm-kernel@lists.infradead.org, Shahed Shaikh
In-Reply-To: <20130801203343.GG23006@n2100.arm.linux.org.uk>

On Thu, 2013-08-01 at 21:33 +0100, Russell King - ARM Linux wrote:
> On Thu, Aug 01, 2013 at 01:31:09PM -0700, Olof Johansson wrote:
> > Hi,

Hi all.

> > On Thu, Aug 1, 2013 at 1:14 PM, Joe Perches <joe@perches.com> wrote:
> > > diff --git a/drivers/net/ethernet/pasemi/pasemi_mac.h b/drivers/net/ethernet/pasemi/pasemi_mac.h
[]
> > > @@ -79,11 +79,11 @@ struct pasemi_mac {
> > >         int             last_cs;
> > >         int             num_cs;
> > >         u32             dma_if;
> > > -       u8              type;
> > >  #define MAC_TYPE_GMAC  1
> > >  #define MAC_TYPE_XAUI  2
> > >
> > > -       u8              mac_addr[6];
> > > +       u8              mac_addr[ETH_ALEN];
> > > +       u8              type;
> > 
> > Just promote 'type' to u32 instead, saves you from moving the #defines
> > down, etc, etc.

type is already u8, why change it?
That would also change struct size.

> There's a more fundamental question which has to be asked: what is the
> point of moving that in the first place?

Some is_<foo>_ether_addr tests assume __aligned(2)
by a casting char * to u16/be16 * and dereferencing.

see patch 0/3 and include/linux/etherdevice.h

^ permalink raw reply

* Re: [PATCH V2 3/3] ethernet: Convert mac address uses of 6 to ETH_ALEN
From: Russell King - ARM Linux @ 2013-08-01 20:58 UTC (permalink / raw)
  To: Joe Perches
  Cc: Olof Johansson, Sam Creasey, Grant Grundler, Wan ZongShun,
	Rajesh Borundia, Network Development, Sony Chacko,
	linux-kernel@vger.kernel.org, Manish Chopra, Don Fry, Ron Mercer,
	Andrew Gallatin, linux-driver, Jitendra Kalsaria,
	linux-arm-kernel@lists.infradead.org, Shahed Shaikh
In-Reply-To: <1375390528.2034.77.camel@joe-AO722>

On Thu, Aug 01, 2013 at 01:55:28PM -0700, Joe Perches wrote:
> On Thu, 2013-08-01 at 21:33 +0100, Russell King - ARM Linux wrote:
> > On Thu, Aug 01, 2013 at 01:31:09PM -0700, Olof Johansson wrote:
> > > Hi,
> 
> Hi all.
> 
> > > On Thu, Aug 1, 2013 at 1:14 PM, Joe Perches <joe@perches.com> wrote:
> > > > diff --git a/drivers/net/ethernet/pasemi/pasemi_mac.h b/drivers/net/ethernet/pasemi/pasemi_mac.h
> []
> > > > @@ -79,11 +79,11 @@ struct pasemi_mac {
> > > >         int             last_cs;
> > > >         int             num_cs;
> > > >         u32             dma_if;
> > > > -       u8              type;
> > > >  #define MAC_TYPE_GMAC  1
> > > >  #define MAC_TYPE_XAUI  2
> > > >
> > > > -       u8              mac_addr[6];
> > > > +       u8              mac_addr[ETH_ALEN];
> > > > +       u8              type;
> > > 
> > > Just promote 'type' to u32 instead, saves you from moving the #defines
> > > down, etc, etc.
> 
> type is already u8, why change it?
> That would also change struct size.
> 
> > There's a more fundamental question which has to be asked: what is the
> > point of moving that in the first place?
> 
> Some is_<foo>_ether_addr tests assume __aligned(2)
> by a casting char * to u16/be16 * and dereferencing.
> 
> see patch 0/3 and include/linux/etherdevice.h

This seems rather obscure - I mean, it's not obvious to driver authors
that should be the case.  Would it not be better to make this a little
more obvious somehow?  Maybe __aligned(2) against mac_addr?  Or
maybe have a debugging check for it?

^ permalink raw reply

* Re: [PATCH V2 3/3] ethernet: Convert mac address uses of 6 to ETH_ALEN
From: Joe Perches @ 2013-08-01 21:04 UTC (permalink / raw)
  To: Russell King - ARM Linux, David Miller
  Cc: Sam Creasey, Grant Grundler, Wan ZongShun, Network Development,
	Don Fry, Sony Chacko, linux-kernel@vger.kernel.org, Manish Chopra,
	linux-driver, Rajesh Borundia, Ron Mercer, Andrew Gallatin,
	Olof Johansson, Jitendra Kalsaria,
	linux-arm-kernel@lists.infradead.org, Shahed Shaikh
In-Reply-To: <20130801205817.GJ23006@n2100.arm.linux.org.uk>

On Thu, 2013-08-01 at 21:58 +0100, Russell King - ARM Linux wrote:
> On Thu, Aug 01, 2013 at 01:55:28PM -0700, Joe Perches wrote:
> > On Thu, 2013-08-01 at 21:33 +0100, Russell King - ARM Linux wrote:
> > > On Thu, Aug 01, 2013 at 01:31:09PM -0700, Olof Johansson wrote:
> > > > On Thu, Aug 1, 2013 at 1:14 PM, Joe Perches <joe@perches.com> wrote:
> > > > > diff --git a/drivers/net/ethernet/pasemi/pasemi_mac.h b/drivers/net/ethernet/pasemi/pasemi_mac.h
> > []
> > > > > @@ -79,11 +79,11 @@ struct pasemi_mac {
> > > > >         int             last_cs;
> > > > >         int             num_cs;
> > > > >         u32             dma_if;
> > > > > -       u8              type;
> > > > >  #define MAC_TYPE_GMAC  1
> > > > >  #define MAC_TYPE_XAUI  2
> > > > >
> > > > > -       u8              mac_addr[6];
> > > > > +       u8              mac_addr[ETH_ALEN];
> > > > > +       u8              type;
> > > > 
> > > > Just promote 'type' to u32 instead, saves you from moving the #defines
> > > > down, etc, etc.
> > 
> > type is already u8, why change it?
> > That would also change struct size.
> > 
> > > There's a more fundamental question which has to be asked: what is the
> > > point of moving that in the first place?
> > 
> > Some is_<foo>_ether_addr tests assume __aligned(2)
> > by a casting char * to u16/be16 * and dereferencing.
> > 
> > see patch 0/3 and include/linux/etherdevice.h
> 
> This seems rather obscure - I mean, it's not obvious to driver authors
> that should be the case.  Would it not be better to make this a little
> more obvious somehow?  Maybe __aligned(2) against mac_addr?  Or
> maybe have a debugging check for it?

That'd be for David Miller (cc'd).

I believe he's argued in the past that any alignment check
for mac addresses was unnecessary.

For all I know it really might not matter because pasemi
can successfully dereference a ushort against an odd char
pointer.

I just noticed it and thought it'd be better moved.

^ permalink raw reply

* Re: [PATCH V2 3/3] ethernet: Convert mac address uses of 6 to ETH_ALEN
From: Russell King - ARM Linux @ 2013-08-01 21:06 UTC (permalink / raw)
  To: Joe Perches
  Cc: David Miller, Olof Johansson, Sam Creasey, Grant Grundler,
	Wan ZongShun, Rajesh Borundia, Network Development, Sony Chacko,
	linux-kernel@vger.kernel.org, Manish Chopra, Don Fry, Ron Mercer,
	Andrew Gallatin, linux-driver, Jitendra Kalsaria,
	linux-arm-kernel@lists.infradead.org, Shahed Shaikh
In-Reply-To: <1375391087.2034.82.camel@joe-AO722>

On Thu, Aug 01, 2013 at 02:04:47PM -0700, Joe Perches wrote:
> On Thu, 2013-08-01 at 21:58 +0100, Russell King - ARM Linux wrote:
> > This seems rather obscure - I mean, it's not obvious to driver authors
> > that should be the case.  Would it not be better to make this a little
> > more obvious somehow?  Maybe __aligned(2) against mac_addr?  Or
> > maybe have a debugging check for it?
> 
> That'd be for David Miller (cc'd).
> 
> I believe he's argued in the past that any alignment check
> for mac addresses was unnecessary.
> 
> For all I know it really might not matter because pasemi
> can successfully dereference a ushort against an odd char
> pointer.
> 
> I just noticed it and thought it'd be better moved.

As can ARM too for years now - either in hardware or via fixup for
kernel code.

^ permalink raw reply

* Re: [PATCH V2 3/3] ethernet: Convert mac address uses of 6 to ETH_ALEN
From: David Miller @ 2013-08-01 21:10 UTC (permalink / raw)
  To: linux
  Cc: joe, olof, sammy, grundler, mcuos.com, rajesh.borundia, netdev,
	sony.chacko, linux-kernel, manish.chopra, pcnet32, ron.mercer,
	gallatin, linux-driver, jitendra.kalsaria, linux-arm-kernel,
	shahed.shaikh
In-Reply-To: <20130801210618.GK23006@n2100.arm.linux.org.uk>

From: Russell King - ARM Linux <linux@arm.linux.org.uk>
Date: Thu, 1 Aug 2013 22:06:18 +0100

> On Thu, Aug 01, 2013 at 02:04:47PM -0700, Joe Perches wrote:
>> On Thu, 2013-08-01 at 21:58 +0100, Russell King - ARM Linux wrote:
>> > This seems rather obscure - I mean, it's not obvious to driver authors
>> > that should be the case.  Would it not be better to make this a little
>> > more obvious somehow?  Maybe __aligned(2) against mac_addr?  Or
>> > maybe have a debugging check for it?
>> 
>> That'd be for David Miller (cc'd).
>> 
>> I believe he's argued in the past that any alignment check
>> for mac addresses was unnecessary.
>> 
>> For all I know it really might not matter because pasemi
>> can successfully dereference a ushort against an odd char
>> pointer.
>> 
>> I just noticed it and thought it'd be better moved.
> 
> As can ARM too for years now - either in hardware or via fixup for
> kernel code.

But probably not a good idea if it's done in a fast path.

^ permalink raw reply

* Re: [patch net 0/2] ipv6: prevent race between address creation and removal
From: David Miller @ 2013-08-01 21:17 UTC (permalink / raw)
  To: jiri; +Cc: netdev, jbenc, kuznet, jmorris, yoshfuji, kaber
In-Reply-To: <1375346488-1001-1-git-send-email-jiri@resnulli.us>

From: Jiri Pirko <jiri@resnulli.us>
Date: Thu,  1 Aug 2013 10:41:26 +0200

> Jiri Benc (1):
>   ipv6: prevent race between address creation and removal
> 
> Jiri Pirko (1):
>   ipv6: move peer_addr init into ipv6_add_addr()

Series applied, thanks Jiri.

^ 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