Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH net-next-2.6] ixgbe: Use the instance of net_device_stats from net_device.
From: Jeff Kirsher @ 2009-10-07 23:34 UTC (permalink / raw)
  To: Ajit Khaparde; +Cc: davem, netdev
In-Reply-To: <20091007124339.GA30083@serverengines.com>

On Wed, Oct 7, 2009 at 05:43, Ajit Khaparde <ajitkhaparde@gmail.com> wrote:
> Since net_device has an instance of net_device_stats,
> we can remove the instance of this from the private adapter structure.
>
> Signed-off-by: Ajit Khaparde <ajitk@serverengines.com>
> ---
>  drivers/net/ixgbe/ixgbe.h         |    1 -
>  drivers/net/ixgbe/ixgbe_ethtool.c |   40 +++++++++++++++++++-----------------
>  drivers/net/ixgbe/ixgbe_main.c    |   26 ++++++++++++------------
>  3 files changed, 34 insertions(+), 33 deletions(-)
>

I have added this patch to my tree for testing purposes, thanks.

-- 
Cheers,
Jeff

^ permalink raw reply

* Re: [PATCH net-next-2.6] e1000e: Use the instance of net_device_stats from net_device.
From: Jeff Kirsher @ 2009-10-07 23:36 UTC (permalink / raw)
  To: Ajit Khaparde; +Cc: davem, netdev
In-Reply-To: <20091007124416.GA30096@serverengines.com>

On Wed, Oct 7, 2009 at 05:44, Ajit Khaparde <ajitkhaparde@gmail.com> wrote:
> Since net_device has an instance of net_device_stats,
> we can remove the instance of this from the private adapter structure.
>
> Signed-off-by: Ajit Khaparde <ajitk@serverengines.com>
> ---
>  drivers/net/e1000e/e1000.h   |    1 -
>  drivers/net/e1000e/ethtool.c |   18 +++++++++-------
>  drivers/net/e1000e/netdev.c  |   43 ++++++++++++++++++++---------------------
>  3 files changed, 31 insertions(+), 31 deletions(-)
>

I have added this patch to my tree for testing purposes, thanks.

-- 
Cheers,
Jeff

^ permalink raw reply

* Re: [PATCH net-next-2.6] ixgb: Use the instance of net_device_stats from net_device.
From: Jeff Kirsher @ 2009-10-07 23:38 UTC (permalink / raw)
  To: Ajit Khaparde; +Cc: davem, netdev
In-Reply-To: <20091007124648.GA30172@serverengines.com>

On Wed, Oct 7, 2009 at 05:46, Ajit Khaparde <ajitkhaparde@gmail.com> wrote:
> Since net_device has an instance of net_device_stats,
> we can remove the instance of this from the private adapter structure.
>
> Signed-off-by: Ajit Khaparde <ajitk@serverengines.com>
> ---
>  drivers/net/ixgb/ixgb.h         |    1 -
>  drivers/net/ixgb/ixgb_ethtool.c |   44 ++++++++++++++++++++------------------
>  drivers/net/ixgb/ixgb_main.c    |   44 ++++++++++++++++++--------------------
>  3 files changed, 44 insertions(+), 45 deletions(-)
>

I have added this patch to my tree for testing purposes, thanks.

-- 
Cheers,
Jeff

^ permalink raw reply

* Re: [PATCH] net: Make UFO on master device independent of attached devices
From: Herbert Xu @ 2009-10-07 23:41 UTC (permalink / raw)
  To: Sridhar Samudrala; +Cc: David Miller, netdev
In-Reply-To: <1254954265.31575.191.camel@w-sridhar.beaverton.ibm.com>

On Wed, Oct 07, 2009 at 03:24:25PM -0700, Sridhar Samudrala wrote:
> Now that software UFO is supported, UFO can be enabled on master
> devices like bridge, bond even though the attached device doesn't
> support this feature in hardware.
> 
> This allows UFO to be used between KVM host and guest even when a
> physical interface attached to the bridge doesn't support UFO.
> 
> Signed-off-by: Sridhar Samudrala <sri@us.ibm.com>

Acked-by: Herbert Xu <herbert@gondor.apana.org.au>
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply

* Re: [PATCH] Add sk_mark route lookup support for IPv4 listening sockets, and for IPv4 multicast forwarding
From: Maciej Żenczykowski @ 2009-10-08  0:03 UTC (permalink / raw)
  To: David Miller; +Cc: atis, netdev, panther, eric.dumazet, brian.haley
In-Reply-To: <20091007.135627.96995518.davem@davemloft.net>

Should wrapping a packet into a tunnel clear the mark?
Should perhaps the mark be a parameter of the tunnel (like ttl or qos,
can be set or can be inherited)?

