Netdev List
 help / color / mirror / Atom feed
* [PATCH] Add encap_rcv support to IPv6
From: Benjamin LaHaise @ 2012-03-20 13:58 UTC (permalink / raw)
  To: David S. Miller; +Cc: netdev

At present, UDP encapsulated protocols (like L2TP) are only able to use the
encap_rcv hook with UDP over IPv4.  This patch adds the same support for use
with UDP over IPv6.

Signed-off-by: Benjamin LaHaise <bcrl@kvack.org>
---
 net/ipv6/udp.c |   31 +++++++++++++++++++++++++++++++
 1 files changed, 31 insertions(+), 0 deletions(-)

diff --git a/net/ipv6/udp.c b/net/ipv6/udp.c
index 37b0699..4d7cd72 100644
--- a/net/ipv6/udp.c
+++ b/net/ipv6/udp.c
@@ -515,6 +515,37 @@ int udpv6_queue_rcv_skb(struct sock * sk, struct sk_buff *skb)
 	if (!xfrm6_policy_check(sk, XFRM_POLICY_IN, skb))
 		goto drop;
 
+	if (up->encap_type) {
+		int (*encap_rcv)(struct sock *sk, struct sk_buff *skb);
+
+		/*
+		 * This is an encapsulation socket so pass the skb to
+		 * the socket's udp_encap_rcv() hook. Otherwise, just
+		 * fall through and pass this up the UDP socket.
+		 * up->encap_rcv() returns the following value:
+		 * =0 if skb was successfully passed to the encap
+		 *    handler or was discarded by it.
+		 * >0 if skb should be passed on to UDP.
+		 * <0 if skb should be resubmitted as proto -N
+		 */
+
+		/* if we're overly short, let UDP handle it */
+		encap_rcv = ACCESS_ONCE(up->encap_rcv);
+		if (skb->len > sizeof(struct udphdr) && encap_rcv != NULL) {
+			int ret;
+
+			ret = encap_rcv(sk, skb);
+			if (ret <= 0) {
+				UDP6_INC_STATS_BH(sock_net(sk),
+						  UDP_MIB_INDATAGRAMS,
+						  is_udplite);
+				return -ret;
+			}
+		}
+
+		/* FALLTHROUGH -- it's a UDP Packet */
+	}
+
 	/*
 	 * UDP-Lite specific tests, ignored on UDP sockets (see net/ipv4/udp.c).
 	 */
-- 
1.7.4.1

^ permalink raw reply related

* [PATCH] Fix pppol2tp getsockname()
From: Benjamin LaHaise @ 2012-03-20 13:57 UTC (permalink / raw)
  To: David S. Miller, James Chapman; +Cc: netdev

While testing L2TP functionality, I came across a bug in getsockname().  The
IP address returned within the pppol2tp_addr's addr memember was not being
set to the IP  address in use.  This bug is caused by using inet_sk() on the
wrong socket (the L2TP socket rather than the underlying UDP socket), and was
likely introduced during the addition of L2TPv3 support.

Signed-off-by: Benjamin LaHaise <bcrl@kvack.org>
---
 net/l2tp/l2tp_ppp.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/net/l2tp/l2tp_ppp.c b/net/l2tp/l2tp_ppp.c
index 96bc7a6..9b07191 100644
--- a/net/l2tp/l2tp_ppp.c
+++ b/net/l2tp/l2tp_ppp.c
@@ -915,7 +915,7 @@ static int pppol2tp_getname(struct socket *sock, struct sockaddr *uaddr,
 		goto end_put_sess;
 	}
 
-	inet = inet_sk(sk);
+	inet = inet_sk(tunnel->sock);
 	if (tunnel->version == 2) {
 		struct sockaddr_pppol2tp sp;
 		len = sizeof(sp);
-- 
1.7.4.1

^ permalink raw reply related

* Re: [PATCH] net: bpf_jit: Simplify code by always using offset8 or offset32.
From: Eric Dumazet @ 2012-03-20 13:56 UTC (permalink / raw)
  To: Indan Zupancic
  Cc: Will Drewry, linux-kernel, linux-arch, linux-doc,
	kernel-hardening, netdev, x86, arnd, davem, hpa, mingo, oleg,
	peterz, rdunlap, mcgrathr, tglx, luto, eparis, serge.hallyn, djm,
	scarybeasts, pmoore, akpm, corbet, markus, coreyb, keescook
In-Reply-To: <cfb04c3fe59e421093f035c035c88b51.squirrel@webmail.greenhost.nl>

On Tue, 2012-03-20 at 22:33 +1100, Indan Zupancic wrote:

> And if the dev, len or data_len fields are really moved past the first
> 127 bytes the JIT code can be changed too. The JIT code already depends
> on some of struct sk_buff's field properties anyway.

Its seems you didnt understand, but I said NO to your patch.

This time I am really tired.

^ permalink raw reply

* Re: Per TCP session statistics
From: Daniel Baluta @ 2012-03-20 13:32 UTC (permalink / raw)
  To: James Courtier-Dutton; +Cc: netdev
In-Reply-To: <CAAMvbhFpCO0GCgYY=ckHWDBPLnj9Szs5jbGQuZcuDjNfJSWqvw@mail.gmail.com>

On Tue, Mar 20, 2012 at 3:26 PM, James Courtier-Dutton
<james.dutton@gmail.com> wrote:
> Hi,
>
> Is there a way to gather stats for each TCP session?
> The stats I am looking for are periodic sampling of the following metrics:
>
> Statistics         Def nition
> Cwin                Current congestion window
> Rwin                Current receive window
> BytesInFlight     # of bytes sent but not ACKed
> BytesInSndBuf   # of bytes written but not ACKed
> SmoothedRTT    Smoothed RTT computed by TCP
> BytesWritten    Cumulative # of bytes written by app
> BytesSent       Cumulative # of bytes sent
> PktsRetrans     Cumulative # of pkts retransmitted
> RwinLimitTime   Cumulative time that a connection is limited by receive window
> CwinLimitTime   Cumulative time that a connection is limited by
> congestion window
>
> So, for example,
> 1) I could say that all TCP sessions being listened to by process ID
> 2002 will get stats written to a file per tcp session per 60 seconds.
> 2) Gather local statistics for all TCP sessions initiated by process "firefox".
>
> Does there already exit such a tool for Linux?

Have a look at ss ([1]).

thanks,
Daniel.

[1] http://linux.die.net/man/8/ss

^ permalink raw reply

* Per TCP session statistics
From: James Courtier-Dutton @ 2012-03-20 13:26 UTC (permalink / raw)
  To: netdev

Hi,

Is there a way to gather stats for each TCP session?
The stats I am looking for are periodic sampling of the following metrics:

Statistics         Def\fnition
Cwin                Current congestion window
Rwin                Current receive window
BytesInFlight     # of bytes sent but not ACKed
BytesInSndBuf   # of bytes written but not ACKed
SmoothedRTT    Smoothed RTT computed by TCP
BytesWritten    Cumulative # of bytes written by app
BytesSent       Cumulative # of bytes sent
PktsRetrans     Cumulative # of pkts retransmitted
RwinLimitTime   Cumulative time that a connection is limited by receive window
CwinLimitTime   Cumulative time that a connection is limited by
congestion window

So, for example,
1) I could say that all TCP sessions being listened to by process ID
2002 will get stats written to a file per tcp session per 60 seconds.
2) Gather local statistics for all TCP sessions initiated by process "firefox".

Does there already exit such a tool for Linux?

Kind Regards

^ permalink raw reply

* netlink: 12 bytes leftover after parsing attributes - triggered by iproute2 libnetlink's rtnl_dump_request()
From: Bruno Prémont @ 2012-03-20 12:41 UTC (permalink / raw)
  To: linux-kernel, netdev, Greg Rose, Stephen Hemminger

Hi,

Starting with 3.3 when using collectd's netlink plugin to monitor
interface stattistics I'm seeing 3 lines of complaint in kernel log per
monitoring loop (10s interval)

  [64951.027953] netlink: 12 bytes leftover after parsing attributes.

It seems link the message is generated for each network interface on the
system.

The same userspace code running on 3.2 does not produce the lines in
kernel log.



Basic source code to reproduce (netlink subset of collectd's netlink plugin):
#include <stdio.h>
#include <string.h>
#include <sys/socket.h>
#include <linux/netlink.h>
#include <linux/rtnetlink.h>
#include <libnetlink.h>

int link_filter (const struct sockaddr_nl *sa, struct nlmsghdr *nmh, void *args) {
	return 0;
}

int main(int argc, char **argv) {
	struct rtnl_handle rth;
	struct ifinfomsg im;
	struct tcmsg tm;

	memset(&rth, 0, sizeof(rth));
	rtnl_open(&rth, 0);
	memset(&im, 0, sizeof(im));
	im.ifi_type = AF_UNSPEC;

	rtnl_dump_request(&rth, RTM_GETLINK, &im, sizeof(im));
	rtnl_dump_filter(&rth, link_filter, NULL, NULL, NULL);
	rtnl_close(&rth);
	return 0;
}



Compile with
  $CC -o test test.c -lnetlink
  (here using libnetlink.a from iproute2-2.6.38)



Strace of test code shows the following:
sendmsg(3, {msg_name(12)={sa_family=AF_NETLINK, pid=0, groups=00000000}, msg_iov(2)=[{" \0\0\0\22\0\1\3\272[hO\0\0\0\0", 16}, {"\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0", 16}], msg_controllen=0, msg_flags=0}, 0) = 32
recvmsg(3, {msg_name(12)={sa_family=AF_NETLINK, pid=0, groups=00000000}, msg_iov(1)=[{..., 16384}], msg_controllen=0, msg_flags=0}, 0) = 2980
recvmsg(3, {msg_name(12)={sa_family=AF_NETLINK, pid=0, groups=00000000}, msg_iov(1)=[{..., 16384}], msg_controllen=0, msg_flags=0}, 0) = 20

Note: when omitting the rtnl_dump_filter() call only two lines appear
in kernel log.

Comparing to iproute2 call (ip -s link list) which does not trigger the same
message in kernel log I have:
send(3, "\24\0\0\0\22\0\1\3\225]hO\0\0\0\0\21\0\0\0", 20, 0) = 20
recvmsg(3, {msg_name(12)={sa_family=AF_NETLINK, pid=0, groups=00000000}, msg_iov(1)=[{..., 16384}], msg_controllen=0, msg_flags=0}, 0) = 2980
recvmsg(3, {msg_name(12)={sa_family=AF_NETLINK, pid=0, groups=00000000}, msg_iov(1)=[{..., 16384}], msg_controllen=0, msg_flags=0}, 0) = 20





Looking at kernel history related to net/netlink I've seen the following
commit which introduced the warning (or rather started using kernel's
nla_parse() function in this path - and that function complains):


commit 115c9b81928360d769a76c632bae62d15206a94a
Author: Greg Rose <gregory.v.rose@intel.com>
Date:   Tue Feb 21 16:54:48 2012 -0500

    rtnetlink: Fix problem with buffer allocation
    
    Implement a new netlink attribute type IFLA_EXT_MASK.  The mask
    is a 32 bit value that can be used to indicate to the kernel that
    certain extended ifinfo values are requested by the user application.
    At this time the only mask value defined is RTEXT_FILTER_VF to
    indicate that the user wants the ifinfo dump to send information
    about the VFs belonging to the interface.
    
    This patch fixes a bug in which certain applications do not have
    large enough buffers to accommodate the extra information returned
    by the kernel with large numbers of SR-IOV virtual functions.
    Those applications will not send the new netlink attribute with
    the interface info dump request netlink messages so they will
    not get unexpectedly large request buffers returned by the kernel.
    
    Modifies the rtnl_calcit function to traverse the list of net
    devices and compute the minimum buffer size that can hold the
    info dumps of all matching devices based upon the filter passed
    in via the new netlink attribute filter mask.  If no filter
    mask is sent then the buffer allocation defaults to NLMSG_GOODSIZE.
    
    With this change it is possible to add yet to be defined netlink
    attributes to the dump request which should make it fairly extensible
    in the future.


A kernel at preceding commit 84338a6c9dbb6ff3de4749864020f8f25d86fc81 (neighbour:
Fixed race condition at tbl->nht) does not show the log message,
starting with that commit the message appears.


Should this get fixed at kernel level, iproute2 libnetlink level or
at end-user level (e.g. collectd)?
Three lines every 10 seconds is a damn lot!

Thanks,
Bruno

^ permalink raw reply

* Re: [PATCH] Fix pppol2tp getsockname()
From: James Chapman @ 2012-03-20 12:21 UTC (permalink / raw)
  To: Benjamin LaHaise; +Cc: David S. Miller, netdev
In-Reply-To: <4F686EE5.4040400@katalix.com>

On 20/03/12 11:49, James Chapman wrote:
> On 19/03/12 03:15, Benjamin LaHaise wrote:
>>
>> While testing L2TP functionality, I came across a bug in getsockname().  The
>> IP address returned within the pppol2tp_addr's addr memember was not being
>> set to the IP address in use.  This bug is caused by using inet_sk() on the
>> wrong socket (the L2TP socket rather than the underlying UDP socket), and was
>> likely introduced during the addition of L2TPv3 support.
>> ---
>>  net/l2tp/l2tp_ppp.c |    2 +-
>>  1 files changed, 1 insertions(+), 1 deletions(-)
>>
>> diff --git a/net/l2tp/l2tp_ppp.c b/net/l2tp/l2tp_ppp.c
>> index 96bc7a6..9b07191 100644
>> --- a/net/l2tp/l2tp_ppp.c
>> +++ b/net/l2tp/l2tp_ppp.c
>> @@ -915,7 +915,7 @@ static int pppol2tp_getname(struct socket *sock, struct sockaddr *uaddr,
>>  		goto end_put_sess;
>>  	}
>>  
>> -	inet = inet_sk(sk);
>> +	inet = inet_sk(tunnel->sock);
>>  	if (tunnel->version == 2) {
>>  		struct sockaddr_pppol2tp sp;
>>  		len = sizeof(sp);
> 
> The patch is incorrect.
> 
> This is supposed to return the socket info of the pppol2tp socket, not
> the tunnel socket. It is used by pppd's pppol2tp plugin to check that
> the fd supplied is the right socket type.

Sorry, my mistake. This patch is fine.

^ permalink raw reply

* Re: [PATCH 1/1] ARC VMAC driver.
From: Francois Romieu @ 2012-03-20 11:54 UTC (permalink / raw)
  To: Andreas Fenkart; +Cc: davem, netdev
In-Reply-To: <1332232945-27869-1-git-send-email-afenkart@gmail.com>

Andreas Fenkart <afenkart@gmail.com> :
> This is a driver for the MAC IP block from ARC International. It
> is based on an existing driver found in ARC Linux distribution,
> but essentially, a full rewrite.
> 
> Signed-off-by: Andreas Fenkart <afenkart@gmail.com>
> ---
>  drivers/net/ethernet/Kconfig   |    9 +
>  drivers/net/ethernet/Makefile  |    1 +
>  drivers/net/ethernet/arcvmac.c | 1472 ++++++++++++++++++++++++++++++++++++++++
>  drivers/net/ethernet/arcvmac.h |  269 ++++++++
>  4 files changed, 1751 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/net/ethernet/Kconfig b/drivers/net/ethernet/Kconfig
> index 597f4d4..4b6baf8 100644
> --- a/drivers/net/ethernet/Kconfig
> +++ b/drivers/net/ethernet/Kconfig
> @@ -23,6 +23,15 @@ source "drivers/net/ethernet/aeroflex/Kconfig"
>  source "drivers/net/ethernet/alteon/Kconfig"
>  source "drivers/net/ethernet/amd/Kconfig"
>  source "drivers/net/ethernet/apple/Kconfig"
> +
> +config ARCVMAC
> +	tristate "ARC VMAC ethernet driver"
> +	select MII
> +	select PHYLIB
> +	select CRC32
> +	help
> +	  MAC present Zoran43xx, IP from ARC international

It may not hurt to tell if it targets gigabit or fast ethernet.

> +
>  source "drivers/net/ethernet/atheros/Kconfig"
>  source "drivers/net/ethernet/cadence/Kconfig"
>  source "drivers/net/ethernet/adi/Kconfig"
> diff --git a/drivers/net/ethernet/Makefile b/drivers/net/ethernet/Makefile
> index be5dde0..cb61799 100644
> --- a/drivers/net/ethernet/Makefile
> +++ b/drivers/net/ethernet/Makefile
> @@ -9,6 +9,7 @@ obj-$(CONFIG_GRETH) += aeroflex/
>  obj-$(CONFIG_NET_VENDOR_ALTEON) += alteon/
>  obj-$(CONFIG_NET_VENDOR_AMD) += amd/
>  obj-$(CONFIG_NET_VENDOR_APPLE) += apple/
> +obj-$(CONFIG_ARCVMAC) += arcvmac.o

I am not sure we want it directly under drivers/net/ethernet.

[...]
> diff --git a/drivers/net/ethernet/arcvmac.c b/drivers/net/ethernet/arcvmac.c
> new file mode 100644
> index 0000000..d512806
> --- /dev/null
> +++ b/drivers/net/ethernet/arcvmac.c
[...]
> +				   unsigned char hwaddr[ETH_ALEN])
> +{
> +	struct vmac_priv *ap = netdev_priv(dev);
> +	u32 mac_lo, mac_hi;
> +
> +	WARN_ON(!hwaddr);

Useless. It will issue a nice Oops a few lines below anyway.

> +	mac_lo = vmac_readl(ap, ADDRL);
> +	mac_hi = vmac_readl(ap, ADDRH);
> +
> +	hwaddr[0] = (mac_lo >> 0) & 0xff;
> +	hwaddr[1] = (mac_lo >> 8) & 0xff;
> +	hwaddr[2] = (mac_lo >> 16) & 0xff;
> +	hwaddr[3] = (mac_lo >> 24) & 0xff;
> +	hwaddr[4] = (mac_hi >> 0) & 0xff;
> +	hwaddr[5] = (mac_hi >> 8) & 0xff;
> +	return hwaddr;
> +}
[...]
> +static int __devinit vmac_mii_init(struct vmac_priv *ap)
> +{
> +	unsigned long flags;
> +	int err, i;
> +
> +	spin_lock_irqsave(&ap->lock, flags);
> +
> +	ap->mii_bus = mdiobus_alloc();

Bug: non GFP_ATOMIC alloc with spinlock held.

> +	if (!ap->mii_bus)
> +		return -ENOMEM;
> +
> +	ap->mii_bus->name = "vmac_mii_bus";
> +	ap->mii_bus->read = &vmac_mdio_read;
> +	ap->mii_bus->write = &vmac_mdio_write;
> +
> +	snprintf(ap->mii_bus->id, MII_BUS_ID_SIZE, "%x", 0);
> +
> +	ap->mii_bus->priv = ap;
> +
> +	err = -ENOMEM;
> +	ap->mii_bus->irq = kmalloc(sizeof(int) * PHY_MAX_ADDR, GFP_KERNEL);

Sic.

[...]
> +static void vmac_mii_exit_unlocked(struct net_device *dev)
> +{
> +	struct vmac_priv *ap = netdev_priv(dev);
> +
> +	if (ap->phy_dev)

It can't be NULL.

> +		phy_disconnect(ap->phy_dev);
> +
> +	mdiobus_unregister(ap->mii_bus);
> +	kfree(ap->mii_bus->irq);
> +	mdiobus_free(ap->mii_bus);
> +}
> +
> +static int vmacether_get_settings(struct net_device *dev,
> +				  struct ethtool_cmd *cmd)
> +{
> +	struct vmac_priv *ap = netdev_priv(dev);
> +	struct phy_device *phydev = ap->phy_dev;
> +
> +	if (!phydev)
> +		return -ENODEV;

It can't be NULL here either.

Please check you driver. Most of times, it can't be NULL.

> +
> +	return phy_ethtool_gset(phydev, cmd);
> +}
> +
> +static int vmacether_set_settings(struct net_device *dev,
> +				  struct ethtool_cmd *cmd)
> +{
> +	struct vmac_priv *ap = netdev_priv(dev);
> +	struct phy_device *phydev = ap->phy_dev;
> +
> +	if (!phydev)
> +		return -ENODEV;

Sic.

> +
> +	return phy_ethtool_sset(phydev, cmd);
> +}
> +
> +static int vmac_ioctl(struct net_device *dev, struct ifreq *rq, int cmd)
> +{
> +	struct vmac_priv *ap = netdev_priv(dev);
> +	struct phy_device *phydev = ap->phy_dev;
> +
> +	if (!netif_running(dev))
> +		return -EINVAL;
> +
> +	if (!phydev)
> +		return -ENODEV;

Sic.

[...]
> +static int update_error_counters_unlocked(struct net_device *dev, int status)
> +{
> +	struct vmac_priv *ap = netdev_priv(dev);
> +
> +	dev_dbg(&ap->pdev->dev, "rx error counter overrun. status = 0x%x\n",
> +		status);
> +
> +	/* programming error */
> +	WARN_ON(status & TXCH_MASK);
> +	WARN_ON(!(status & (MSER_MASK | RXCR_MASK | RXFR_MASK | RXFL_MASK)));
> +
> +	if (status & MSER_MASK)
> +		dev->stats.rx_over_errors += 256; /* ran out of BD */
> +	if (status & RXCR_MASK)
> +		dev->stats.rx_crc_errors += 256;
> +	if (status & RXFR_MASK)
> +		dev->stats.rx_frame_errors += 256;
> +	if (status & RXFL_MASK)
> +		dev->stats.rx_fifo_errors += 256;

I would not mind a local &dev->stats variable.

[...]
> +static void update_tx_errors_unlocked(struct net_device *dev, int status)
> +{
> +	struct vmac_priv *ap = netdev_priv(dev);
> +
> +	if (status & BD_UFLO)
> +		dev->stats.tx_fifo_errors++;
> +
> +	if (ap->duplex)
> +		return;
> +
> +	/* half duplex flags */
> +	if (status & BD_LTCL)
> +		dev->stats.tx_window_errors++;
> +	if (status & BD_RETRY_CT)
> +		dev->stats.collisions += (status & BD_RETRY_CT) >> 24;
> +	if (status & BD_DROP)  /* too many retries */
> +		dev->stats.tx_aborted_errors++;
> +	if (status & BD_DEFER)
> +		dev_vdbg(&ap->pdev->dev, "\"defer to traffic\"\n");
> +	if (status & BD_CARLOSS)
> +		dev->stats.tx_carrier_errors++;

Same thing as above.

[...]
> +static int vmac_poll(struct napi_struct *napi, int budget)
> +{
> +	struct vmac_priv *ap;
> +	int rx_work_done;
> +
> +	ap = container_of(napi, struct vmac_priv, napi);

You can 'struct vmac_priv *ap = container_of(napi ..."

> +
> +	vmac_tx_reclaim_unlocked(ap->dev, false);
> +
> +	rx_work_done = vmac_rx_receive(ap->dev, budget);
> +	if (rx_work_done >= budget) {
> +		/* rx queue is not yet empty/clean */
> +		return rx_work_done;
> +	}

Please no comment nor curly brace.

> +
> +	/* no more packet in rx/tx queue, remove device from poll queue */
> +	napi_complete(napi);
> +
> +	/* clear status, only 1' affect register state */
> +	vmac_writel(ap, RXINT_MASK | TXINT_MASK, STAT);
> +
> +	/* reenable IRQ */
> +	vmac_toggle_rxint_unlocked(ap->dev, true);
> +	vmac_toggle_txint_unlocked(ap->dev, true);
> +
> +	return rx_work_done;
> +}
> +
> +static irqreturn_t vmac_intr(int irq, void *dev_instance)
> +{
> +	struct net_device *dev = dev_instance;
> +	struct vmac_priv *ap = netdev_priv(dev);
> +	u32 status;
> +
> +	spin_lock(&ap->lock);
> +
> +	status = vmac_readl(ap, STAT);
> +	vmac_writel(ap, status, STAT);

I do not know what you are trying to achieve with the lock but it is not
held in start_xmit when STAT is written.

> +
> +	if (unlikely(ap->shutdown))
> +		dev_err(&ap->pdev->dev, "ISR during close\n");
> +
> +	if (unlikely(!status & (RXINT_MASK|MDIO_MASK|ERR_MASK)))
> +		dev_err(&ap->pdev->dev, "Spurious IRQ\n");
> +
> +	if ((status & RXINT_MASK) && (vmac_readl(ap, ENABLE) & RXINT_MASK) &&
> +	    (ap->dma_rx_head != vmac_readl(ap, MAC_RXRING_HEAD))) {
> +		vmac_toggle_rxint_unlocked(dev, false);
> +		napi_schedule(&ap->napi);
> +	}
> +
> +	if ((status & TXINT_MASK) && (vmac_readl(ap, ENABLE) & TXINT_MASK)) {
> +		vmac_toggle_txint_unlocked(dev, false);
> +		napi_schedule(&ap->napi);
> +	}
> +
> +	if (status & MDIO_MASK)
> +		complete(&ap->mdio_complete);
> +
> +	if (unlikely(status & ERR_MASK))
> +		update_error_counters_unlocked(dev, status);

Do yourself a favor : move everything to NAPI context.

[...]
> +static int vmac_start_xmit(struct sk_buff *skb, struct net_device *dev)
> +{
> +	struct vmac_priv *ap = netdev_priv(dev);
> +	struct vmac_buffer_desc *desc;
> +
> +	/* running under xmit lock */
> +	/* locking: modifies tx_ring head, tx_reclaim only tail */
> +
> +	/* no scatter/gatter see features below */
> +	WARN_ON(skb_shinfo(skb)->nr_frags != 0);
> +	WARN_ON(skb->len > MAX_TX_BUFFER_LEN);
> +
> +	if (unlikely(fifo_full(&ap->tx_ring))) {
> +		netif_stop_queue(dev);
> +		dev_err(&ap->pdev->dev,
> +			"xmit called while no tx desc available\n");
> +		return NETDEV_TX_BUSY;
> +	}
> +
> +	if (unlikely(skb->len < ETH_ZLEN)) {
> +		if (skb_padto(skb, ETH_ZLEN))
> +			return NETDEV_TX_OK;
> +		skb_put(skb, ETH_ZLEN - skb->len);
> +	}
> +
> +	/* fill descriptor */
> +	ap->tx_skbuff[ap->tx_ring.head] = skb;
> +	desc = &ap->txbd[ap->tx_ring.head];
> +	WARN_ON(desc->info & cpu_to_le32(BD_DMA_OWN));
> +
> +	desc->data = dma_map_single(&ap->pdev->dev, skb->data, skb->len,
> +				    DMA_TO_DEVICE);
> +
> +	/* dma might already be polling */
> +	wmb();
> +	desc->info = cpu_to_le32(BD_DMA_OWN | BD_FRST | BD_LAST | skb->len);
> +
> +	/* kick tx dma, only 1' affect register */
> +	vmac_writel(ap, TXPL_MASK, STAT);
> +
> +	dev->stats.tx_packets++;
> +	dev->stats.tx_bytes += skb->len;

No byte queue limit ?

> +	fifo_inc_head(&ap->tx_ring);
> +
> +	/* stop queue if no more desc available */
> +	if (fifo_full(&ap->tx_ring))
> +		netif_stop_queue(dev);
> +
> +	return NETDEV_TX_OK;
> +}
> +
> +static int alloc_buffers_unlocked(struct net_device *dev)
> +{
> +	struct vmac_priv *ap = netdev_priv(dev);
> +	int size, err = -ENOMEM;
> +
> +	fifo_init(&ap->rx_ring, RX_BDT_LEN);
> +	fifo_init(&ap->tx_ring, TX_BDT_LEN);
> +
> +	/* initialize skb list */
> +	memset(ap->rx_skbuff, 0, sizeof(ap->rx_skbuff));
> +	memset(ap->tx_skbuff, 0, sizeof(ap->tx_skbuff));
> +
> +	/* allocate DMA received descriptors */
> +	size = sizeof(*ap->rxbd) * ap->rx_ring.size;
> +	ap->rxbd = dma_alloc_coherent(&ap->pdev->dev, size,
> +				      &ap->rxbd_dma,
> +				      GFP_KERNEL);
> +	if (!ap->rxbd)
> +		goto err_out;
> +
> +	/* allocate DMA transmit descriptors */
> +	size = sizeof(*ap->txbd) * ap->tx_ring.size;
> +	ap->txbd = dma_alloc_coherent(&ap->pdev->dev, size,
> +				      &ap->txbd_dma,
> +				      GFP_KERNEL);
> +	if (!ap->txbd)
> +		goto err_free_rxbd;
> +
> +	/* ensure 8-byte aligned */
> +	WARN_ON(((uintptr_t)ap->txbd & 0x7) || ((uintptr_t)ap->rxbd & 0x7));
> +
> +	memset(ap->txbd, 0, sizeof(*ap->txbd) * ap->tx_ring.size);
> +	memset(ap->rxbd, 0, sizeof(*ap->rxbd) * ap->rx_ring.size);

It is already zeroed after dma_alloc_coherent.

[...]
> +static int vmac_open(struct net_device *dev)
> +{
> +	struct vmac_priv *ap = netdev_priv(dev);
> +	struct phy_device *phydev;
> +	unsigned long flags;
> +	u32 mask, ctrl;
> +	int err = 0;
> +
> +	/* locking: no concurrency yet */
> +
> +	if (!ap)
> +		return -ENODEV;
> +
> +	spin_lock_irqsave(&ap->lock, flags);
> +	ap->shutdown = false;
> +
> +	err = get_register_map(ap);
> +	if (err)
> +		return err;
> +
> +	vmac_hw_init(dev);
> +
> +	/* mac address changed? */
> +	write_mac_reg(dev, dev->dev_addr);
> +
> +	err = alloc_buffers_unlocked(dev);

dma_alloc_coherent(... GFP_KERNEL) with spinlock held.

What are you trying to lock against anyway ?

[...]
> +static int vmac_close(struct net_device *dev)
> +{
> +	struct vmac_priv *ap = netdev_priv(dev);
> +	unsigned long flags;
> +	u32 tmp;
> +
> +	/* locking: protect everything, DMA / IRQ / timer */
> +	spin_lock_irqsave(&ap->lock, flags);
> +
> +	/* complete running transfer, then stop */
> +	tmp = vmac_readl(ap, CONTROL);
> +	tmp &= ~(TXRN_MASK | RXRN_MASK);
> +	vmac_writel(ap, tmp, CONTROL);
> +
> +	/* save statistics, before unmapping */
> +	update_vmac_stats_unlocked(dev);
> +
> +	/* reenable IRQ, process pending */
> +	spin_unlock_irqrestore(&ap->lock, flags);
> +
> +	set_current_state(TASK_INTERRUPTIBLE);
> +	schedule_timeout(msecs_to_jiffies(20));
> +
> +	/* shut it down now */
> +	spin_lock_irqsave(&ap->lock, flags);
> +	ap->shutdown = true;
> +
> +	netif_stop_queue(dev);
> +	napi_disable(&ap->napi);

Bug: napi_disable may sleep and the driver is holding a spinlock.

> +	/* disable phy */
> +	phy_stop(ap->phy_dev);
> +	vmac_mii_exit_unlocked(dev);
> +	netif_carrier_off(dev);
> +
> +	/* disable interrupts */
> +	vmac_writel(ap, 0, ENABLE);
> +	free_irq(dev->irq, dev);
> +
> +	/* turn off vmac */
> +	vmac_writel(ap, 0, CONTROL);
> +	/* vmac_reset_hw(vmac) */
> +
> +	/* locking: concurrency off */
> +	spin_unlock_irqrestore(&ap->lock, flags);

[...]
> +static void update_vmac_stats_unlocked(struct net_device *dev)
> +{
> +	struct net_device_stats *_stats = &dev->stats;

What's wrong with plain 'stats' ?

[...]
> +static void create_multicast_filter(struct net_device *dev,
> +				    int32_t *bitmask)
> +{
> +	char *addrs;
> +	u32 crc;
> +
> +	/* locking: done by net_device */
> +
> +	WARN_ON(netdev_mc_count(dev) == 0);
> +	WARN_ON(dev->flags & IFF_ALLMULTI);
> +
> +	bitmask[0] = bitmask[1] = 0;
> +
> +	{

?

[...]
> +static int __devinit vmac_probe(struct platform_device *pdev)
> +{
> +	struct net_device *dev;
> +	struct vmac_priv *ap;
> +	struct resource *mem;
> +	int err;
> +
> +	/* locking: no concurrency */
> +
> +	if (dma_get_mask(&pdev->dev) > DMA_BIT_MASK(32) ||
> +	    pdev->dev.coherent_dma_mask > DMA_BIT_MASK(32)) {
> +		dev_err(&pdev->dev,
> +			"arcvmac supports only 32-bit DMA addresses\n");
> +		return -ENODEV;
> +	}
> +
> +	dev = alloc_etherdev(sizeof(*ap));
> +	if (!dev) {
> +		dev_err(&pdev->dev, "etherdev alloc failed, aborting.\n");
> +		return -ENOMEM;
> +	}
> +
> +	ap = netdev_priv(dev);
> +
> +	err = -ENODEV;
> +	mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	if (!mem) {
> +		dev_err(&pdev->dev, "no mmio resource defined\n");
> +		goto err_out;
> +	}
> +	ap->mem = mem;
> +
> +	err = platform_get_irq(pdev, 0);
> +	if (err < 0) {
> +		dev_err(&pdev->dev, "no irq found\n");
> +		goto err_out;
> +	}
> +	dev->irq = err;

net_device.{irq, base_addr} have been obsolete for years. Please don't use
it to expose kernel internals to userspace.

> +
> +	spin_lock_init(&ap->lock);
> +
> +	SET_NETDEV_DEV(dev, &pdev->dev);
> +	ap->dev = dev;
> +	ap->pdev = pdev;
> +
> +	/* init rx timeout (used for oom) */
> +	init_timer(&ap->refill_timer);
> +	ap->refill_timer.function = vmac_refill_rx_timer;
> +	ap->refill_timer.data = (unsigned long)dev;
> +	spin_lock_init(&ap->refill_lock);
> +
> +	netif_napi_add(dev, &ap->napi, vmac_poll, 64);
> +	dev->netdev_ops = &vmac_netdev_ops;
> +	dev->ethtool_ops = &vmac_ethtool_ops;
> +
> +	dev->base_addr = (unsigned long)ap->regs;

(see above)

> +
> +	/* prevent buffer chaining, favor speed over space */
> +	ap->rx_skb_size = ETH_FRAME_LEN + VMAC_BUFFER_PAD;
> +
> +	/* private struct functional */
> +
> +	/* temporarily map registers to fetch mac addr */
> +	err = get_register_map(ap);
> +	if (err)
> +		goto err_out;

Please use 'goto err_perform_some_unwinding_action' style.

You don't need to be literate. 'goto err_napi_del' will be good enough :o)

> +
> +	/* mac address intialize, set vmac_open  */
> +	read_mac_reg(dev, dev->dev_addr);
> +
> +	if (!is_valid_ether_addr(dev->dev_addr))
> +		dev_hw_addr_random(dev, dev->dev_addr);
> +
> +	err = register_netdev(dev);
> +	if (err) {
> +		dev_err(&pdev->dev, "Cannot register net device, aborting.\n");
> +		goto err_out;

Should be something like 'goto err_I_wont_leak_register_memory_region'

> +	}
> +
> +	/* release the memory region, till open is called */

Why ? It adds a failure opportunity.

[...]
> diff --git a/drivers/net/ethernet/arcvmac.h b/drivers/net/ethernet/arcvmac.h
> new file mode 100644
> index 0000000..f94ab38
> --- /dev/null
> +++ b/drivers/net/ethernet/arcvmac.h
[...]
> +/* stat/enable use same bit mask */
> +#define VMAC_STAT		0x04
> +#define VMAC_ENABLE		0x08
> +#  define TXINT_MASK		0x00000001 /* Transmit interrupt */
> +#  define RXINT_MASK		0x00000002 /* Receive interrupt */
> +#  define ERR_MASK		0x00000004 /* Error interrupt */
> +#  define TXCH_MASK		0x00000008 /* Transmit chaining error */
> +#  define MSER_MASK		0x00000010 /* Missed packet counter error */
> +#  define RXCR_MASK		0x00000100 /* RXCRCERR counter rolled over	 */

Please keep things aligned and add the extra space after "define" (see tg3.h).

[...]
> +struct vmac_priv {
> +	struct net_device *dev;
> +	struct platform_device *pdev;
> +
> +	struct completion mdio_complete;
> +	spinlock_t lock; /* protects structure plus hw regs of device */
> +
> +	/* base address of register set */
> +	char *regs;

It should be __iomem annotated.

[...]
> +/* DMA ring management */
> +
> +/* for a fifo with size n,
> + * - [0..n] fill levels are n + 1 states
> + * - there are only n different deltas (head - tail) values
> + * => not all fill levels can be represented with head, tail
> + *    pointers only
> + * we give up the n fill level, aka fifo full */
> +
> +/* sacrifice one elt as a sentinel */
> +static inline int fifo_used(struct dma_fifo *f);
> +static inline int fifo_inc_ct(int ct, int size);
> +static inline void fifo_dump(struct dma_fifo *fifo);

Please reorder and remove the forward declarations.

-- 
Ueimor

^ permalink raw reply

* Re: [PATCH] Fix pppol2tp getsockname()
From: James Chapman @ 2012-03-20 11:49 UTC (permalink / raw)
  To: Benjamin LaHaise; +Cc: David S. Miller, netdev
In-Reply-To: <20120319031534.GC11293@kvack.org>

On 19/03/12 03:15, Benjamin LaHaise wrote:
> 
> While testing L2TP functionality, I came across a bug in getsockname().  The
> IP address returned within the pppol2tp_addr's addr memember was not being
> set to the IP address in use.  This bug is caused by using inet_sk() on the
> wrong socket (the L2TP socket rather than the underlying UDP socket), and was
> likely introduced during the addition of L2TPv3 support.
> ---
>  net/l2tp/l2tp_ppp.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/net/l2tp/l2tp_ppp.c b/net/l2tp/l2tp_ppp.c
> index 96bc7a6..9b07191 100644
> --- a/net/l2tp/l2tp_ppp.c
> +++ b/net/l2tp/l2tp_ppp.c
> @@ -915,7 +915,7 @@ static int pppol2tp_getname(struct socket *sock, struct sockaddr *uaddr,
>  		goto end_put_sess;
>  	}
>  
> -	inet = inet_sk(sk);
> +	inet = inet_sk(tunnel->sock);
>  	if (tunnel->version == 2) {
>  		struct sockaddr_pppol2tp sp;
>  		len = sizeof(sp);

The patch is incorrect.

This is supposed to return the socket info of the pppol2tp socket, not
the tunnel socket. It is used by pppd's pppol2tp plugin to check that
the fd supplied is the right socket type.


-- 
James Chapman
Katalix Systems Ltd
http://www.katalix.com
Catalysts for your Embedded Linux software development

^ permalink raw reply

* Re: ath: DMA failed to stop in 10 ms AR_CR=0x00000024 AR_DIAG_SW=0x02000020 DMADBG_7=0x00008040
From: Mohammed Shafi @ 2012-03-20 11:46 UTC (permalink / raw)
  To: Justin Mattock
  Cc: Linux-wireless, Felix Fietkau, linux-kernel, Linux-netdev,
	Sujith Manoharan
In-Reply-To: <CAD2nsn1SxhX3MZTF+XYN=cQEfHuD9bu5_TuLtVOCNmVBoZNJ2A@mail.gmail.com>

this should also fix
https://bugzilla.kernel.org/show_bug.cgi?id=42903

-- 
thanks,
shafi

^ permalink raw reply

* RE: [PATCH] net: bpf_jit: Simplify code by always using offset8 or offset32.
From: David Laight @ 2012-03-20 11:41 UTC (permalink / raw)
  To: Indan Zupancic, Eric Dumazet
  Cc: Will Drewry, linux-kernel, linux-arch, linux-doc,
	kernel-hardening, netdev, x86, arnd, davem, hpa, mingo, oleg,
	peterz, rdunlap, mcgrathr, tglx, luto, eparis, serge.hallyn, djm,
	scarybeasts, pmoore, akpm, corbet, markus, coreyb, keescook
In-Reply-To: <cfb04c3fe59e421093f035c035c88b51.squirrel@webmail.greenhost.nl>

 
> On Tue, March 20, 2012 13:59, Eric Dumazet wrote:
> > On Tue, 2012-03-20 at 13:24 +1100, Indan Zupancic wrote:
> >
> >> If it does then perhaps the fast path should be made faster by
inlining
> >> the code instead of calling a function which may not be cached.
> >>
> >
> > inlining 400 times a sequence of code is waste of icache you
probably
> > missed this.
> 
> Well, according to you most filters were small, inling 26 bytes a few
> times should be faster than calling an external function. Not 
> all calls need to be inlined either.

I wouldn't bet on it.
If the number of arguments is small enough to fit in the registers,
the called function doesn't to save any registers, and the call
doesn't mean the calling code runs out of registers,
the actual cost of the call will be minimal.

OTOH the benefit of only having to fetch the code once,
and the higher likelyhood that it will be in the i-cache
from some other use, will make the version with the calls
faster.

You need to do real benchmarks on a real system running
a real workload to find out which is better.
Oh and beware that changes in which code shares cache
lines can have a measuarable effect (typified by unrelated
changes affecting measured performance).

	David



^ permalink raw reply

* Re: ath: DMA failed to stop in 10 ms AR_CR=0x00000024 AR_DIAG_SW=0x02000020 DMADBG_7=0x00008040
From: Mohammed Shafi @ 2012-03-20 11:36 UTC (permalink / raw)
  To: Sujith Manoharan
  Cc: Justin P. Mattock, Linux-wireless, Felix Fietkau, linux-kernel,
	Linux-netdev
In-Reply-To: <CAD2nsn10svpDFkpbkCtFMGFdqW9G0syWrhL3s-ApQH2bKt0P2g@mail.gmail.com>

On Tue, Mar 20, 2012 at 4:45 PM, Mohammed Shafi
<shafi.wireless@gmail.com> wrote:
> On Tue, Mar 20, 2012 at 3:07 PM, Mohammed Shafi
> <shafi.wireless@gmail.com> wrote:
>> Hi Sujith/Justin,
>>
>> On Tue, Mar 20, 2012 at 1:59 PM, Sujith Manoharan
>> <c_manoha@qca.qualcomm.com> wrote:
>>> Justin P. Mattock wrote:
>>>> yeah this works:
>>>>
>>>> eading symbols from
>>>> /home/kernel/linux-next/drivers/net/wireless/ath/ath9k/ath9k.o...done.
>>>> (gdb) l *(ath_tx_start+0x284)
>>>> 0xcad4 is in ath_tx_start (drivers/net/wireless/ath/ath9k/xmit.c:1878).
>>>> 1873                  ieee80211_is_data_qos(hdr->frame_control)) {
>>>> 1874                  tidno = ieee80211_get_qos_ctl(hdr)[0] &
>>>> 1875                          IEEE80211_QOS_CTL_TID_MASK;
>>>> 1876                  tid = ATH_AN_2_TID(txctl->an, tidno);
>>>> 1877
>>>> 1878                  WARN_ON(tid->ac->txq != txctl->txq);
>>>> 1879          }
>>>> 1880
>>>> 1881          if ((tx_info->flags & IEEE80211_TX_CTL_AMPDU) && tid) {
>>>> 1882                  /*
>>>> (gdb)
>>>
>>> Can you try this patch ?
>>
>> just found out that 'ht_supported' may not  be set, if assoc response
>> does not has ht_cap IE (or) if we could not parse it (why), then the
>> driver won't initialize those tid related structures ath_tx_node_init,
>> while we later access them in ath_tx_start. so this should fix the
>> issue.
>
> now i was able to quite easily recreate this issue in wireless testing
> even without suspend resume and with the patch the issue seems to get
> fixed.
> i was using 2ghz channel 6 and TKIP (no HT!).

i was keep on trying with 5ghz TKIP and it seems HT enable even with
TKIP (need to see why this happens), thats why even i could not
recreate this issue. mac80211 seems to disable HT, ht_supported false
while in driver we go with ATH9K_HW_CAP HT


>
>
>>
>>>
>>> From: Sujith Manoharan <c_manoha@qca.qualcomm.com>
>>> Date: Tue, 20 Mar 2012 13:51:26 +0530
>>> Subject: [PATCH] ath9k: Use HW HT capabilites properly
>>>
>>> The commit "ath9k: Remove aggregation flags" changed how
>>> nodes were being initialized. Use the HW HT cap bits
>>> to initialize/de-initialize nodes, else we would be
>>> accessing an uninitialized entry during a suspend/resume cycle,
>>> resulting in a panic.
>>>
>>> Reported-by: Justin P. Mattock <justinmattock@gmail.com>
>>> Signed-off-by: Sujith Manoharan <c_manoha@qca.qualcomm.com>
>>> ---
>>>  drivers/net/wireless/ath/ath9k/main.c |    4 ++--
>>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/net/wireless/ath/ath9k/main.c b/drivers/net/wireless/ath/ath9k/main.c
>>> index 3879485..215eb25 100644
>>> --- a/drivers/net/wireless/ath/ath9k/main.c
>>> +++ b/drivers/net/wireless/ath/ath9k/main.c
>>> @@ -640,7 +640,7 @@ static void ath_node_attach(struct ath_softc *sc, struct ieee80211_sta *sta,
>>>        an->sta = sta;
>>>        an->vif = vif;
>>>
>>> -       if (sta->ht_cap.ht_supported) {
>>> +       if (sc->sc_ah->caps.hw_caps & ATH9K_HW_CAP_HT) {
>>>                ath_tx_node_init(sc, an);
>>>                an->maxampdu = 1 << (IEEE80211_HT_MAX_AMPDU_FACTOR +
>>>                                     sta->ht_cap.ampdu_factor);
>>> @@ -659,7 +659,7 @@ static void ath_node_detach(struct ath_softc *sc, struct ieee80211_sta *sta)
>>>        an->sta = NULL;
>>>  #endif
>>>
>>> -       if (sta->ht_cap.ht_supported)
>>> +       if (sc->sc_ah->caps.hw_caps & ATH9K_HW_CAP_HT)
>>>                ath_tx_node_cleanup(sc, an);
>>>  }
>>>
>>> --
>>> 1.7.9.4
>>>
>>
>>
>>
>> --
>> thanks,
>> shafi
>
>
>
> --
> thanks,
> shafi



-- 
thanks,
shafi

^ permalink raw reply

* Re: [PATCH] net: bpf_jit: Simplify code by always using offset8 or offset32.
From: Indan Zupancic @ 2012-03-20 11:33 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: Will Drewry, linux-kernel, linux-arch, linux-doc,
	kernel-hardening, netdev, x86, arnd, davem, hpa, mingo, oleg,
	peterz, rdunlap, mcgrathr, tglx, luto, eparis, serge.hallyn, djm,
	scarybeasts, pmoore, akpm, corbet, markus, coreyb, keescook
In-Reply-To: <1332212386.22737.20.camel@edumazet-glaptop>

On Tue, March 20, 2012 13:59, Eric Dumazet wrote:
> On Tue, 2012-03-20 at 13:24 +1100, Indan Zupancic wrote:
>
>> If it does then perhaps the fast path should be made faster by inlining
>> the code instead of calling a function which may not be cached.
>>
>
> inlining 400 times a sequence of code is waste of icache, you probably
> missed this.

Well, according to you most filters were small, inling 26 bytes a few
times should be faster than calling an external function. Not all calls
need to be inlined either.

>
> I spent a lot of time on working on this implementation, tried many
> different strategies before choosing the one in place.
>
> Listen, I am tired of this thread, it seems you want to push changes
> that have almost no value but still need lot of review.

The latest patch didn't change generated code except for a few ancillary
instructions. The one before that just added documentation. The first
patch was indeed bad.

>
> Unless you make benchmarks and can make at least 5 % improvement of the
> speed, or improve maintainability of this code, I am not interested.

My next patch would have changed the compiler to always compile in two
passes instead of looping till the result is stable. But never mind.

>
> We certainly _can_ one day have sizeof(struct sk_buff) > 256, and actual
> code is ready for this. You want to break this for absolutely no valid
> reason.

I've the feeling you didn't read the latest patch, it doesn't assume
sizeof(struct sk_buff) < 256, nor that fields aren't reordered.

>
> We _can_ change fields order anytime in struct sk_buff, even if you
> state "its very unlikely that those fields are ever moved to the end
> of sk_buff".

And if the dev, len or data_len fields are really moved past the first
127 bytes the JIT code can be changed too. The JIT code already depends
on some of struct sk_buff's field properties anyway.

Greetings,

Indan



^ permalink raw reply

* Re: [PATCH 0/8] net/mlx4_en: DCB QoS support
From: Amir Vadai @ 2012-03-20 11:29 UTC (permalink / raw)
  To: David S. Miller, John Fastabend
  Cc: netdev, Roland Dreier, Oren Duer, Yevgeny Petrilin
In-Reply-To: <1331659323-12904-1-git-send-email-amirv@mellanox.com>

On 03/13/2012 07:21 PM, Amir Vadai wrote:
> DCBX version 802.1qaz is supported.
> User Priority (UP) is set in QP context instead of in WQE (QP Work Queue
> Element), which means that all traffic from a queue will have the same UP.
> UP is also set for untagged traffic to be able to classify such traffic too.
>
> Mapping from sk_prio to User Priority is done by sch_mqprio mapping. Although
> confusingly sch_mqprio maps sk_prio to something called TC, it is not related
> to DCBX's TC, and is interpreted by mlx4_en driver as UP.
>
> The Current HW based QoS mechanism which was introduced in commit 4f57c087de9
> "net: implement mechanism for HW based QOS" is in orientation to ETS traffic
> class. Patch 7/8 introduces an approach which allow to use this mechanism also
> with hardware who has queues per user priority (UP). After the change,
> __skb_tx_hash() will direct a flow to a tx ring from a range of tx rings. This
> range is defined by the caller function by the specific HW. If TC based queues,
> the range is by TC number and for UP based queues, the range is by UP.
>
> Amir Vadai (8):
>    net/mlx4_en: Force user priority by QP attribute
>    net/mlx4_core: set port QoS attributes
>    net/mlx4_en: DCB QoS support
>    net/mlx4_en: Set max rate-limit for a TC
>    net/mlx4_en: sk_prio<=>  UP for untagged traffic
>    IB/rdma_cm: TOS<=>  UP mapping for IBoE
>    net: support tx_ring per UP in HW based QoS mechanism
>    net/mlx4_en: num cores tx rings for every UP
>
>   drivers/infiniband/core/cma.c                     |   35 ++++-
>   drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c   |   11 +-
>   drivers/net/ethernet/mellanox/mlx4/Kconfig        |   12 ++
>   drivers/net/ethernet/mellanox/mlx4/Makefile       |    1 +
>   drivers/net/ethernet/mellanox/mlx4/en_dcb_nl.c    |  215 +++++++++++++++++++++
>   drivers/net/ethernet/mellanox/mlx4/en_main.c      |    6 +-
>   drivers/net/ethernet/mellanox/mlx4/en_netdev.c    |   64 ++++++-
>   drivers/net/ethernet/mellanox/mlx4/en_port.h      |    2 +
>   drivers/net/ethernet/mellanox/mlx4/en_resources.c |    6 +-
>   drivers/net/ethernet/mellanox/mlx4/en_rx.c        |    4 +-
>   drivers/net/ethernet/mellanox/mlx4/en_sysfs.c     |  120 ++++++++++++
>   drivers/net/ethernet/mellanox/mlx4/en_tx.c        |   20 +-
>   drivers/net/ethernet/mellanox/mlx4/mlx4.h         |   20 ++
>   drivers/net/ethernet/mellanox/mlx4/mlx4_en.h      |   38 +++-
>   drivers/net/ethernet/mellanox/mlx4/port.c         |   62 ++++++
>   include/linux/mlx4/cmd.h                          |    4 +
>   include/linux/mlx4/device.h                       |    3 +
>   include/linux/mlx4/qp.h                           |    3 +-
>   include/linux/netdevice.h                         |   12 +-
>   include/linux/skbuff.h                            |    3 +-
>   net/core/dev.c                                    |   10 +-
>   21 files changed, 615 insertions(+), 36 deletions(-)
>   create mode 100644 drivers/net/ethernet/mellanox/mlx4/en_dcb_nl.c
>   create mode 100644 drivers/net/ethernet/mellanox/mlx4/en_sysfs.c
>


Hi Dave,

Patches 7-8 who deal with the interaction between the kernel HW QoS 
constructs to the queue selection logic are still under discussion with 
John and some changes might be needed there.
At this point, we ask for patches 1-6 to be pulled in, and continue the 
discussion from there.

Thanks,
Amir

^ permalink raw reply

* Re: ath: DMA failed to stop in 10 ms AR_CR=0x00000024 AR_DIAG_SW=0x02000020 DMADBG_7=0x00008040
From: Mohammed Shafi @ 2012-03-20 11:15 UTC (permalink / raw)
  To: Sujith Manoharan
  Cc: Justin P. Mattock, Linux-wireless, Felix Fietkau, linux-kernel,
	Linux-netdev
In-Reply-To: <CAD2nsn0JCKd4NnXOLfw4QXHTJdxrfepw9s6yYca=uSHwWXq3-A@mail.gmail.com>

On Tue, Mar 20, 2012 at 3:07 PM, Mohammed Shafi
<shafi.wireless@gmail.com> wrote:
> Hi Sujith/Justin,
>
> On Tue, Mar 20, 2012 at 1:59 PM, Sujith Manoharan
> <c_manoha@qca.qualcomm.com> wrote:
>> Justin P. Mattock wrote:
>>> yeah this works:
>>>
>>> eading symbols from
>>> /home/kernel/linux-next/drivers/net/wireless/ath/ath9k/ath9k.o...done.
>>> (gdb) l *(ath_tx_start+0x284)
>>> 0xcad4 is in ath_tx_start (drivers/net/wireless/ath/ath9k/xmit.c:1878).
>>> 1873                  ieee80211_is_data_qos(hdr->frame_control)) {
>>> 1874                  tidno = ieee80211_get_qos_ctl(hdr)[0] &
>>> 1875                          IEEE80211_QOS_CTL_TID_MASK;
>>> 1876                  tid = ATH_AN_2_TID(txctl->an, tidno);
>>> 1877
>>> 1878                  WARN_ON(tid->ac->txq != txctl->txq);
>>> 1879          }
>>> 1880
>>> 1881          if ((tx_info->flags & IEEE80211_TX_CTL_AMPDU) && tid) {
>>> 1882                  /*
>>> (gdb)
>>
>> Can you try this patch ?
>
> just found out that 'ht_supported' may not  be set, if assoc response
> does not has ht_cap IE (or) if we could not parse it (why), then the
> driver won't initialize those tid related structures ath_tx_node_init,
> while we later access them in ath_tx_start. so this should fix the
> issue.

now i was able to quite easily recreate this issue in wireless testing
even without suspend resume and with the patch the issue seems to get
fixed.
i was using 2ghz channel 6 and TKIP (no HT!).


>
>>
>> From: Sujith Manoharan <c_manoha@qca.qualcomm.com>
>> Date: Tue, 20 Mar 2012 13:51:26 +0530
>> Subject: [PATCH] ath9k: Use HW HT capabilites properly
>>
>> The commit "ath9k: Remove aggregation flags" changed how
>> nodes were being initialized. Use the HW HT cap bits
>> to initialize/de-initialize nodes, else we would be
>> accessing an uninitialized entry during a suspend/resume cycle,
>> resulting in a panic.
>>
>> Reported-by: Justin P. Mattock <justinmattock@gmail.com>
>> Signed-off-by: Sujith Manoharan <c_manoha@qca.qualcomm.com>
>> ---
>>  drivers/net/wireless/ath/ath9k/main.c |    4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/net/wireless/ath/ath9k/main.c b/drivers/net/wireless/ath/ath9k/main.c
>> index 3879485..215eb25 100644
>> --- a/drivers/net/wireless/ath/ath9k/main.c
>> +++ b/drivers/net/wireless/ath/ath9k/main.c
>> @@ -640,7 +640,7 @@ static void ath_node_attach(struct ath_softc *sc, struct ieee80211_sta *sta,
>>        an->sta = sta;
>>        an->vif = vif;
>>
>> -       if (sta->ht_cap.ht_supported) {
>> +       if (sc->sc_ah->caps.hw_caps & ATH9K_HW_CAP_HT) {
>>                ath_tx_node_init(sc, an);
>>                an->maxampdu = 1 << (IEEE80211_HT_MAX_AMPDU_FACTOR +
>>                                     sta->ht_cap.ampdu_factor);
>> @@ -659,7 +659,7 @@ static void ath_node_detach(struct ath_softc *sc, struct ieee80211_sta *sta)
>>        an->sta = NULL;
>>  #endif
>>
>> -       if (sta->ht_cap.ht_supported)
>> +       if (sc->sc_ah->caps.hw_caps & ATH9K_HW_CAP_HT)
>>                ath_tx_node_cleanup(sc, an);
>>  }
>>
>> --
>> 1.7.9.4
>>
>
>
>
> --
> thanks,
> shafi



-- 
thanks,
shafi

^ permalink raw reply

* Re: [PATCH 1/1] net: phylib: remove the length limitation of mii bus id
From: Dong Aisheng @ 2012-03-20 10:53 UTC (permalink / raw)
  To: Shawn Guo; +Cc: Dong Aisheng, netdev, davem, linux-kernel, linux-arm-kernel
In-Reply-To: <CAAQ0ZWQEoPORMXsf3xV-KPP+hOHk2euj460fORyUALnfPn3Q8g@mail.gmail.com>

On Tue, Mar 20, 2012 at 1:32 AM, Shawn Guo <shawn.guo@linaro.org> wrote:
> On 20 March 2012 12:23, Dong Aisheng <b29396@freescale.com> wrote:
>> From: Dong Aisheng <dong.aisheng@linaro.org>
>>
>> When convert to dt, the length of old mii bus id (17 bytes) is not
>> sufficent to use.
>> For example, the bus id could be 800f0000.ethernet-1:00 in DT.
>>
>> This patch removes the bus id length limitation by changing the
>> bus id to a const char pionter and user could dynamically set the
>> bus id via kasprintf function call.
>>
>> Since then no users use MII_BUS_ID_SIZE any more, just remove it.
>>
>> Signed-off-by: Dong Aisheng <dong.aisheng@linaro.org>
>> ---
>> The simplest way may just change MII_BUS_ID_SIZE to a more bigger size,
>> but i'm not sure that's gonna be accepted.
>
> The simplest fix has been applied on -next tree as below.
>
Oh, i missed it, seems my patch covers that change.
But that patch only fixed wrong phy_name buffer length issue.
My patch is totally remove the length limitation of bus id and phy_name
or the bus id name will be truncated if it's longer than MII_BUS_ID_SIZE
which is not a comfortable limitation.

> commit a7ed07d51c8abdb407be454c6cb6cfad613759d9
> Author: Richard Zhao <richard.zhao@linaro.org>
> Date:   Sun Jan 29 22:08:12 2012 +0000
>
>    net: fec: correct phy_name buffer length when init phy_name
>
>    Fix the bug that we got wrong phy_name on imx6q sabrelite board.
>    snprintf used wrong length of phy_name.
>    phy_name length is MII_BUS_ID_SIZE + 3 rather not MII_BUS_ID_SIZE.
>    I change it to sizeof(phy_name).
>
>    Signed-off-by: Richard Zhao <richard.zhao@linaro.org>
>    Acked-by: Shawn Guo <shawn.guo@linaro.org>
>    Acked-by: Florian Fainelli <florian@openwrt.org>
>    Signed-off-by: David S. Miller <davem@davemloft.net>
>
> diff --git a/drivers/net/ethernet/freescale/fec.c
> b/drivers/net/ethernet/freescale/fec.c
> index 7b25e9c..1c7aad8 100644
> --- a/drivers/net/ethernet/freescale/fec.c
> +++ b/drivers/net/ethernet/freescale/fec.c
> @@ -990,7 +990,7 @@ static int fec_enet_mii_probe(struct net_device *ndev)
>                phy_id = 0;
>        }
>
> -       snprintf(phy_name, MII_BUS_ID_SIZE, PHY_ID_FMT, mdio_bus_id, phy_id);
> +       snprintf(phy_name, sizeof(phy_name), PHY_ID_FMT, mdio_bus_id, phy_id);
>        phy_dev = phy_connect(ndev, phy_name, &fec_enet_adjust_link, 0,
>                              fep->phy_interface);
>        if (IS_ERR(phy_dev)) {

^ permalink raw reply

* Re: [PATCH wireless-next 3/3] ath6kl: Add __printf verification to ath6kl_printk
From: Kalle Valo @ 2012-03-20 10:06 UTC (permalink / raw)
  To: Joe Perches; +Cc: John W. Linville, linux-wireless, netdev, linux-kernel
In-Reply-To: <a27a7b0fcddc2f2366ff5ab0968ca0ff78d420c5.1332116921.git.joe@perches.com>

On 03/19/2012 02:30 AM, Joe Perches wrote:
> Make sure printf formats and arguments match.
> 
> Signed-off-by: Joe Perches <joe@perches.com>

Thanks, I have applied this to ath6kl.git. John, I hope this is ok for you.

Kalle

^ permalink raw reply

* Re: [E1000-devel] [patch net] e1000: fix vlan processing regression
From: Jeff Kirsher @ 2012-03-20 10:03 UTC (permalink / raw)
  To: Jiri Pirko; +Cc: netdev, e1000-devel, davem
In-Reply-To: <1332237473-11311-1-git-send-email-jpirko@redhat.com>

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

On Tue, 2012-03-20 at 10:57 +0100, Jiri Pirko wrote:
> This patch fixes a regression introduced by commit "e1000: do vlan
> cleanup (799d531)".
> 
> Apparently some e1000 chips (not mine) are sensitive about the order
> of
> setting vlan filter and vlan stripping/inserting functionality. So
> this
> patch changes the order so it's the same as before vlan cleanup.
> 
> Reported-by: Ben Greear <greearb@candelatech.com>
> Tested-by: Ben Greear <greearb@candelatech.com>
> Signed-off-by: Jiri Pirko <jpirko@redhat.com>
> ---
>  drivers/net/ethernet/intel/e1000/e1000_main.c |   35
> +++++++++++++++---------
>  1 files changed, 22 insertions(+), 13 deletions(-) 

Thanks Jiri and Ben! I will add this to my queue.

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

^ permalink raw reply

* Re: [PATCH 1/1] ARC VMAC driver.
From: Florian Fainelli @ 2012-03-20 10:00 UTC (permalink / raw)
  To: Andreas Fenkart; +Cc: davem, netdev
In-Reply-To: <1332232945-27869-1-git-send-email-afenkart@gmail.com>

Hi Andreas,

Le 03/20/12 09:42, Andreas Fenkart a écrit :
> This is a driver for the MAC IP block from ARC International. It
> is based on an existing driver found in ARC Linux distribution,
> but essentially, a full rewrite.
>
> Signed-off-by: Andreas Fenkart<afenkart@gmail.com>

I have some phylib-related comments inline.

> ---
>   drivers/net/ethernet/Kconfig   |    9 +
>   drivers/net/ethernet/Makefile  |    1 +
>   drivers/net/ethernet/arcvmac.c | 1472 ++++++++++++++++++++++++++++++++++++++++
>   drivers/net/ethernet/arcvmac.h |  269 ++++++++
>   4 files changed, 1751 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/net/ethernet/Kconfig b/drivers/net/ethernet/Kconfig
> index 597f4d4..4b6baf8 100644
> --- a/drivers/net/ethernet/Kconfig
> +++ b/drivers/net/ethernet/Kconfig
> @@ -23,6 +23,15 @@ source "drivers/net/ethernet/aeroflex/Kconfig"
>   source "drivers/net/ethernet/alteon/Kconfig"
>   source "drivers/net/ethernet/amd/Kconfig"
>   source "drivers/net/ethernet/apple/Kconfig"
> +
> +config ARCVMAC
> +	tristate "ARC VMAC ethernet driver"
> +	select MII
> +	select PHYLIB
> +	select CRC32
> +	help
> +	  MAC present Zoran43xx, IP from ARC international
> +
>   source "drivers/net/ethernet/atheros/Kconfig"
>   source "drivers/net/ethernet/cadence/Kconfig"
>   source "drivers/net/ethernet/adi/Kconfig"
> diff --git a/drivers/net/ethernet/Makefile b/drivers/net/ethernet/Makefile
> index be5dde0..cb61799 100644
> --- a/drivers/net/ethernet/Makefile
> +++ b/drivers/net/ethernet/Makefile
> @@ -9,6 +9,7 @@ obj-$(CONFIG_GRETH) += aeroflex/
>   obj-$(CONFIG_NET_VENDOR_ALTEON) += alteon/
>   obj-$(CONFIG_NET_VENDOR_AMD) += amd/
>   obj-$(CONFIG_NET_VENDOR_APPLE) += apple/
> +obj-$(CONFIG_ARCVMAC) += arcvmac.o
>   obj-$(CONFIG_NET_VENDOR_ATHEROS) += atheros/
>   obj-$(CONFIG_NET_ATMEL) += cadence/
>   obj-$(CONFIG_NET_BFIN) += adi/
> diff --git a/drivers/net/ethernet/arcvmac.c b/drivers/net/ethernet/arcvmac.c
> new file mode 100644
> index 0000000..d512806
> --- /dev/null
> +++ b/drivers/net/ethernet/arcvmac.c
> @@ -0,0 +1,1472 @@
> +/*
> + * ARC VMAC Driver
> + *
> + * Copyright (C) 2009-2012 Andreas Fenkart
> + * All Rights Reserved.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
> + *
> + * Initial work taken from arc linux distribution, any bugs are mine
> + *
> + *	-----<snip>-----
> + * Copyright (C) 2003-2006 Codito Technologies, for linux-2.4 port
> + * Copyright (C) 2006-2007 Celunite Inc, for linux-2.6 port
> + * Authors: amit.bhor@celunite.com, sameer.dhavale@celunite.com
> + *	-----<snip>-----
> + */
> +
> +#include<linux/clk.h>
> +#include<linux/crc32.h>
> +#include<linux/delay.h>
> +#include<linux/dma-mapping.h>
> +#include<linux/etherdevice.h>
> +#include<linux/init.h>
> +#include<linux/interrupt.h>
> +#include<linux/io.h>
> +#include<linux/kernel.h>
> +#include<linux/module.h>
> +#include<linux/moduleparam.h>
> +#include<linux/netdevice.h>
> +#include<linux/phy.h>
> +#include<linux/platform_device.h>
> +#include<linux/slab.h>
> +#include<linux/sched.h>
> +#include<linux/types.h>
> +
> +#include "arcvmac.h"
> +
> +/* Register access macros */
> +#define vmac_writel(port, value, reg)	\
> +	writel(cpu_to_le32(value), (port)->regs + VMAC_##reg)
> +#define vmac_readl(port, reg)	le32_to_cpu(readl((port)->regs + VMAC_##reg))
> +
> +static int get_register_map(struct vmac_priv *ap);
> +static int put_register_map(struct vmac_priv *ap);
> +static void update_vmac_stats_unlocked(struct net_device *dev);
> +static int vmac_tx_reclaim_unlocked(struct net_device *dev, bool force);
> +
> +static unsigned char *read_mac_reg(struct net_device *dev,
> +				   unsigned char hwaddr[ETH_ALEN])
> +{
> +	struct vmac_priv *ap = netdev_priv(dev);
> +	u32 mac_lo, mac_hi;
> +
> +	WARN_ON(!hwaddr);
> +	mac_lo = vmac_readl(ap, ADDRL);
> +	mac_hi = vmac_readl(ap, ADDRH);
> +
> +	hwaddr[0] = (mac_lo>>  0)&  0xff;
> +	hwaddr[1] = (mac_lo>>  8)&  0xff;
> +	hwaddr[2] = (mac_lo>>  16)&  0xff;
> +	hwaddr[3] = (mac_lo>>  24)&  0xff;
> +	hwaddr[4] = (mac_hi>>  0)&  0xff;
> +	hwaddr[5] = (mac_hi>>  8)&  0xff;
> +	return hwaddr;
> +}
> +
> +static void write_mac_reg(struct net_device *dev, unsigned char* hwaddr)
> +{
> +	struct vmac_priv *ap = netdev_priv(dev);
> +	u32 mac_lo, mac_hi;
> +
> +	mac_lo = hwaddr[3]<<  24 | hwaddr[2]<<  16 | hwaddr[1]<<  8 |
> +		hwaddr[0];
> +	mac_hi = hwaddr[5]<<  8 | hwaddr[4];
> +
> +	vmac_writel(ap, mac_lo, ADDRL);
> +	vmac_writel(ap, mac_hi, ADDRH);
> +}
> +
> +static void vmac_mdio_xmit(struct vmac_priv *ap, u32 val)
> +{
> +	init_completion(&ap->mdio_complete);
> +	vmac_writel(ap, val, MDIO_DATA);
> +	wait_for_completion(&ap->mdio_complete);

I would rather switch to a variant which allows a timeout, just in case 
the MDIO transaction never completes, so you are not stuck here, and you 
can guarantee the maximum waiting time. You might also want to test if 
you need the _interruptible variant too in case you have a process like 
ethtool calling into your driver's ethtool ops callbacks. Then you also 
need to propagate the error to the callers.

> +}
> +
> +static int vmac_mdio_read(struct mii_bus *bus, int phy_id, int phy_reg)
> +{
> +	struct vmac_priv *vmac = bus->priv;
> +	u32 val;
> +
> +	/* only 5 bits allowed for phy-addr and reg_offset */
> +	WARN_ON(phy_id&  ~0x1f || phy_reg&  ~0x1f);

A WARN_ON() here is a little too hard, just return -EINVAL.

> +
> +	val = MDIO_BASE | MDIO_OP_READ;
> +	val |= phy_id<<  23 | phy_reg<<  18;
> +	vmac_mdio_xmit(vmac, val);
> +
> +	val = vmac_readl(vmac, MDIO_DATA);
> +	return val&  MDIO_DATA_MASK;
> +}
> +
> +static int vmac_mdio_write(struct mii_bus *bus, int phy_id, int phy_reg,
> +			   u16 value)
> +{
> +	struct vmac_priv *vmac = bus->priv;
> +	u32 val;
> +
> +	/* only 5 bits allowed for phy-addr and reg_offset */
> +	WARN_ON(phy_id&  ~0x1f || phy_reg&  ~0x1f);

Same here.

> +
> +	val = MDIO_BASE | MDIO_OP_WRITE;
> +	val |= phy_id<<  23 | phy_reg<<  18;
> +	val |= (value&  MDIO_DATA_MASK);
> +	vmac_mdio_xmit(vmac, val);
> +
> +	return 0;
> +}
> +
> +static void vmac_handle_link_change(struct net_device *dev)
> +{
> +	struct vmac_priv *ap = netdev_priv(dev);
> +	struct phy_device *phydev = ap->phy_dev;
> +	unsigned long flags;
> +	bool report_change = false;
> +
> +	spin_lock_irqsave(&ap->lock, flags);
> +
> +	if (phydev->duplex != ap->duplex) {
> +		u32 tmp;
> +
> +		tmp = vmac_readl(ap, ENABLE);
> +
> +		if (phydev->duplex)
> +			tmp |= ENFL_MASK;
> +		else
> +			tmp&= ~ENFL_MASK;
> +
> +		vmac_writel(ap, tmp, ENABLE);
> +
> +		ap->duplex = phydev->duplex;
> +		report_change = true;
> +	}
> +
> +	if (phydev->speed != ap->speed) {
> +		ap->speed = phydev->speed;
> +		report_change = true;
> +	}
> +
> +	if (phydev->link != ap->link) {
> +		ap->link = phydev->link;
> +		report_change = true;
> +	}
> +
> +	spin_unlock_irqrestore(&ap->lock, flags);
> +
> +	if (report_change)
> +		phy_print_status(ap->phy_dev);
> +}
> +
> +static int __devinit vmac_mii_probe(struct net_device *dev)
> +{
> +	struct vmac_priv *ap = netdev_priv(dev);
> +	struct phy_device *phydev = NULL;
> +	struct clk *vmac_clk;
> +	unsigned long clock_rate;
> +	int phy_addr, err;
> +
> +	/* find the first phy */
> +	for (phy_addr = 0; phy_addr<  PHY_MAX_ADDR; phy_addr++) {
> +		if (ap->mii_bus->phy_map[phy_addr]) {
> +			phydev = ap->mii_bus->phy_map[phy_addr];
> +			break;
> +		}
> +	}

Use phy_find_first() instead of open-coding the iteration.

> +
> +	if (!phydev) {
> +		dev_err(&ap->pdev->dev, "no PHY found\n");
> +		return -ENODEV;
> +	}
> +
> +	/* FIXME: add pin_irq, if avail */
> +
> +	phydev = phy_connect(dev, dev_name(&phydev->dev),
> +			&vmac_handle_link_change, 0,
> +			     PHY_INTERFACE_MODE_MII);
> +
> +	if (IS_ERR(phydev)) {
> +		err = PTR_ERR(phydev);
> +		dev_err(&ap->pdev->dev, "could not attach to PHY %d\n", err);
> +		goto err_out;
> +	}
> +
> +	phydev->supported&= PHY_BASIC_FEATURES;
> +	phydev->supported |= SUPPORTED_Asym_Pause | SUPPORTED_Pause;
> +
> +	vmac_clk = clk_get(&ap->pdev->dev, "arcvmac");
> +	if (IS_ERR(vmac_clk)) {
> +		err = PTR_ERR(vmac_clk);
> +		goto err_disconnect;
> +	}
> +
> +	clock_rate = clk_get_rate(vmac_clk);
> +	clk_put(vmac_clk);
> +
> +	dev_dbg(&ap->pdev->dev, "vmac_clk: %lu Hz\n", clock_rate);
> +
> +	if (clock_rate<  25000000)
> +		phydev->supported&= ~(SUPPORTED_100baseT_Half |
> +				       SUPPORTED_100baseT_Full);
> +
> +	phydev->advertising = phydev->supported;
> +
> +	ap->link = 0;
> +	ap->speed = 0;
> +	ap->duplex = -1;
> +	ap->phy_dev = phydev;
> +
> +	return 0;
> +
> +err_disconnect:
> +	phy_disconnect(phydev);
> +err_out:
> +	return err;
> +}
> +
> +static int __devinit vmac_mii_init(struct vmac_priv *ap)
> +{
> +	unsigned long flags;
> +	int err, i;
> +
> +	spin_lock_irqsave(&ap->lock, flags);
> +
> +	ap->mii_bus = mdiobus_alloc();
> +	if (!ap->mii_bus)
> +		return -ENOMEM;
> +
> +	ap->mii_bus->name = "vmac_mii_bus";
> +	ap->mii_bus->read =&vmac_mdio_read;
> +	ap->mii_bus->write =&vmac_mdio_write;
> +
> +	snprintf(ap->mii_bus->id, MII_BUS_ID_SIZE, "%x", 0);

Please use an unique name such as:

snprintf(ap->mii_bus->id, MII_BUS_ID_SIZE, "%s-%x", ap->pdev->name, 
ap->pdev->id);

> +
> +	ap->mii_bus->priv = ap;
> +
> +	err = -ENOMEM;
> +	ap->mii_bus->irq = kmalloc(sizeof(int) * PHY_MAX_ADDR, GFP_KERNEL);
> +	if (!ap->mii_bus->irq)
> +		goto err_out;

This is a little unusual, rather do this:
if (!ap->mii_bus->irq) {
	err = -ENOMEM;
	goto err_out;
}

> +
> +	for (i = 0; i<  PHY_MAX_ADDR; i++)
> +		ap->mii_bus->irq[i] = PHY_POLL;
> +
> +	spin_unlock_irqrestore(&ap->lock, flags);
> +
> +	/* locking: mdio concurrency */
> +
> +	err = mdiobus_register(ap->mii_bus);
> +	if (err)
> +		goto err_out_free_mdio_irq;
> +
> +	err = vmac_mii_probe(ap->dev);
> +	if (err)
> +		goto err_out_unregister_bus;
> +
> +	return 0;
> +
> +err_out_unregister_bus:
> +	mdiobus_unregister(ap->mii_bus);
> +err_out_free_mdio_irq:
> +	kfree(ap->mii_bus->irq);
> +err_out:
> +	mdiobus_free(ap->mii_bus);
> +	return err;
> +}
> +
--
Florian

^ permalink raw reply

* [patch net] e1000: fix vlan processing regression
From: Jiri Pirko @ 2012-03-20  9:57 UTC (permalink / raw)
  To: netdev; +Cc: e1000-devel, davem

This patch fixes a regression introduced by commit "e1000: do vlan
cleanup (799d531)".

Apparently some e1000 chips (not mine) are sensitive about the order of
setting vlan filter and vlan stripping/inserting functionality. So this
patch changes the order so it's the same as before vlan cleanup.

Reported-by: Ben Greear <greearb@candelatech.com>
Tested-by: Ben Greear <greearb@candelatech.com>
Signed-off-by: Jiri Pirko <jpirko@redhat.com>
---
 drivers/net/ethernet/intel/e1000/e1000_main.c |   35 +++++++++++++++---------
 1 files changed, 22 insertions(+), 13 deletions(-)

diff --git a/drivers/net/ethernet/intel/e1000/e1000_main.c b/drivers/net/ethernet/intel/e1000/e1000_main.c
index d94d64b..b444f21 100644
--- a/drivers/net/ethernet/intel/e1000/e1000_main.c
+++ b/drivers/net/ethernet/intel/e1000/e1000_main.c
@@ -164,6 +164,8 @@ static int e1000_82547_fifo_workaround(struct e1000_adapter *adapter,
 static bool e1000_vlan_used(struct e1000_adapter *adapter);
 static void e1000_vlan_mode(struct net_device *netdev,
 			    netdev_features_t features);
+static void e1000_vlan_filter_on_off(struct e1000_adapter *adapter,
+				     bool filter_on);
 static int e1000_vlan_rx_add_vid(struct net_device *netdev, u16 vid);
 static int e1000_vlan_rx_kill_vid(struct net_device *netdev, u16 vid);
 static void e1000_restore_vlan(struct e1000_adapter *adapter);
@@ -1213,7 +1215,7 @@ static int __devinit e1000_probe(struct pci_dev *pdev,
 	if (err)
 		goto err_register;
 
-	e1000_vlan_mode(netdev, netdev->features);
+	e1000_vlan_filter_on_off(adapter, false);
 
 	/* print bus type/speed/width info */
 	e_info(probe, "(PCI%s:%dMHz:%d-bit) %pM\n",
@@ -4549,6 +4551,22 @@ static bool e1000_vlan_used(struct e1000_adapter *adapter)
 	return false;
 }
 
+static void __e1000_vlan_mode(struct e1000_adapter *adapter,
+			      netdev_features_t features)
+{
+	struct e1000_hw *hw = &adapter->hw;
+	u32 ctrl;
+
+	ctrl = er32(CTRL);
+	if (features & NETIF_F_HW_VLAN_RX) {
+		/* enable VLAN tag insert/strip */
+		ctrl |= E1000_CTRL_VME;
+	} else {
+		/* disable VLAN tag insert/strip */
+		ctrl &= ~E1000_CTRL_VME;
+	}
+	ew32(CTRL, ctrl);
+}
 static void e1000_vlan_filter_on_off(struct e1000_adapter *adapter,
 				     bool filter_on)
 {
@@ -4558,6 +4576,7 @@ static void e1000_vlan_filter_on_off(struct e1000_adapter *adapter,
 	if (!test_bit(__E1000_DOWN, &adapter->flags))
 		e1000_irq_disable(adapter);
 
+	__e1000_vlan_mode(adapter, adapter->netdev->features);
 	if (filter_on) {
 		/* enable VLAN receive filtering */
 		rctl = er32(RCTL);
@@ -4578,24 +4597,14 @@ static void e1000_vlan_filter_on_off(struct e1000_adapter *adapter,
 }
 
 static void e1000_vlan_mode(struct net_device *netdev,
-	netdev_features_t features)
+			    netdev_features_t features)
 {
 	struct e1000_adapter *adapter = netdev_priv(netdev);
-	struct e1000_hw *hw = &adapter->hw;
-	u32 ctrl;
 
 	if (!test_bit(__E1000_DOWN, &adapter->flags))
 		e1000_irq_disable(adapter);
 
-	ctrl = er32(CTRL);
-	if (features & NETIF_F_HW_VLAN_RX) {
-		/* enable VLAN tag insert/strip */
-		ctrl |= E1000_CTRL_VME;
-	} else {
-		/* disable VLAN tag insert/strip */
-		ctrl &= ~E1000_CTRL_VME;
-	}
-	ew32(CTRL, ctrl);
+	__e1000_vlan_mode(adapter, features);
 
 	if (!test_bit(__E1000_DOWN, &adapter->flags))
 		e1000_irq_enable(adapter);
-- 
1.7.6.4


------------------------------------------------------------------------------
This SF email is sponsosred by:
Try Windows Azure free for 90 days Click Here 
http://p.sf.net/sfu/sfd2d-msazure
_______________________________________________
E1000-devel mailing list
E1000-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/e1000-devel
To learn more about Intel&#174; Ethernet, visit http://communities.intel.com/community/wired

^ permalink raw reply related

* Re: [PATCH v3] ipv6: fix incorrent ipv6 ipsec packet fragment
From: David Miller @ 2012-03-20  9:40 UTC (permalink / raw)
  To: steffen.klassert; +Cc: gaofeng, netdev
In-Reply-To: <20120320085405.GG29891@secunet.com>

From: Steffen Klassert <steffen.klassert@secunet.com>
Date: Tue, 20 Mar 2012 09:54:05 +0100

> On Tue, Mar 20, 2012 at 04:36:10PM +0800, Gao feng wrote:
>> Since commit 299b0767(ipv6: Fix IPsec slowpath fragmentation problem)
>> In func ip6_append_data,after call skb_put(skb, fraglen + dst_exthdrlen)
>> the skb->len contains dst_exthdrlen,and we don't reduce dst_exthdrlen at last
>> This will make fraggap>0 in next "while cycle",and cause the size of skb incorrent
>> 
>> Fix this by reserve headroom for dst_exthdrlen.
>> 
>> Signed-off-by: Gao feng <gaofeng@cn.fujitsu.com>
> 
> Acked-by: Steffen Klassert <steffen.klassert@secunet.com>

Applied and queued up for -stable, thanks.

 

^ permalink raw reply

* Re: use-after-free in usbnet
From: Ming Lei @ 2012-03-20  9:40 UTC (permalink / raw)
  To: Dave Jones; +Cc: netdev, linux-usb, Fedora Kernel Team
In-Reply-To: <20120319151224.GA16031@redhat.com>

Hi,

On Mon, Mar 19, 2012 at 11:12 PM, Dave Jones <davej@redhat.com> wrote:
> We've had two reports of this use after free in Fedora now recently..

Could you provide output of 'dmesg' and 'lsusb -v' from the reported machine?



Thanks,
-- 
Ming Lei

^ permalink raw reply

* Re: ath: DMA failed to stop in 10 ms AR_CR=0x00000024 AR_DIAG_SW=0x02000020 DMADBG_7=0x00008040
From: Mohammed Shafi @ 2012-03-20  9:37 UTC (permalink / raw)
  To: Sujith Manoharan
  Cc: Justin P. Mattock, Linux-wireless, Felix Fietkau, linux-kernel,
	Linux-netdev
In-Reply-To: <20328.16366.166909.255035@gargle.gargle.HOWL>

Hi Sujith/Justin,

On Tue, Mar 20, 2012 at 1:59 PM, Sujith Manoharan
<c_manoha@qca.qualcomm.com> wrote:
> Justin P. Mattock wrote:
>> yeah this works:
>>
>> eading symbols from
>> /home/kernel/linux-next/drivers/net/wireless/ath/ath9k/ath9k.o...done.
>> (gdb) l *(ath_tx_start+0x284)
>> 0xcad4 is in ath_tx_start (drivers/net/wireless/ath/ath9k/xmit.c:1878).
>> 1873                  ieee80211_is_data_qos(hdr->frame_control)) {
>> 1874                  tidno = ieee80211_get_qos_ctl(hdr)[0] &
>> 1875                          IEEE80211_QOS_CTL_TID_MASK;
>> 1876                  tid = ATH_AN_2_TID(txctl->an, tidno);
>> 1877
>> 1878                  WARN_ON(tid->ac->txq != txctl->txq);
>> 1879          }
>> 1880
>> 1881          if ((tx_info->flags & IEEE80211_TX_CTL_AMPDU) && tid) {
>> 1882                  /*
>> (gdb)
>
> Can you try this patch ?

just found out that 'ht_supported' may not  be set, if assoc response
does not has ht_cap IE (or) if we could not parse it (why), then the
driver won't initialize those tid related structures ath_tx_node_init,
while we later access them in ath_tx_start. so this should fix the
issue.

>
> From: Sujith Manoharan <c_manoha@qca.qualcomm.com>
> Date: Tue, 20 Mar 2012 13:51:26 +0530
> Subject: [PATCH] ath9k: Use HW HT capabilites properly
>
> The commit "ath9k: Remove aggregation flags" changed how
> nodes were being initialized. Use the HW HT cap bits
> to initialize/de-initialize nodes, else we would be
> accessing an uninitialized entry during a suspend/resume cycle,
> resulting in a panic.
>
> Reported-by: Justin P. Mattock <justinmattock@gmail.com>
> Signed-off-by: Sujith Manoharan <c_manoha@qca.qualcomm.com>
> ---
>  drivers/net/wireless/ath/ath9k/main.c |    4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/wireless/ath/ath9k/main.c b/drivers/net/wireless/ath/ath9k/main.c
> index 3879485..215eb25 100644
> --- a/drivers/net/wireless/ath/ath9k/main.c
> +++ b/drivers/net/wireless/ath/ath9k/main.c
> @@ -640,7 +640,7 @@ static void ath_node_attach(struct ath_softc *sc, struct ieee80211_sta *sta,
>        an->sta = sta;
>        an->vif = vif;
>
> -       if (sta->ht_cap.ht_supported) {
> +       if (sc->sc_ah->caps.hw_caps & ATH9K_HW_CAP_HT) {
>                ath_tx_node_init(sc, an);
>                an->maxampdu = 1 << (IEEE80211_HT_MAX_AMPDU_FACTOR +
>                                     sta->ht_cap.ampdu_factor);
> @@ -659,7 +659,7 @@ static void ath_node_detach(struct ath_softc *sc, struct ieee80211_sta *sta)
>        an->sta = NULL;
>  #endif
>
> -       if (sta->ht_cap.ht_supported)
> +       if (sc->sc_ah->caps.hw_caps & ATH9K_HW_CAP_HT)
>                ath_tx_node_cleanup(sc, an);
>  }
>
> --
> 1.7.9.4
>



-- 
thanks,
shafi

^ permalink raw reply

* Re: WARNING: at net/sched/sch_generic.c:256 dev_watchdog+0x1f2/0x200()
From: Francois Romieu @ 2012-03-20  9:37 UTC (permalink / raw)
  To: Justin Mattock; +Cc: linux-kernel, netdev
In-Reply-To: <CAKFRV=NghW3kokuD-VBAzVJf09r-9ePMG+L7qDYfqmuHwWo4mg@mail.gmail.com>

(Larry removed)

Justin Mattock <justinmattock@gmail.com> :
[...]
> seems I see this with the latest linux-next:

Thanks for testing.

[...]
> [21740.318685] r8169 0000:06:00.0: eth0: link up
> [21752.292679] r8169 0000:06:00.0: eth0: link up
> [21764.268569] r8169 0000:06:00.0: eth0: link up
> [21776.254393] r8169 0000:06:00.0: eth0: link up
> [21788.235797] r8169 0000:06:00.0: eth0: link up
> [21800.196524] r8169 0000:06:00.0: eth0: link up
> [21812.172497] r8169 0000:06:00.0: eth0: link up

This is completely broken. I could understand a few up/down link changes
until things settles but the driver should not claim periodically that
the link is up when there is no cable, at least not with a supported chipset.

Can you apply the debug helper below and report a complete dmesg from
boot with the same test (please remove l-k, netdev is good enough) ?

diff --git a/drivers/net/ethernet/realtek/r8169.c b/drivers/net/ethernet/realtek/r8169.c
index 61e6ab4..880264a 100644
--- a/drivers/net/ethernet/realtek/r8169.c
+++ b/drivers/net/ethernet/realtek/r8169.c
@@ -61,7 +61,8 @@
 #endif /* RTL8169_DEBUG */
 
 #define R8169_MSG_DEFAULT \
-	(NETIF_MSG_DRV | NETIF_MSG_PROBE | NETIF_MSG_IFUP | NETIF_MSG_IFDOWN)
+	(NETIF_MSG_DRV | NETIF_MSG_PROBE | NETIF_MSG_IFUP | NETIF_MSG_IFDOWN | \
+	 NETIF_MSG_LINK)
 
 #define TX_BUFFS_AVAIL(tp) \
 	(tp->dirty_tx + NUM_TX_DESC - tp->cur_tx - 1)
@@ -731,7 +732,7 @@ struct rtl8169_private {
 	void (*phy_reset_enable)(struct rtl8169_private *tp);
 	void (*hw_start)(struct net_device *);
 	unsigned int (*phy_reset_pending)(struct rtl8169_private *tp);
-	unsigned int (*link_ok)(void __iomem *);
+	unsigned int (*link_ok)(struct rtl8169_private *);
 	int (*do_ioctl)(struct rtl8169_private *tp, struct mii_ioctl_data *data, int cmd);
 
 	struct {
@@ -1260,14 +1261,28 @@ static unsigned int rtl8169_xmii_reset_pending(struct rtl8169_private *tp)
 	return rtl_readphy(tp, MII_BMCR) & BMCR_RESET;
 }
 
-static unsigned int rtl8169_tbi_link_ok(void __iomem *ioaddr)
+static unsigned int rtl8169_tbi_link_ok(struct rtl8169_private *tp)
 {
+	void __iomem *ioaddr = tp->mmio_addr;
+
 	return RTL_R32(TBICSR) & TBILinkOk;
 }
 
-static unsigned int rtl8169_xmii_link_ok(void __iomem *ioaddr)
+static unsigned int rtl8169_xmii_link_ok(struct rtl8169_private *tp)
 {
-	return RTL_R8(PHYstatus) & LinkStatus;
+	void __iomem *ioaddr = tp->mmio_addr;
+	struct net_device *dev = tp->dev;
+	u8 status;
+
+	status = RTL_R8(PHYstatus) & LinkStatus;
+	netif_info(tp, link, dev,
+		   "bmcr: %04x bmsr: %04x gbcr: %04x gbsr: %04x\n",
+		   rtl_readphy(tp, MII_BMCR),
+		   rtl_readphy(tp, MII_BMSR),
+		   rtl_readphy(tp, MII_CTRL1000),
+		   rtl_readphy(tp, MII_STAT1000));
+
+	return status;
 }
 
 static void rtl8169_tbi_reset_enable(struct rtl8169_private *tp)
@@ -1335,7 +1350,7 @@ static void __rtl8169_check_link_status(struct net_device *dev,
 					struct rtl8169_private *tp,
 					void __iomem *ioaddr, bool pm)
 {
-	if (tp->link_ok(ioaddr)) {
+	if (tp->link_ok(tp)) {
 		rtl_link_chg_patch(tp);
 		/* This is to cancel a scheduled suspend if there's one. */
 		if (pm)
@@ -3309,7 +3324,6 @@ static void rtl_hw_phy_config(struct net_device *dev)
 static void rtl_phy_work(struct rtl8169_private *tp)
 {
 	struct timer_list *timer = &tp->timer;
-	void __iomem *ioaddr = tp->mmio_addr;
 	unsigned long timeout = RTL8169_PHY_TIMEOUT;
 
 	assert(tp->mac_version > RTL_GIGA_MAC_VER_01);
@@ -3323,7 +3337,7 @@ static void rtl_phy_work(struct rtl8169_private *tp)
 		goto out_mod_timer;
 	}
 
-	if (tp->link_ok(ioaddr))
+	if (tp->link_ok(tp))
 		return;
 
 	netif_warn(tp, link, tp->dev, "PHY reset until link up\n");
-- 
1.7.7.6

^ permalink raw reply related

* [PATCH] e1000: Silence sparse warnings by correcting type
From: Andrei Emeltchenko @ 2012-03-20  8:59 UTC (permalink / raw)
  To: netdev

From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>

Silence sparse warnings shown below:
...
drivers/net/ethernet/intel/e1000/e1000_main.c:3435:17: warning:
	cast to restricted __le64
drivers/net/ethernet/intel/e1000/e1000_main.c:3435:17: warning:
	cast to restricted __le64
...

Signed-off-by: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
---
 drivers/net/ethernet/intel/e1000/e1000_main.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/intel/e1000/e1000_main.c b/drivers/net/ethernet/intel/e1000/e1000_main.c
index 6419a88..1bf73cf 100644
--- a/drivers/net/ethernet/intel/e1000/e1000_main.c
+++ b/drivers/net/ethernet/intel/e1000/e1000_main.c
@@ -3377,7 +3377,7 @@ static void e1000_dump(struct e1000_adapter *adapter)
 	for (i = 0; tx_ring->desc && (i < tx_ring->count); i++) {
 		struct e1000_tx_desc *tx_desc = E1000_TX_DESC(*tx_ring, i);
 		struct e1000_buffer *buffer_info = &tx_ring->buffer_info[i];
-		struct my_u { u64 a; u64 b; };
+		struct my_u { __le64 a; __le64 b; };
 		struct my_u *u = (struct my_u *)tx_desc;
 		const char *type;
 
@@ -3421,7 +3421,7 @@ rx_ring_summary:
 	for (i = 0; rx_ring->desc && (i < rx_ring->count); i++) {
 		struct e1000_rx_desc *rx_desc = E1000_RX_DESC(*rx_ring, i);
 		struct e1000_buffer *buffer_info = &rx_ring->buffer_info[i];
-		struct my_u { u64 a; u64 b; };
+		struct my_u { __le64 a; __le64 b; };
 		struct my_u *u = (struct my_u *)rx_desc;
 		const char *type;
 
-- 
1.7.9.1

^ permalink raw reply related


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