On Wed, Oct 7, 2009 at 13:56, David Miller <davem@davemloft.net> wrote:
> From: Atis Elsts <atis@mikrotik.com>
> Date: Wed, 7 Oct 2009 15:59:56 +0300
>
>> Here is the sk_mark part.
>
> Applied, thanks.
>
>> As for the ipmr.c code, I agree with your comment. Using mark from
>> skb probably is wrong in case of tunnel interface (i.e. in the "if
>> (vif->flags&VIFF_TUNNEL)" part of the patch), my mistake. I still
>> think that the "else" part is correct, though, because using mark
>> from skb there mirrors behaviour for unicast forwarding routing
>> lookup in ip_route_input_slow(). The same applies to IPv6 code in
>> ip6mr_forward2().
>
> Ok submit just the else part and we'll have a look at it.
>
> Thanks.
>

^ permalink raw reply

* [PATCH 2/2] ethoc: clear only pending irqs
From: Thomas Chou @ 2009-10-08  0:16 UTC (permalink / raw)
  To: netdev; +Cc: thierry.reding, Nios2 development list, linux-kernel, Thomas Chou
In-Reply-To: <1254961003-2453-1-git-send-email-thomas@wytron.com.tw>

This patch fixed the problem of dropped packets due to lost of
interrupt requests. We should only clear what was pending at the
moment we read the irq source reg.

Signed-off-by: Thomas Chou <thomas@wytron.com.tw>
---
 drivers/net/ethoc.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/net/ethoc.c b/drivers/net/ethoc.c
index 6b39723..ecc53d9 100644
--- a/drivers/net/ethoc.c
+++ b/drivers/net/ethoc.c
@@ -509,7 +509,7 @@ static irqreturn_t ethoc_interrupt(int irq, void *dev_id)
 		return IRQ_NONE;
 	}
 
-	ethoc_ack_irq(priv, INT_MASK_ALL);
+	ethoc_ack_irq(priv, pending);
 
 	if (pending & INT_MASK_BUSY) {
 		dev_err(&dev->dev, "packet dropped\n");
-- 
1.6.2.5


^ permalink raw reply related

* [PATCH 1/2] ethoc: inline regs access
From: Thomas Chou @ 2009-10-08  0:16 UTC (permalink / raw)
  To: netdev; +Cc: thierry.reding, Nios2 development list, linux-kernel, Thomas Chou

Signed-off-by: Thomas Chou <thomas@wytron.com.tw>
---
 drivers/net/ethoc.c |   19 ++++++++++---------
 1 files changed, 10 insertions(+), 9 deletions(-)

diff --git a/drivers/net/ethoc.c b/drivers/net/ethoc.c
index 22df0a6..6b39723 100644
--- a/drivers/net/ethoc.c
+++ b/drivers/net/ethoc.c
@@ -222,24 +222,25 @@ struct ethoc_bd {
 	u32 addr;
 };
 
-static u32 ethoc_read(struct ethoc *dev, loff_t offset)
+static inline u32 ethoc_read(struct ethoc *dev, loff_t offset)
 {
 	return ioread32(dev->iobase + offset);
 }
 
-static void ethoc_write(struct ethoc *dev, loff_t offset, u32 data)
+static inline void ethoc_write(struct ethoc *dev, loff_t offset, u32 data)
 {
 	iowrite32(data, dev->iobase + offset);
 }
 
-static void ethoc_read_bd(struct ethoc *dev, int index, struct ethoc_bd *bd)
+static inline void ethoc_read_bd(struct ethoc *dev, int index,
+		struct ethoc_bd *bd)
 {
 	loff_t offset = ETHOC_BD_BASE + (index * sizeof(struct ethoc_bd));
 	bd->stat = ethoc_read(dev, offset + 0);
 	bd->addr = ethoc_read(dev, offset + 4);
 }
 
-static void ethoc_write_bd(struct ethoc *dev, int index,
+static inline void ethoc_write_bd(struct ethoc *dev, int index,
 		const struct ethoc_bd *bd)
 {
 	loff_t offset = ETHOC_BD_BASE + (index * sizeof(struct ethoc_bd));
@@ -247,33 +248,33 @@ static void ethoc_write_bd(struct ethoc *dev, int index,
 	ethoc_write(dev, offset + 4, bd->addr);
 }
 
-static void ethoc_enable_irq(struct ethoc *dev, u32 mask)
+static inline void ethoc_enable_irq(struct ethoc *dev, u32 mask)
 {
 	u32 imask = ethoc_read(dev, INT_MASK);
 	imask |= mask;
 	ethoc_write(dev, INT_MASK, imask);
 }
 
-static void ethoc_disable_irq(struct ethoc *dev, u32 mask)
+static inline void ethoc_disable_irq(struct ethoc *dev, u32 mask)
 {
 	u32 imask = ethoc_read(dev, INT_MASK);
 	imask &= ~mask;
 	ethoc_write(dev, INT_MASK, imask);
 }
 
-static void ethoc_ack_irq(struct ethoc *dev, u32 mask)
+static inline void ethoc_ack_irq(struct ethoc *dev, u32 mask)
 {
 	ethoc_write(dev, INT_SOURCE, mask);
 }
 
-static void ethoc_enable_rx_and_tx(struct ethoc *dev)
+static inline void ethoc_enable_rx_and_tx(struct ethoc *dev)
 {
 	u32 mode = ethoc_read(dev, MODER);
 	mode |= MODER_RXEN | MODER_TXEN;
 	ethoc_write(dev, MODER, mode);
 }
 
-static void ethoc_disable_rx_and_tx(struct ethoc *dev)
+static inline void ethoc_disable_rx_and_tx(struct ethoc *dev)
 {
 	u32 mode = ethoc_read(dev, MODER);
 	mode &= ~(MODER_RXEN | MODER_TXEN);
-- 
1.6.2.5

^ permalink raw reply related

* Re: [PATCH net-next-2.6] ixgbe: Use the instance of net_device_stats from net_device.
From: Jeff Kirsher @ 2009-10-08  0:45 UTC (permalink / raw)
  To: Ajit Khaparde; +Cc: davem, netdev
In-Reply-To: <9929d2390910071634o42210a96s87e5703080d825b3@mail.gmail.com>

On Wed, Oct 7, 2009 at 16:34, Jeff Kirsher <jeffrey.t.kirsher@intel.com> wrote:
> On Wed, Oct 7, 2009 at 05:43, Ajit Khaparde <ajitkhaparde@gmail.com> wrote:
>> Since net_device has an instance of net_device_stats,
>> we can remove the instance of this from the private adapter structure.
>>
>> Signed-off-by: Ajit Khaparde <ajitk@serverengines.com>
>> ---
>>  drivers/net/ixgbe/ixgbe.h         |    1 -
>>  drivers/net/ixgbe/ixgbe_ethtool.c |   40 +++++++++++++++++++-----------------
>>  drivers/net/ixgbe/ixgbe_main.c    |   26 ++++++++++++------------
>>  3 files changed, 34 insertions(+), 33 deletions(-)
>>

Acked-by: Peter P Waskiewicz Jr <peter.p.waskiewicz.jr@intel.com>

^ permalink raw reply

* Re: [PATCH] Generalize socket rx gap / receive queue overflow cmsg
From: Eric Dumazet @ 2009-10-08  1:05 UTC (permalink / raw)
  To: Neil Horman; +Cc: netdev, davem, socketcan
In-Reply-To: <20091007180835.GB20524@hmsreliant.think-freely.org>

Neil Horman a écrit :
> diff --git a/net/core/sock.c b/net/core/sock.c
> index 7626b6a..8bd366f 100644
> --- a/net/core/sock.c
> +++ b/net/core/sock.c
> @@ -306,6 +306,7 @@ int sock_queue_rcv_skb(struct sock *sk, struct sk_buff *skb)
>  	skb_len = skb->len;
>  


>  	skb_queue_tail(&sk->sk_receive_queue, skb);
> +	skb->dropcount = atomic_read(&sk->sk_drops);

No, skb was given to skb_queue_tail(), you are not allowed to touch it now,
another cpu might already consume it.

You better do :

struct sk_buff_head *list = &sk->sk_receive_queue;

spin_lock_irqsave(&list->lock, flags);
skb->dropcount = atomic_read(&sk->sk_drops); // should be done under lock protection
__skb_queue_tail(list, newsk);
spin_unlock_irqrestore(&list->lock, flags);



>  
>  	if (!sock_flag(sk, SOCK_DEAD))
>  		sk->sk_data_ready(sk, skb_len);
> @@ -702,6 +703,12 @@ set_rcvbuf:
>  
>  		/* We implement the SO_SNDLOWAT etc to
>  		   not be settable (1003.1g 5.3) */
> +	case SO_RXQ_OVFL:
> +		if (valbool)
> +			set_bit(SOCK_RXQ_OVFL, &sock->flags);
> +		else
> +			clear_bit(SOCK_RXQ_OVFL, &sock->flags);
> +		break;
>  	default:
>  		ret = -ENOPROTOOPT;
>  		break;
> @@ -901,6 +908,10 @@ int sock_getsockopt(struct socket *sock, int level, int optname,
>  		v.val = sk->sk_mark;
>  		break;
>  
> +	case SO_RXQ_OVFL:
> +		v.val = test_bit(SOCK_RXQ_OVFL, &sock->flags);
> +		break;
> +
>  	default:
>  		return -ENOPROTOOPT;
>  	}
> diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c
> index d7ecca0..920ae1e 100644
> --- a/net/packet/af_packet.c
> +++ b/net/packet/af_packet.c
> @@ -617,6 +617,7 @@ static int packet_rcv(struct sk_buff *skb, struct net_device *dev,
>  	if (pskb_trim(skb, snaplen))
>  		goto drop_n_acct;
>  

> +	skb->dropcount = atomic_read(&sk->sk_drops);
This should be done a litle bit after, right before "__skb_queue_tail(&sk->sk_receive_queue, skb); "

>  	skb_set_owner_r(skb, sk);
>  	skb->dev = NULL;
>  	skb_dst_drop(skb);
> @@ -634,6 +635,7 @@ static int packet_rcv(struct sk_buff *skb, struct net_device *dev,
>  drop_n_acct:


>  	spin_lock(&sk->sk_receive_queue.lock);
>  	po->stats.tp_drops++;
> +	atomic_inc(&sk->sk_drops);
>  	spin_unlock(&sk->sk_receive_queue.lock);

You could replace this block of four lines by : po->stat.tp_drop = atomic_inc_return(&sk->sk_drops);

>  
>  drop_n_restore:
> diff --git a/net/socket.c b/net/socket.c
> index 7565536..ad157a3 100644
> --- a/net/socket.c
> +++ b/net/socket.c
> @@ -673,6 +673,12 @@ static inline int __sock_recvmsg(struct kiocb *iocb, struct socket *sock,
>  {
>  	int err;
>  	struct sock_iocb *si = kiocb_to_siocb(iocb);
> +	struct sk_buff *skb;
> +	int rc;
> +	struct sock *sk = sock->sk;
> +	unsigned long cpu_flags;
> +	__u32 gap = 0;

> +	int check_drops = test_bit(SOCK_RXQ_OVFL, &sock->flags);
>  
>  	si->sock = sock;
>  	si->scm = NULL;
> @@ -684,7 +690,21 @@ static inline int __sock_recvmsg(struct kiocb *iocb, struct socket *sock,
>  	if (err)
>  		return err;
>  
> -	return sock->ops->recvmsg(iocb, sock, msg, size, flags);




> +	if (check_drops) {
> +		skb = skb_recv_datagram(sk, flags|MSG_PEEK,
> +				flags & MSG_DONTWAIT, &err);

	Ouch, this is too expensive, please find another way :)

> +		if (skb) {
> +			gap = skb->dropcount;
> +			consume_skb(skb);
> +		}
> +	}
> +
> +	rc = sock->ops->recvmsg(iocb, sock, msg, size, flags);
> +
> +	if (check_drops && (rc > 0))

		&& gap != 0

> +		put_cmsg(msg, SOL_SOCKET, SO_RXQ_OVFL, sizeof(__u32), &gap);
> +


^ permalink raw reply

* Re: [BUG net-2.6] bluetooth/rfcomm : sleeping function called from invalid context at mm/slub.c:1719
From: Dave Young @ 2009-10-08  1:25 UTC (permalink / raw)
  To: Oliver Hartkopp
  Cc: Marcel Holtmann, Linux Netdev List,
	linux-bluetooth-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <4ACCCCF5.9060203-fJ+pQTUTwRTk1uMJSBkQmQ@public.gmane.org>

On Thu, Oct 8, 2009 at 1:16 AM, Oliver Hartkopp <oliver-fJ+pQTUTwRTk1uMJSBkQmQ@public.gmane.org> wrote:
> Oliver Hartkopp wrote:
>> Dave Young wrote:
>>> On Fri, Oct 2, 2009 at 2:28 PM, Oliver Hartkopp <oliver-fJ+pQTUTwRSDGRHsOpWV0g@public.gmane.orgt> wrote:
>>>> Hello Marcel,
>>>>
>>>> with current net-2.6 tree ...
>>>>
>>>> While starting my PPP Bluetooth dialup networking, i got this:
>>> Hi, oliver
>>>
>>> please try following patch:
>>> http://patchwork.kernel.org/patch/51326/
>>
>> Hi Dave,
>>
>> that fixed it at ppp startup!
>>
>> Tested-by: Oliver Hartkopp <oliver-fJ+pQTUTwRTk1uMJSBkQmQ@public.gmane.org>
>
> Hi Dave,
>
> what's the state of this patch?
>
> Has it gone upstream?

Not yet.

Marcel, could you pick it?  If there's potential problem please tell.

>
> Regards,
> Oliver
>
>>
>> Btw. when shutting down the ppp connection i still get this:
>>
>> [  361.996887] INFO: trying to register non-static key.
>> [  361.996897] the code is fine but needs lockdep annotation.
>> [  361.996902] turning off the locking correctness validator.
>> [  361.996912] Pid: 0, comm: swapper Not tainted 2.6.31-08939-gdb8abec-dirty #22
>> [  361.996919] Call Trace:
>> [  361.996933]  [<c12e4fb2>] ? printk+0xf/0x11
>> [  361.996947]  [<c1042214>] register_lock_class+0x5a/0x295
>> [  361.996957]  [<c1043af2>] __lock_acquire+0x9b/0xc03
>> [  361.996967]  [<c104464b>] ? __lock_acquire+0xbf4/0xc03
>> [  361.996985]  [<fa59a168>] ? l2cap_get_chan_by_scid+0x35/0x43 [l2cap]
>> [  361.996995]  [<c104491f>] ? lock_release_non_nested+0x17b/0x1db
>> [  361.997008]  [<fa59a168>] ? l2cap_get_chan_by_scid+0x35/0x43 [l2cap]
>> [  361.997018]  [<c10426fd>] ? trace_hardirqs_off+0xb/0xd
>> [  361.997028]  [<c10446b6>] lock_acquire+0x5c/0x73
>> [  361.997039]  [<c124cd14>] ? skb_dequeue+0x12/0x4c
>> [  361.997049]  [<c12e6e23>] _spin_lock_irqsave+0x24/0x34
>> [  361.997058]  [<c124cd14>] ? skb_dequeue+0x12/0x4c
>> [  361.997066]  [<c124cd14>] skb_dequeue+0x12/0x4c
>> [  361.997075]  [<c124d579>] skb_queue_purge+0x14/0x1b
>> [  361.997088]  [<fa59ce3f>] l2cap_recv_frame+0xe9e/0x129a [l2cap]
>> [  361.997099]  [<c10421d1>] ? register_lock_class+0x17/0x295
>> [  361.997110]  [<c104464b>] ? __lock_acquire+0xbf4/0xc03
>> [  361.997128]  [<c104464b>] ? __lock_acquire+0xbf4/0xc03
>> [  361.997139]  [<c120de74>] ? uhci_giveback_urb+0xf2/0x162
>> [  361.997163]  [<f8bb4c45>] ? hci_rx_task+0xfe/0x1f8 [bluetooth]
>> [  361.997177]  [<fa59d2e4>] l2cap_recv_acldata+0xa9/0x1be [l2cap]
>> [  361.997190]  [<fa59d23b>] ? l2cap_recv_acldata+0x0/0x1be [l2cap]
>> [  361.997208]  [<f8bb4c77>] hci_rx_task+0x130/0x1f8 [bluetooth]
>> [  361.997219]  [<c102a098>] tasklet_action+0x6b/0xb2
>> [  361.997228]  [<c102a46b>] __do_softirq+0x82/0x101
>> [  361.997237]  [<c102a515>] do_softirq+0x2b/0x43
>> [  361.997246]  [<c102a619>] irq_exit+0x35/0x68
>> [  361.997256]  [<c1004513>] do_IRQ+0x80/0x96
>> [  361.997265]  [<c10030ae>] common_interrupt+0x2e/0x34
>> [  361.997275]  [<c104007b>] ? tick_device_uses_broadcast+0x71/0x7c
>> [  361.997286]  [<c11747a8>] ? acpi_idle_enter_simple+0x103/0x12e
>> [  361.997296]  [<c1174515>] acpi_idle_enter_bm+0xc3/0x253
>> [  361.997306]  [<c1238b6f>] cpuidle_idle_call+0x60/0x91
>> [  361.997315]  [<c1001d44>] cpu_idle+0x49/0x65
>> [  361.997324]  [<c12e2f0e>] start_secondary+0x190/0x195
>>
>>
>> Thanks,
>> Oliver
>>
>>
>
>



-- 
Regards
dave

^ permalink raw reply

* [PATCH] ethoc: use NET_IP_ALIGN in skb_reserve()
From: Thomas Chou @ 2009-10-08  2:32 UTC (permalink / raw)
  To: netdev; +Cc: thierry.reding, Nios2 development list, linux-kernel, Thomas Chou

As suggested by Stephen Hemminger.

Signed-off-by: Thomas Chou <thomas@wytron.com.tw>
---
 drivers/net/ethoc.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/net/ethoc.c b/drivers/net/ethoc.c
index ecc53d9..4a1ed81 100644
--- a/drivers/net/ethoc.c
+++ b/drivers/net/ethoc.c
@@ -409,7 +409,7 @@ static int ethoc_rx(struct net_device *dev, int limit)
 			struct sk_buff *skb = netdev_alloc_skb(dev, size);
 
 			size -= 4; /* strip the CRC */
-			skb_reserve(skb, 2); /* align TCP/IP header */
+			skb_reserve(skb, NET_IP_ALIGN);
 
 			if (likely(skb)) {
 				void *src = phys_to_virt(bd.addr);
-- 
1.6.2.5


^ permalink raw reply related

* Re: [PATCH 3/4] ethoc: align received packet to make IP header at word boundary
From: Thomas Chou @ 2009-10-08  2:42 UTC (permalink / raw)
  To: David Miller; +Cc: shemminger, netdev
In-Reply-To: <20091007.135217.73491149.davem@davemloft.net>

On 10/08/2009 04:52 AM, David Miller wrote:
> From: Stephen Hemminger<shemminger@vyatta.com>
> Date: Wed, 7 Oct 2009 09:13:37 -0700
>
>> On Mon,  5 Oct 2009 17:33:19 +0800
>> Thomas Chou<thomas@wytron.com.tw>  wrote:
>>
>>> diff --git a/drivers/net/ethoc.c b/drivers/net/ethoc.c
>>> index f92747f..0c6c7f4 100644
>>> --- a/drivers/net/ethoc.c
>>> +++ b/drivers/net/ethoc.c
>>> @@ -399,6 +399,10 @@ static int ethoc_rx(struct net_device *dev, int limit)
>>>   		if (ethoc_update_rx_stats(priv,&bd) == 0) {
>>>   			int size = bd.stat>>  16;
>>>   			struct sk_buff *skb = netdev_alloc_skb(dev, size);
>>> +
>>> +			size -= 4; /* strip the CRC */
>>> +			skb_reserve(skb, 2); /* align TCP/IP header */
>>
>> Please use NET_IP_ALIGN rather than hard coding 2 so that the value
>> can be changed on a per-cpu architecture basis if desired.
>
> Indeed.
>
> Thomas please send a patch to fix this up, thanks.
>
>
Submitted. Thanks.

- Thomas

^ permalink raw reply

* [PATCH] net: Add netdev_alloc_skb_ip_align() helper
From: Eric Dumazet @ 2009-10-08  3:11 UTC (permalink / raw)
  To: Thomas Chou
  Cc: netdev, thierry.reding, Nios2 development list, linux-kernel,
	David S. Miller
In-Reply-To: <1254969161-3609-1-git-send-email-thomas@wytron.com.tw>

Thomas Chou a écrit :
> As suggested by Stephen Hemminger.
> 
> Signed-off-by: Thomas Chou <thomas@wytron.com.tw>
> ---
>  drivers/net/ethoc.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/net/ethoc.c b/drivers/net/ethoc.c
> index ecc53d9..4a1ed81 100644
> --- a/drivers/net/ethoc.c
> +++ b/drivers/net/ethoc.c
> @@ -409,7 +409,7 @@ static int ethoc_rx(struct net_device *dev, int limit)
>  			struct sk_buff *skb = netdev_alloc_skb(dev, size);
>  
>  			size -= 4; /* strip the CRC */
> -			skb_reserve(skb, 2); /* align TCP/IP header */
> +			skb_reserve(skb, NET_IP_ALIGN);
>  
>  			if (likely(skb)) {
>  				void *src = phys_to_virt(bd.addr);

Sorry to be dense here, but this code breaks if NET_IP_ALIGN > 4.
Its also suboptimal, you alloc two bytes in excess.


You should do :

size -= 4; /* strip the CRC */
skb = netdev_alloc_skb(dev, size + NET_IP_ALIGN);
if (skb) {
	skb_reserve(skb, NET_IP_ALIGN);
	...
}


Please check other implementations...

David, maybe we should add following helper :

[PATCH] net: Add netdev_alloc_skb_ip_align() helper

Instead of hardcoding NET_IP_ALIGN stuff in various network drivers, we can
add a helper around netdev_alloc_skb()

Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
---
diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index df7b23a..fed788e 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -1489,6 +1489,16 @@ static inline struct sk_buff *netdev_alloc_skb(struct net_device *dev,
 	return __netdev_alloc_skb(dev, length, GFP_ATOMIC);
 }
 
+static inline struct sk_buff *netdev_alloc_skb_ip_align(struct net_device *dev,
+		unsigned int length)
+{
+	struct sk_buff *skb = netdev_alloc_skb(dev, length + NET_IP_ALIGN);
+
+	if (NET_IP_ALIGN && skb)
+		skb_reserve(skb, NET_IP_ALIGN);
+	return skb;
+}
+
 extern struct page *__netdev_alloc_page(struct net_device *dev, gfp_t gfp_mask);
 
 /**

^ permalink raw reply related

* Re: [PATCH] net: Add netdev_alloc_skb_ip_align() helper
From: Thomas Chou @ 2009-10-08  4:36 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: netdev, thierry.reding, Nios2 development list, linux-kernel,
	David S. Miller
In-Reply-To: <4ACD585B.5080106@gmail.com>

On 10/08/2009 11:11 AM, Eric Dumazet wrote:
> Thomas Chou a écrit :
> You should do :
>
> size -= 4; /* strip the CRC */
> skb = netdev_alloc_skb(dev, size + NET_IP_ALIGN);
> if (skb) {
> 	skb_reserve(skb, NET_IP_ALIGN);
> 	...
> }
>
>
> Please check other implementations...
>
> David, maybe we should add following helper :
>
> [PATCH] net: Add netdev_alloc_skb_ip_align() helper

Hi Eric,

Thanks for the suggestion. I will follow your commit and send revised patch.

- Thomas

^ permalink raw reply

* Re: [PATCH net-next-2.6] udp: dynamically size hash tables at boot time
From: David Miller @ 2009-10-08  5:01 UTC (permalink / raw)
  To: eric.dumazet; +Cc: rick.jones2, netdev
In-Reply-To: <4ACC6F87.1050306@gmail.com>

From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Wed, 07 Oct 2009 12:37:59 +0200

> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>

Applied.

^ permalink raw reply

* Re: [PATCH net-next-2.6 0/8] Get rid of net_device_stats from adapter.
From: David Miller @ 2009-10-08  5:01 UTC (permalink / raw)
  To: ajitk, ajitkhaparde; +Cc: netdev
In-Reply-To: <20091007124129.GA29781@serverengines.com>

From: Ajit Khaparde <ajitkhaparde@gmail.com>
Date: Wed, 7 Oct 2009 18:11:40 +0530

> Since net_device already has an instance of net_device stats,
> its not necessary for the drivers to maintain their own copy
> in their private adapter structure.
> This patch series takes care of this for some of the drivers.
> I have compiled them all in my setup. 
> And not all of them have been tested.  :-)

All applied, thanks!

^ permalink raw reply

* Re: [PATCH] net: Make UFO on master device independent of attached devices
From: David Miller @ 2009-10-08  5:01 UTC (permalink / raw)
  To: sri; +Cc: herbert, netdev
In-Reply-To: <1254954265.31575.191.camel@w-sridhar.beaverton.ibm.com>

From: Sridhar Samudrala <sri@us.ibm.com>
Date: Wed, 07 Oct 2009 15:24:25 -0700

> Now that software UFO is supported, UFO can be enabled on master
> devices like bridge, bond even though the attached device doesn't
> support this feature in hardware.
> 
> This allows UFO to be used between KVM host and guest even when a
> physical interface attached to the bridge doesn't support UFO.
> 
> Signed-off-by: Sridhar Samudrala <sri@us.ibm.com>

Applied.

^ permalink raw reply

* Re: [PATCH] bridge: Allow enable/disable UFO on bridge device via ethtool
From: David Miller @ 2009-10-08  5:01 UTC (permalink / raw)
  To: sri; +Cc: netdev
In-Reply-To: <1254955277.31575.208.camel@w-sridhar.beaverton.ibm.com>

From: Sridhar Samudrala <sri@us.ibm.com>
Date: Wed, 07 Oct 2009 15:41:17 -0700

> Allow enable/disable UFO on bridge device via ethtool
> 
> Signed-off-by: Sridhar Samudrala <sri@us.ibm.com>

Applied.

^ permalink raw reply

* Re: [net-next-2.6 PATCH V3] can: add TI CAN (HECC) driver
From: David Miller @ 2009-10-08  5:03 UTC (permalink / raw)
  To: anantgole; +Cc: netdev, socketcan-core, linux-arm-kernel
In-Reply-To: <1254920387-20939-1-git-send-email-anantgole@ti.com>

From: Anant Gole <anantgole@ti.com>
Date: Wed,  7 Oct 2009 18:29:47 +0530

> TI HECC (High End CAN Controller) module is found on many TI devices. It
> has 32 hardware mailboxes with full implementation of CAN protocol 2.0B
> with bus speeds up to 1Mbps. Specifications of the module are available
> on TI web <http://www.ti.com>
> 
> Signed-off-by: Anant Gole <anantgole@ti.com>

Applied, thanks.

^ permalink raw reply

* Re: [BUG] znet.c sleeping function called from invalid context
From: David Miller @ 2009-10-08  5:15 UTC (permalink / raw)
  To: vapier.adi-Re5JQEeQqe8AvxtiuMwx3w
  Cc: netdev-u79uwXL29TY76Z2rM5mHXA, strakh-ufN2psIa012HXe+LvDLADg,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	michael.hennerich-OyLXuOCK7orQT0dZR+AlfA,
	uclinux-dist-devel-ZG0+EudsQA8dtHy/vicBwGD2FQJk+8+b
In-Reply-To: <8bd0f97a0910071144k1d0bf30bv60656181edae8af7-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

From: Mike Frysinger <vapier.adi-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Date: Wed, 7 Oct 2009 14:44:45 -0400

> On Wed, Oct 7, 2009 at 14:47, Alexander Strakh wrote:
>>        KERNEL_VERSION: 2.6.31
>>        DESCRIBE:
>> Driver drivers/net/znet.c might sleep in atomic context, because it calls
>> free_dma under claim_dma_lock:
>>
>> .drivers/net/znet.c:
>>  168 static int znet_request_resources (struct net_device *dev)
>> ...
>>  189        flags = claim_dma_lock();
>>  190        free_dma (znet->tx_dma);
>>  191        release_dma_lock (flags);
>> ...
>>
>> Path to might_sleep macro from znet_request_resources:
>> 1. znet_request_resources calls free_dma at
>> arch/blackfin/kernel/bfin_dma_5xx.c:181
>> 2. free_dma calls arch/blackfin/kernel/bfin_dma_5xx.c:195
> 
> i dont think we need the dmalock mutex.  it's only used to protect
> read/writes to .chan_status, and that should be atomic already.
> -mike

I'm checking in the obvious fix to net-2.6, thanks for the report:

znet: Don't claim DMA lock around free_dma() calls.

It's not necessary and it's illegal too.

Reported-by: Alexander Strakh <strakh-ufN2psIa012HXe+LvDLADg@public.gmane.org>
Signed-off-by: David S. Miller <davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org>
---
 drivers/net/znet.c |    8 --------
 1 files changed, 0 insertions(+), 8 deletions(-)

diff --git a/drivers/net/znet.c b/drivers/net/znet.c
index a0384b6..b423473 100644
--- a/drivers/net/znet.c
+++ b/drivers/net/znet.c
@@ -169,7 +169,6 @@ static void znet_tx_timeout (struct net_device *dev);
 static int znet_request_resources (struct net_device *dev)
 {
 	struct znet_private *znet = netdev_priv(dev);
-	unsigned long flags;
 
 	if (request_irq (dev->irq, &znet_interrupt, 0, "ZNet", dev))
 		goto failed;
@@ -187,13 +186,9 @@ static int znet_request_resources (struct net_device *dev)
  free_sia:
 	release_region (znet->sia_base, znet->sia_size);
  free_tx_dma:
-	flags = claim_dma_lock();
 	free_dma (znet->tx_dma);
-	release_dma_lock (flags);
  free_rx_dma:
-	flags = claim_dma_lock();
 	free_dma (znet->rx_dma);
-	release_dma_lock (flags);
  free_irq:
 	free_irq (dev->irq, dev);
  failed:
@@ -203,14 +198,11 @@ static int znet_request_resources (struct net_device *dev)
 static void znet_release_resources (struct net_device *dev)
 {
 	struct znet_private *znet = netdev_priv(dev);
-	unsigned long flags;
 
 	release_region (znet->sia_base, znet->sia_size);
 	release_region (dev->base_addr, znet->io_size);
-	flags = claim_dma_lock();
 	free_dma (znet->tx_dma);
 	free_dma (znet->rx_dma);
-	release_dma_lock (flags);
 	free_irq (dev->irq, dev);
 }
 
-- 
1.6.4.4

^ permalink raw reply related

* Re: [PATCH] Add sk_mark route lookup support for IPv4 listening sockets, and for IPv4 multicast forwarding
From: David Miller @ 2009-10-08  5:39 UTC (permalink / raw)
  To: zenczykowski; +Cc: atis, netdev, panther, eric.dumazet, brian.haley
In-Reply-To: <55a4f86e0910071703i4735f1aan1ddba81eec7eef19@mail.gmail.com>

From: Maciej Żenczykowski <zenczykowski@gmail.com>
Date: Wed, 7 Oct 2009 17:03:30 -0700

> Should wrapping a packet into a tunnel clear the mark?
> Should perhaps the mark be a parameter of the tunnel (like ttl or qos,
> can be set or can be inherited)?

That's the big question.

It may be that the only logical thing we can do is only apply
the socket mark to the top-level path and that's what happens
now.

Because we really don't have any reasonable way to let the user
control where it applies.  We have no way for it to say "don't
apply the mark to the top-level path, but do apply it to the
multicast tunnel' or something like that.

So, to be honest, I'd say we should skip these skb->mark cases
for now.

^ permalink raw reply

* Re: [PATCH] net: Add netdev_alloc_skb_ip_align() helper
From: David Miller @ 2009-10-08  5:40 UTC (permalink / raw)
  To: eric.dumazet; +Cc: thomas, netdev, thierry.reding, nios2-dev, linux-kernel
In-Reply-To: <4ACD585B.5080106@gmail.com>

From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Thu, 08 Oct 2009 05:11:23 +0200

> David, maybe we should add following helper :
> 
> [PATCH] net: Add netdev_alloc_skb_ip_align() helper
> 
> Instead of hardcoding NET_IP_ALIGN stuff in various network drivers, we can
> add a helper around netdev_alloc_skb()
> 
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>

Looks ok, but I want to look at how often this exact sequence
would match.  If it applies to a lot of cases, I'll add this
but I know of many exceptions in my head already :-)

^ permalink raw reply

* [RFC] multiqueue changes
From: Eric Dumazet @ 2009-10-08  7:18 UTC (permalink / raw)
  To: David S. Miller, Jarek Poplawski, Patrick McHardy; +Cc: Linux Netdev List

Say I have such non multiqueue device :

03:00.0 Ethernet controller: Broadcom Corporation NetXtreme II BCM5708S Gigabit Ethernet (rev 12)

Driver bnx2

This drivers does an alloc_etherdev_mq(sizeof(*bp), TX_MAX_RINGS),
regardless of real capabilities of the NIC.

Then, a bit later it does :

bp->dev->real_num_tx_queues = bp->num_tx_rings;  

(one in my non multiqueue case)

Now I have :

# tc -s -d qdisc show dev eth0
qdisc mq 0: root
 Sent 117693091 bytes 1542188 pkt (dropped 0, overlimits 0 requeues 0)
 backlog 0b 0p requeues 0
# tc -s -d class show dev eth0
class mq :1 root
 Sent 113935509 bytes 1492598 pkt (dropped 0, overlimits 0 requeues 0)
 backlog 0b 0p requeues 0
class mq :2 root
 Sent 0 bytes 0 pkt (dropped 0, overlimits 0 requeues 0)
 backlog 0b 0p requeues 0
class mq :3 root
 Sent 0 bytes 0 pkt (dropped 0, overlimits 0 requeues 0)
 backlog 0b 0p requeues 0
class mq :4 root
 Sent 0 bytes 0 pkt (dropped 0, overlimits 0 requeues 0)
 backlog 0b 0p requeues 0
class mq :5 root
 Sent 0 bytes 0 pkt (dropped 0, overlimits 0 requeues 0)
 backlog 0b 0p requeues 0
class mq :6 root
 Sent 0 bytes 0 pkt (dropped 0, overlimits 0 requeues 0)
 backlog 0b 0p requeues 0
class mq :7 root
 Sent 0 bytes 0 pkt (dropped 0, overlimits 0 requeues 0)
 backlog 0b 0p requeues 0
class mq :8 root
 Sent 0 bytes 0 pkt (dropped 0, overlimits 0 requeues 0)
 backlog 0b 0p requeues 0

While in previous kernels I had :

# tc -s -d qdisc show dev eth0
qdisc pfifo_fast 0: root bands 3 priomap  1 2 2 2 1 2 0 0 1 1 1 1 1 1 1 1
 Sent 26292963818 bytes 347139141 pkts (dropped 0, overlimits 0 requeues 0)
# tc -s -d class show dev eth0


So I lost the default pfifo_fast classification.

Just wondering if it could hurt some people.

Anyway, should we change bnx2/tg3 drivers so that single queue devices
have same default qdisc/class than before ?

eg :

diff --git a/drivers/net/bnx2.c b/drivers/net/bnx2.c
index 08cddb6..7cac205 100644
--- a/drivers/net/bnx2.c
+++ b/drivers/net/bnx2.c
@@ -6152,6 +6152,7 @@ bnx2_setup_int_mode(struct bnx2 *bp, int dis_msi)
 
 	bp->num_tx_rings = rounddown_pow_of_two(bp->irq_nvecs);
 	bp->dev->real_num_tx_queues = bp->num_tx_rings;
+	bp->dev->num_tx_queues = bp->dev->real_num_tx_queues;
 
 	bp->num_rx_rings = bp->irq_nvecs;
 }

^ permalink raw reply related

* [PATCH net-next-2.6] netlink: fix typo in initialization
From: Jiri Pirko @ 2009-10-08  7:23 UTC (permalink / raw)
  To: netdev; +Cc: davem, kaber

Commit 9ef1d4c7c7aca1cd436612b6ca785b726ffb8ed8 introduced a typo in
initialization. This patch fixes this.

Signed-off-by: Jiri Pirko <jpirko@redhat.com>

diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c
index 6a53694..7cf6c0f 100644
--- a/net/sched/cls_api.c
+++ b/net/sched/cls_api.c
@@ -350,7 +350,7 @@ static int tcf_fill_node(struct sk_buff *skb, struct tcf_proto *tp,
 	tcm = NLMSG_DATA(nlh);
 	tcm->tcm_family = AF_UNSPEC;
 	tcm->tcm__pad1 = 0;
-	tcm->tcm__pad1 = 0;
+	tcm->tcm__pad2 = 0;
 	tcm->tcm_ifindex = qdisc_dev(tp->q)->ifindex;
 	tcm->tcm_parent = tp->classid;
 	tcm->tcm_info = TC_H_MAKE(tp->prio, tp->protocol);

^ permalink raw reply related

* Re: [PATCH net-next-2.6] netlink: fix typo in initialization
From: David Miller @ 2009-10-08  7:57 UTC (permalink / raw)
  To: jpirko; +Cc: netdev, kaber
In-Reply-To: <20091008072323.GA3255@psychotron.lab.eng.brq.redhat.com>

From: Jiri Pirko <jpirko@redhat.com>
Date: Thu, 8 Oct 2009 09:23:23 +0200

> Commit 9ef1d4c7c7aca1cd436612b6ca785b726ffb8ed8 introduced a typo in
> initialization. This patch fixes this.
> 
> Signed-off-by: Jiri Pirko <jpirko@redhat.com>

Doesn't this leak some uninitialized bytes to userspace?

Therefore, this fix probably belongs in net-2.6 and -stable instead
of net-next-2.6, right?

> diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c
> index 6a53694..7cf6c0f 100644
> --- a/net/sched/cls_api.c
> +++ b/net/sched/cls_api.c
> @@ -350,7 +350,7 @@ static int tcf_fill_node(struct sk_buff *skb, struct tcf_proto *tp,
>  	tcm = NLMSG_DATA(nlh);
>  	tcm->tcm_family = AF_UNSPEC;
>  	tcm->tcm__pad1 = 0;
> -	tcm->tcm__pad1 = 0;
> +	tcm->tcm__pad2 = 0;
>  	tcm->tcm_ifindex = qdisc_dev(tp->q)->ifindex;
>  	tcm->tcm_parent = tp->classid;
>  	tcm->tcm_info = TC_H_MAKE(tp->prio, tp->protocol);

^ 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