Netdev List
 help / color / mirror / Atom feed
* Re: how to utilize multi tx queue to sent packets
From: Stephen Hemminger @ 2011-03-14 18:14 UTC (permalink / raw)
  To: Jon Zhou; +Cc: netdev@vger.kernel.org
In-Reply-To: <4A6A2125329CFD4D8CC40C9E8ABCAB9F24FB47E9EE@MILEXCH2.ds.jdsu.net>

On Sun, 13 Mar 2011 01:06:11 -0800
Jon Zhou <Jon.Zhou@jdsu.com> wrote:

> 
> 
> > -----Original Message-----
> > From: Stephen Hemminger [mailto:shemminger@vyatta.com]
> > Sent: Saturday, March 12, 2011 1:07 AM
> > To: Jon Zhou
> > Cc: netdev@vger.kernel.org
> > Subject: Re: how to utilize multi tx queue to sent packets
> > 
> > On Thu, 10 Mar 2011 22:52:30 -0800
> > Jon Zhou <Jon.Zhou@jdsu.com> wrote:
> > 
> > > hi
> > >
> > > I am doing some test according to the
> > > website:http://wiki.ipxwarzone.com/index.php5?title=Linux_packet_mmap
> > >
> > > use packet_mmap tx_ring to send packet.
> > >
> > > I modified the sample code "packetmmap.c" to make it send packets
> > have different outer ip.
> > > so that with the help of RSS, I can achieve higher throughput.
> > >
> > > but one thing I saw at the tx side, is that all the packets are sent
> > via the same tx_queue,
> > > which is conflict with what I saw at the rx side.
> > > any idea to make it sent packets via different tx_queues? (spread
> > across the tx_queues)
> > 
> > You need to have multiple threads to get Tx scaling.
> > In you case that also means multiple AF_PACKET sockets and separate
> > rings.
> > 
> > Or just run multiple copies of the same test each with a different IP
> 
> How does kernel know which tx_queue to use?
> Will dev_queue_xmit determine tx_queue(sw queue) to send packets?
> Or just let NIC select a tx_queue(hardware) ?
> 

On most hardware, with multi-queue there is one queue per CPU.
In that case the queue selected corresponds to the CPU.



-- 

^ permalink raw reply

* Re: [GIT PULL nf-next-2.6] IPVS
From: Patrick McHardy @ 2011-03-14 18:17 UTC (permalink / raw)
  To: Simon Horman
  Cc: lvs-devel, netdev, netfilter-devel, netfilter, Hans Schillstrom,
	Julian Anastasov
In-Reply-To: <1300056261-14709-1-git-send-email-horms@verge.net.au>

On 13.03.2011 23:44, Simon Horman wrote:
> Hi Patrick,
> 
> please consider pulling
> git://git.kernel.org/pub/scm/linux/kernel/git/horms/lvs-test-2.6.git master
> to get the following changes.
> 
> Jesper Juhl (1):
>       Fix variable assignment in ip_vs_notrack
> 
> Julian Anastasov (2):
>       ipvs: avoid lookup for fwmark 0
>       ipvs: remove _bh from percpu stats reading
> 
> Shan Wei (1):
>       netfilter:ipvs: use kmemdup
> 
>  include/net/ip_vs.h               |    2 +-
>  net/netfilter/ipvs/ip_vs_ctl.c    |    8 +++++---
>  net/netfilter/ipvs/ip_vs_est.c    |    8 ++++----
>  net/netfilter/ipvs/ip_vs_pe_sip.c |    9 ++++-----
>  net/netfilter/ipvs/ip_vs_sync.c   |    3 +--
>  5 files changed, 15 insertions(+), 15 deletions(-)

The pull contained quite a lot of other changes. I'm undoing the
pull for now, please resubmit the patches you actually want me to
pull. Thanks!

Jesper Juhl (1):
      Fix variable assignment in ip_vs_notrack

Julian Anastasov (8):
      ipvs: avoid lookup for fwmark 0
      ipvs: remove _bh from percpu stats reading
      ipvs: move struct netns_ipvs
      ipvs: reorganize tot_stats
      ipvs: properly zero stats and rates
      ipvs: remove unused seqcount stats
      ipvs: optimize rates reading
      ipvs: rename estimator functions

Patrick McHardy (1):
      Merge branch 'master' of git://git.kernel.org/.../horms/lvs-test-2.6

Shan Wei (1):
      netfilter:ipvs: use kmemdup

Simon Horman (14):
      IPVS: Add ip_vs_route_me_harder()
      IPVS: Add sysctl_snat_reroute()
      IPVS: Add sysctl_nat_icmp_send()
      IPVS: Add {sysctl_sync_threshold,period}()
      IPVS: Add sysctl_sync_ver()
      IPVS: Add sysctl_expire_nodest_conn()
      IPVS: Add expire_quiescent_template()
      IPVS: Conditinally use sysctl_lblc{r}_expiration
      IPVS: ip_vs_todrop() becomes a noop when CONFIG_SYSCTL is undefined
      IPVS: Conditional ip_vs_conntrack_enabled()
      IPVS: Minimise ip_vs_leave when CONFIG_SYSCTL is undefined
      IPVS: Conditionally define and use ip_vs_lblc{r}_table
      IPVS: Add __ip_vs_control_{init,cleanup}_sysctl()
      IPVS: Conditionally include sysctl members of struct netns_ipvs

^ permalink raw reply

* Re: [PATCH] conntrack: fix sysctl memory leak
From: Patrick McHardy @ 2011-03-14 18:21 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: David S. Miller, netdev, netfilter-devel
In-Reply-To: <20110310101422.62006620@nehalam>

On 10.03.2011 19:14, Stephen Hemminger wrote:
> Message in log because sysctl table was not empty at netns exit
>  WARNING: at net/sysctl_net.c:84 sysctl_net_exit+0x2a/0x2c()
> 
> Instrumenting showed that the nf_conntrack_timestamp was the entry
> that was being created but not cleared.

Applied, thanks Stephen.

^ permalink raw reply

* Re: [PATCH 1/4] netfilter: xt_connlimit: fix daddr connlimit in SNAT scenario
From: Patrick McHardy @ 2011-03-14 18:32 UTC (permalink / raw)
  To: Changli Gao; +Cc: Jan Engelhardt, David S. Miller, netfilter-devel, netdev
In-Reply-To: <AANLkTi=QgVBVAPcTmb_La=VJOLq5LmGPqMfw7bbCuByP@mail.gmail.com>

On 14.03.2011 13:42, Changli Gao wrote:
> On Mon, Mar 14, 2011 at 8:26 PM, Jan Engelhardt <jengelh@medozas.de> wrote:
>> On Monday 2011-03-14 07:50, Changli Gao wrote:
>>
>>> We use the reply tuples when limiting the connections by the destination
>>> addresses, however, in SNAT scenario, the final reply tuples won't be
>>> ready until SNAT is done in POSTROUING or INPUT chain
>>
>> If I am not mistaken: if you do daddr counting, SNAT is irrelevant.
>> Consider ruleset
>>  -t nat -A PREROUTING -p tcp --dport 80 -j DNAT --to 1.2.3.4:80
>>  -t nat -A PREROUTING -p tcp --dport 443 -j DNAT --to 1.2.3.5:443
>>
>> The tuple will first be (as per conntrack -L):
>>  src=home dst=router src=router dst=home
>> After DNAT:
>>  src=home dst=router src=1.2.3.4 dst=home
>>
>> Thus looking at the src of the reply tuple seems correct — at least this
>> is what was wanted, counting per stashed servers (=1 customer), not per
>> globally visible address.
>>
> 
> Yes, you are correct only when there is no SNAT rule. If there is an SNAT rule:
> 
> -t nat -A POSTROUTING -p tcp --dport 80 -j SNAT --to-source 192.168.0.1
> 
> the final tuples will be:
> src = home dst = router src=1.2.3.4 dst=192.168.0.1
> 
> However, the tuple saved by connlimit is src=1.2.3.4 dst=home, so this
> conn will be removed later as there isn't any conntrack, which has
> this tuple in any direction.
> 
> You can't prevent a user from doing such a configuration, although you
> might think it is stupid to do that.
> 
> Thanks for your review.

Jan, please let me know whether you want me to apply these patches.
Thanks.

^ permalink raw reply

* Re: [GIT/PATCH v5] xen network backend driver
From: Ben Hutchings @ 2011-03-14 18:40 UTC (permalink / raw)
  To: Ian Campbell
  Cc: netdev@vger.kernel.org, xen-devel, Jeremy Fitzhardinge,
	Herbert Xu, Konrad Rzeszutek Wilk, Francois Romieu,
	Stephen Hemminger, Michał Mirosław
In-Reply-To: <1300112879.17339.2162.camel@zakaz.uk.xensource.com>

On Mon, 2011-03-14 at 14:27 +0000, Ian Campbell wrote:
> The following patch is the fifth iteration of the Xen network backend
> driver for upstream Linux.
> 
> This driver ("netback") is the host side counterpart to the frontend
> driver in drivers/net/xen-netfront.c. The PV protocol is also
> implemented by frontend drivers in other OSes too, such as the BSDs and
> even Windows.
> 
> Changes since the fourth posting, due to review from Stephen Hemminger:
>       * Treat dev->features as u32 (it effectively is already and
>         becomes literally so in net-next -- there's no harm in netback
>         switching earlier)
>       * Use stats from struct netdevice instead of a local copy.
[...]

All looks good (or good enough) to me.  I would add Reviewed-by, but
this isn't a single patch.

I know you have asked for this to be pulled, in order to retain its
history, but I'm not sure that David Miller will do this unless the
driver is at least buildable at each stage in the history.  (However,
given that the driver wouldn't be enabled before it's added, I'm not
sure it matters in this case.)

Ben.

-- 
Ben Hutchings, Senior Software Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.


^ permalink raw reply

* Re: [PATCH 4/7] tcp_cubic: fix clock dependency
From: Eric Dumazet @ 2011-03-14 18:51 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: David S. Miller, netdev
In-Reply-To: <20110314175439.564307922@vyatta.com>

Le lundi 14 mars 2011 à 10:52 -0700, Stephen Hemminger a écrit :
> pièce jointe document texte brut (tcp-cubic-minrtt.patch)
> The hystart code was written with assumption that HZ=1000.
> Replace the use of jiffies with bictcp_clock as a millisecond
> real time clock. 
> 
> Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>
> Reported-by: Lucas Nussbaum <lucas.nussbaum@loria.fr>
> 
> --- a/net/ipv4/tcp_cubic.c	2011-03-14 08:19:18.000000000 -0700
> +++ b/net/ipv4/tcp_cubic.c	2011-03-14 08:22:42.486690594 -0700
> @@ -88,7 +88,7 @@ struct bictcp {
>  	u32	last_time;	/* time when updated last_cwnd */
>  	u32	bic_origin_point;/* origin point of bic function */
>  	u32	bic_K;		/* time to origin point from the beginning of the current epoch */
> -	u32	delay_min;	/* min delay */
> +	u32	delay_min;	/* min delay (msec << 3) */
>  	u32	epoch_start;	/* beginning of an epoch */
>  	u32	ack_cnt;	/* number of acks */
>  	u32	tcp_cwnd;	/* estimated tcp cwnd */
> @@ -98,7 +98,7 @@ struct bictcp {
>  	u8	found;		/* the exit point is found? */
>  	u32	round_start;	/* beginning of each round */
>  	u32	end_seq;	/* end_seq of the round */
> -	u32	last_jiffies;	/* last time when the ACK spacing is close */
> +	u32	last_ack;	/* last time when the ACK spacing is close */
>  	u32	curr_rtt;	/* the minimum rtt of current round */
>  };
>  
> @@ -119,12 +119,21 @@ static inline void bictcp_reset(struct b
>  	ca->found = 0;
>  }
>  
> +static inline u32 bictcp_clock(void)
> +{
> +#if HZ < 1000
> +	return ktime_to_ms(ktime_get_real());

Small point : This can be changed if date/time is changed

Maybe use monotonic time (aka ktime_get_ts()) ?




^ permalink raw reply

* Re: [PATCH 1/4] netfilter: xt_connlimit: fix daddr connlimit in SNAT scenario
From: Jan Engelhardt @ 2011-03-14 19:00 UTC (permalink / raw)
  To: Changli Gao; +Cc: Patrick McHardy, David S. Miller, netfilter-devel, netdev
In-Reply-To: <AANLkTi=QgVBVAPcTmb_La=VJOLq5LmGPqMfw7bbCuByP@mail.gmail.com>

On Monday 2011-03-14 13:42, Changli Gao wrote:

>On Mon, Mar 14, 2011 at 8:26 PM, Jan Engelhardt <jengelh@medozas.de> wrote:
>> On Monday 2011-03-14 07:50, Changli Gao wrote:
>>
>>>We use the reply tuples when limiting the connections by the destination
>>>addresses, however, in SNAT scenario, the final reply tuples won't be
>>>ready until SNAT is done in POSTROUING or INPUT chain
>>
>> If I am not mistaken: if you do daddr counting, SNAT is irrelevant.
>> Consider ruleset
>>  -t nat -A PREROUTING -p tcp --dport 80 -j DNAT --to 1.2.3.4:80
>>  -t nat -A PREROUTING -p tcp --dport 443 -j DNAT --to 1.2.3.5:443
>>
>> The tuple will first be (as per conntrack -L):
>>  src=home dst=router src=router dst=home
>> After DNAT:
>>  src=home dst=router src=1.2.3.4 dst=home
>>
>> Thus looking at the src of the reply tuple seems correct — at least this
>> is what was wanted, counting per stashed servers (=1 customer), not per
>> globally visible address.
>>
>
>Yes, you are correct only when there is no SNAT rule. If there is an 
>SNAT rule:
>
>-t nat -A POSTROUTING -p tcp --dport 80 -j SNAT --to-source 192.168.0.1
>
>the final tuples will be:
>src = home dst = router src=1.2.3.4 dst=192.168.0.1
>
>However, the tuple saved by connlimit is src=1.2.3.4 dst=home, so this
>conn will be removed later as there isn't any conntrack, which has
>this tuple in any direction.

But I don't yet see how your patch #1 can help. At the time 
conn->tuple = *tuple is done, *tuple still contains the non-SNATed 
tuple, and it is never updated again.


(Patrick: patches 2-4 are ok)

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

^ permalink raw reply

* Re: [PATCH 2/2] dev : fix mtu check when TSO is enabled
From: Daniel Lezcano @ 2011-03-14 19:00 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: davem, kaber, nightnord, netdev
In-Reply-To: <1300122105.3423.131.camel@edumazet-laptop>

On 03/14/2011 06:01 PM, Eric Dumazet wrote:
> Le lundi 14 mars 2011 à 17:08 +0100, Daniel Lezcano a écrit :
>> In case the device where is coming from the packet has TSO enabled,
>> we should not check the mtu size value as this one could be bigger
>> than the expected value.
>>
>> This is the case for the macvlan driver when the lower device has
>> TSO enabled. The macvlan inherit this feature and forward the packets
>> without fragmenting them. Then the packets go through dev_forward_skb
>> and are dropped. This patch fix this by checking TSO is not enabled
>> when we want to check the mtu size.
>>
>> Signed-off-by: Daniel Lezcano<daniel.lezcano@free.fr>
>> Cc: Patrick McHardy<kaber@trash.net>
>> Cc: Andrian Nord<nightnord@gmail.com>
>> ---
>>   net/core/dev.c |    6 ++++--
>>   1 files changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/net/core/dev.c b/net/core/dev.c
>> index 6561021..010a0a9 100644
>> --- a/net/core/dev.c
>> +++ b/net/core/dev.c
>> @@ -1526,8 +1526,10 @@ int dev_forward_skb(struct net_device *dev, struct sk_buff *skb)
>>   	skb_orphan(skb);
>>   	nf_reset(skb);
>>
>> -	if (unlikely(!(dev->flags&  IFF_UP) ||
>> -		     (skb->len>  (dev->mtu + dev->hard_header_len + VLAN_HLEN)))) {
>> +	if (unlikely(!(skb->dev->features&  NETIF_F_TSO)&&
>
> Sorry, this needs a comment at least, or even better a helper function
> on its own.
>
> We test both skb->dev&  dev, pointing to different devices, this is
> really error prone.
>
> Are we sure all callers set skb->dev before calling  dev_forward_skb() ?

Thanks Eric for the comments, I will resend a new version.

   -- Daniel


^ permalink raw reply

* Re: fcoe: correct checking for bonding
From: Robert Love @ 2011-03-14 19:04 UTC (permalink / raw)
  To: David Miller
  Cc: jpirko@redhat.com, James.Bottomley@suse.de,
	linux-scsi@vger.kernel.org, devel@open-fcoe.org,
	netdev@vger.kernel.org, fubar@us.ibm.com, joe.eykholt@gmail.com
In-Reply-To: <20110312.105944.193725929.davem@davemloft.net>

On Sat, 2011-03-12 at 10:59 -0800, David Miller wrote:
> From: Jiri Pirko <jpirko@redhat.com>
> Date: Sat, 12 Mar 2011 13:01:10 +0100
> 
> > Thu, Mar 03, 2011 at 02:09:18AM CET, robert.w.love@intel.com wrote:
> >>On Wed, 2011-03-02 at 01:55 -0800, Jiri Pirko wrote:
> >>> Or perhaps this should be applied to net-next?
> >>> 
> >>I think this should go through scsi-misc as all the other
> >>libfc/libfcoe/fcoe patches do.
> >>
> >>> Wed, Mar 02, 2011 at 07:05:35AM CET, jpirko@redhat.com wrote:
> >>> >Check for bonding master and refuse to use that.
> >>> >
> >>> >Signed-off-by: Jiri Pirko <jpirko@redhat.com>
> >>> >---
> >>> > drivers/scsi/fcoe/fcoe.c |    4 +---
> >>> > 1 files changed, 1 insertions(+), 3 deletions(-)
> >>> >
> >>> >diff --git a/drivers/scsi/fcoe/fcoe.c b/drivers/scsi/fcoe/fcoe.c
> >>> >index 9f9600b..3becc6a 100644
> >>> >--- a/drivers/scsi/fcoe/fcoe.c
> >>> >+++ b/drivers/scsi/fcoe/fcoe.c
> >>> >@@ -285,9 +285,7 @@ static int fcoe_interface_setup(struct fcoe_interface *fcoe,
> >>> > 	}
> >>> > 
> >>> > 	/* Do not support for bonding device */
> >>> >-	if ((netdev->priv_flags & IFF_MASTER_ALB) ||
> >>> >-	    (netdev->priv_flags & IFF_SLAVE_INACTIVE) ||
> >>> >-	    (netdev->priv_flags & IFF_MASTER_8023AD)) {
> >>> >+	if (netdev->priv_flags & IFF_BONDING && netdev->flags & IFF_MASTER) {
> >>> > 		FCOE_NETDEV_DBG(netdev, "Bonded interfaces not supported\n");
> >>> > 		return -EOPNOTSUPP;
> >>> > 	}
> >>> >-- 
> >>> >1.7.3.4
> >>> >
> >>
> >>James, feel free to pick up this patch. I don't have anything in my fcoe
> >>tree right now that it would conflict with. I'll also put it in my tree
> >>and resend if you don't put it into scsi-misc directly.
> > 
> > What's the status of this? Maybe this should rather go thru net-next
> 
> Sure, I can take this.  I'll look at it later.

Hi Dave,

   I'd rather have this patch go through scsi-misc. Most, if not all,
libfc, libfcoe and fcoe patches have taken this path. The way it has
been working is that I have been collecting fcoe patches and re-posting
them to scsi-misc after I have reviewed them and done some basic
testing.

   Taking a patch like this through net{-next} could cause a merge
problem at Linus' level if a later patch makes it though the normal
process and conflicts. This is what I want to avoid.

   This patch, although appreciated, isn't critical. I have collected it
into my tree and will re-post it to scsi-misc. I see no reason to treat
this patch differently from other patches.

   Ultimately I just want things to go smoothly, so I'll leave it up to
James and you to figure out what to do.

Thanks, //Rob


^ permalink raw reply

* [net-next-2.6 PATCH 1/3] net: dcbnl: Update copyright dates
From: John Fastabend @ 2011-03-14 19:01 UTC (permalink / raw)
  To: davem; +Cc: mark.d.rustad, netdev, shmulikr

From: Mark Rustad <mark.d.rustad@intel.com>

Signed-off-by: Mark Rustad <mark.d.rustad@intel.com>
Signed-off-by: John Fastabend <john.r.fastabend@intel.com>
---

 include/linux/dcbnl.h |    2 +-
 net/dcb/dcbnl.c       |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/linux/dcbnl.h b/include/linux/dcbnl.h
index a3680a1..97c90b9 100644
--- a/include/linux/dcbnl.h
+++ b/include/linux/dcbnl.h
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2008, Intel Corporation.
+ * Copyright (c) 2008-2011, Intel Corporation.
  *
  * This program is free software; you can redistribute it and/or modify it
  * under the terms and conditions of the GNU General Public License,
diff --git a/net/dcb/dcbnl.c b/net/dcb/dcbnl.c
index 118392f..3609eac 100644
--- a/net/dcb/dcbnl.c
+++ b/net/dcb/dcbnl.c
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2008, Intel Corporation.
+ * Copyright (c) 2008-2011, Intel Corporation.
  *
  * This program is free software; you can redistribute it and/or modify it
  * under the terms and conditions of the GNU General Public License,


^ permalink raw reply related

* [net-next-2.6 PATCH 2/3] net: dcbnl: Fix misspellings
From: John Fastabend @ 2011-03-14 19:01 UTC (permalink / raw)
  To: davem; +Cc: mark.d.rustad, netdev, shmulikr
In-Reply-To: <20110314190102.12070.21182.stgit@jf-dev1-dcblab>

From: Mark Rustad <mark.d.rustad@intel.com>

Fix a few spelling errors in dcbnl.h.

Signed-off-by: Mark Rustad <mark.d.rustad@intel.com>
Signed-off-by: John Fastabend <john.r.fastabend@intel.com>
---

 include/linux/dcbnl.h |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/include/linux/dcbnl.h b/include/linux/dcbnl.h
index 97c90b9..eedf79b 100644
--- a/include/linux/dcbnl.h
+++ b/include/linux/dcbnl.h
@@ -26,13 +26,13 @@
 #define IEEE_8021QAZ_MAX_TCS	8
 
 #define IEEE_8021QAZ_TSA_STRICT		0
-#define IEEE_8021QAZ_TSA_CB_SHABER	1
+#define IEEE_8021QAZ_TSA_CB_SHAPER	1
 #define IEEE_8021QAZ_TSA_ETS		2
 #define IEEE_8021QAZ_TSA_VENDOR		255
 
 /* This structure contains the IEEE 802.1Qaz ETS managed object
  *
- * @willing: willing bit in ETS configuratin TLV
+ * @willing: willing bit in ETS configuration TLV
  * @ets_cap: indicates supported capacity of ets feature
  * @cbs: credit based shaper ets algorithm supported
  * @tc_tx_bw: tc tx bandwidth indexed by traffic class
@@ -92,7 +92,7 @@ struct ieee_pfc {
 #define CEE_DCBX_MAX_PRIO	8
 
 /**
- * struct cee_pg - CEE Prioity-Group managed object
+ * struct cee_pg - CEE Priority-Group managed object
  *
  * @willing: willing bit in the PG tlv
  * @error: error bit in the PG tlv


^ permalink raw reply related

* Re: [GIT/PATCH v5] xen network backend driver
From: Eric Dumazet @ 2011-03-14 18:57 UTC (permalink / raw)
  To: Ian Campbell
  Cc: netdev@vger.kernel.org, xen-devel, Jeremy Fitzhardinge,
	Herbert Xu, Konrad Rzeszutek Wilk, Francois Romieu, Ben Hutchings,
	Stephen Hemminger, Michał Mirosław
In-Reply-To: <1300112879.17339.2162.camel@zakaz.uk.xensource.com>

Le lundi 14 mars 2011 à 14:27 +0000, Ian Campbell a écrit :

...

> +struct xenvif {
> +	/* Unique identifier for this interface. */
> +	domid_t          domid;
> +	unsigned int     handle;
> +
> +	/* Reference to netback processing backend. */
> +	struct xen_netbk *netbk;
> +
> +	u8               fe_dev_addr[6];
> +
> +	/* Physical parameters of the comms window. */
> +	grant_handle_t   tx_shmem_handle;
> +	grant_ref_t      tx_shmem_ref;
> +	grant_handle_t   rx_shmem_handle;
> +	grant_ref_t      rx_shmem_ref;
> +	unsigned int     irq;
> +
> +	/* List of frontends to notify after a batch of frames sent. */
> +	struct list_head notify_list;
> +
> +	/* The shared rings and indexes. */
> +	struct xen_netif_tx_back_ring tx;
> +	struct xen_netif_rx_back_ring rx;
> +	struct vm_struct *tx_comms_area;
> +	struct vm_struct *rx_comms_area;
> +
> +	/* Flags that must not be set in dev->features */
> +	u32 features_disabled;
> +
> +	/* Frontend feature information. */
> +	u8 can_sg:1;
> +	u8 gso:1;
> +	u8 gso_prefix:1;
> +	u8 csum:1;
> +
> +	/* Internal feature information. */
> +	u8 can_queue:1;	    /* can queue packets for receiver? */
> +
> +	/*
> +	 * Allow xenvif_start_xmit() to peek ahead in the rx request
> +	 * ring.  This is a prediction of what rx_req_cons will be
> +	 * once all queued skbs are put on the ring.
> +	 */
> +	RING_IDX rx_req_cons_peek;
> +
> +	/* Transmit shaping: allow 'credit_bytes' every 'credit_usec'. */
> +	unsigned long   credit_bytes;
> +	unsigned long   credit_usec;
> +	unsigned long   remaining_credit;
> +	struct timer_list credit_timeout;
> +
> +	/* Statistics */
> +	unsigned long rx_gso_checksum_fixup;

this is an "unsigned long" field

> +
> +	/* Miscellaneous private stuff. */
> +	struct list_head schedule_list;
> +	atomic_t         refcnt;
> +	struct net_device *dev;
> +
> +	wait_queue_head_t waiting_to_free;
> +};
> +


> +static void xenvif_get_ethtool_stats(struct net_device *dev,
> +				     struct ethtool_stats *stats, u64 * data)
> +{
> +	void *vif = netdev_priv(dev);
> +	int i;
> +
> +	for (i = 0; i < ARRAY_SIZE(xenvif_stats); i++)
> +		data[i] = *(int *)(vif + xenvif_stats[i].offset);

so use : data[i] = *(unsigned long *)(vif + xenvif_stats[i].offset);

> +}
> +




^ permalink raw reply

* [net-next-2.6 PATCH 3/3] net: dcbnl: Add IEEE app selector value definitions
From: John Fastabend @ 2011-03-14 19:01 UTC (permalink / raw)
  To: davem; +Cc: mark.d.rustad, netdev, shmulikr
In-Reply-To: <20110314190102.12070.21182.stgit@jf-dev1-dcblab>

From: Mark Rustad <mark.d.rustad@intel.com>

This adds defines for the app selector values currently
defined in the IEEE 802.1Qaz specification.

Signed-off-by: Mark Rustad <mark.d.rustad@intel.com>
Signed-off-by: John Fastabend <john.r.fastabend@intel.com>
---

 include/linux/dcbnl.h |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/include/linux/dcbnl.h b/include/linux/dcbnl.h
index eedf79b..c522800 100644
--- a/include/linux/dcbnl.h
+++ b/include/linux/dcbnl.h
@@ -125,6 +125,11 @@ struct cee_pfc {
 	__u8    tcs_supported;
 };
 
+/* IEEE 802.1Qaz std supported values */
+#define IEEE_8021QAZ_APP_SEL_ETHERTYPE	1
+#define IEEE_8021QAZ_APP_SEL_STREAM	2
+#define IEEE_8021QAZ_APP_SEL_DGRAM	3
+#define IEEE_8021QAZ_APP_SEL_ANY	4
 
 /* This structure contains the IEEE 802.1Qaz APP managed object. This
  * object is also used for the CEE std as well. There is no difference


^ permalink raw reply related

* Re: [PATCH] tcp: avoid cwnd moderation in undo
From: Yuchung Cheng @ 2011-03-14 19:10 UTC (permalink / raw)
  To: Carsten Wolff; +Cc: David Miller, Ilpo Jarvinen, Nandita Dukkipati, netdev
In-Reply-To: <201103141106.19679.carsten@wolffcarsten.de>

On Mon, Mar 14, 2011 at 3:06 AM, Carsten Wolff <carsten@wolffcarsten.de> wrote:
> The moderation is in place to avoid gigantic segment bursts, which could cause
> unnecessary pressure on buffers. In my eyes it's already suboptimal that the
> moderation is weakened in the presence of (detected) reordering, let alone
> removing it completely.

In the presence of reordering, cwnd is already moderated in Disorder
state before
 entering the (false) recovery.


>
> More importantly, the prior ssthresh is restored and not affected by
> moderation. This means, if moderation reduces cwnd to a small value, then cwnd
> < ssthresh and TCP will quickly slow-start back to the previous state, without
> sending a big burst of segments.
>
> Also, you intended to remove cwnd moderation only from an undo during
> recovery, but I think your patch also removes cwnd moderation when the undo is
> caused by D-SACK, i.e. most likely after recovery already ended.

Thanks. I will update my patch description. But the same principle
applies that cwnd
should not be moderated on false events. Whether it should be moderated on
reordering or other events is another (complex) design issue. But this
patch does not
touch that.

Yuchung

^ permalink raw reply

* [PATCH][v2] dev : fix mtu check when TSO is enabled
From: Daniel Lezcano @ 2011-03-14 19:10 UTC (permalink / raw)
  To: davem; +Cc: eric.dumazet, kaber, nightnord, netdev

In case the device where is coming from the packet has TSO enabled,
we should not check the mtu size value as this one could be bigger
than the expected value.

This is the case for the macvlan driver when the lower device has
TSO enabled. The macvlan inherit this feature and forward the packets
without fragmenting them. Then the packets go through dev_forward_skb
and are dropped. This patch fix this by checking TSO is not enabled
when we want to check the mtu size.

Signed-off-by: Daniel Lezcano <daniel.lezcano@free.fr>
Cc: Eric Dumazet <eric.dumazet@gmail.com>
Cc: Patrick McHardy <kaber@trash.net>
Cc: Andrian Nord <nightnord@gmail.com>
---
 net/core/dev.c |   24 ++++++++++++++++++++++--
 1 files changed, 22 insertions(+), 2 deletions(-)

diff --git a/net/core/dev.c b/net/core/dev.c
index 6561021..f1607a0 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -1503,6 +1503,27 @@ static inline void net_timestamp_check(struct sk_buff *skb)
 		__net_timestamp(skb);
 }
 
+static inline bool is_skb_forwardable(struct net_device *dev,
+				      struct sk_buff *skb)
+{
+	unsigned int len;
+
+	if (!dev->flags & IFF_UP)
+		return false;
+
+	/* we should not check the mtu size if TSO is enabled otherwise
+	 * we may have a packet with a length bigger than the expected
+	 * one as it was not segmented before */
+	if (skb->dev && skb->dev->features & NETIF_F_TSO)
+		return true;
+
+	len = dev->mtu + dev->hard_header_len + VLAN_HLEN;
+	if (skb->len > len)
+		return false;
+
+	return true;
+}
+
 /**
  * dev_forward_skb - loopback an skb to another netif
  *
@@ -1526,8 +1547,7 @@ int dev_forward_skb(struct net_device *dev, struct sk_buff *skb)
 	skb_orphan(skb);
 	nf_reset(skb);
 
-	if (unlikely(!(dev->flags & IFF_UP) ||
-		     (skb->len > (dev->mtu + dev->hard_header_len + VLAN_HLEN)))) {
+	if (unlikely(!is_skb_forwardable(dev, skb))) {
 		atomic_long_inc(&dev->rx_dropped);
 		kfree_skb(skb);
 		return NET_RX_DROP;
-- 
1.7.1


^ permalink raw reply related

* Re: fcoe: correct checking for bonding
From: Jiri Pirko @ 2011-03-14 19:22 UTC (permalink / raw)
  To: Robert Love
  Cc: David Miller, James.Bottomley@suse.de, linux-scsi@vger.kernel.org,
	devel@open-fcoe.org, netdev@vger.kernel.org, fubar@us.ibm.com,
	joe.eykholt@gmail.com
In-Reply-To: <1300129447.19083.258.camel@fritz>

Mon, Mar 14, 2011 at 08:04:07PM CET, robert.w.love@intel.com wrote:
>On Sat, 2011-03-12 at 10:59 -0800, David Miller wrote:
>> From: Jiri Pirko <jpirko@redhat.com>
>> Date: Sat, 12 Mar 2011 13:01:10 +0100
>> 
>> > Thu, Mar 03, 2011 at 02:09:18AM CET, robert.w.love@intel.com wrote:
>> >>On Wed, 2011-03-02 at 01:55 -0800, Jiri Pirko wrote:
>> >>> Or perhaps this should be applied to net-next?
>> >>> 
>> >>I think this should go through scsi-misc as all the other
>> >>libfc/libfcoe/fcoe patches do.
>> >>
>> >>> Wed, Mar 02, 2011 at 07:05:35AM CET, jpirko@redhat.com wrote:
>> >>> >Check for bonding master and refuse to use that.
>> >>> >
>> >>> >Signed-off-by: Jiri Pirko <jpirko@redhat.com>
>> >>> >---
>> >>> > drivers/scsi/fcoe/fcoe.c |    4 +---
>> >>> > 1 files changed, 1 insertions(+), 3 deletions(-)
>> >>> >
>> >>> >diff --git a/drivers/scsi/fcoe/fcoe.c b/drivers/scsi/fcoe/fcoe.c
>> >>> >index 9f9600b..3becc6a 100644
>> >>> >--- a/drivers/scsi/fcoe/fcoe.c
>> >>> >+++ b/drivers/scsi/fcoe/fcoe.c
>> >>> >@@ -285,9 +285,7 @@ static int fcoe_interface_setup(struct fcoe_interface *fcoe,
>> >>> > 	}
>> >>> > 
>> >>> > 	/* Do not support for bonding device */
>> >>> >-	if ((netdev->priv_flags & IFF_MASTER_ALB) ||
>> >>> >-	    (netdev->priv_flags & IFF_SLAVE_INACTIVE) ||
>> >>> >-	    (netdev->priv_flags & IFF_MASTER_8023AD)) {
>> >>> >+	if (netdev->priv_flags & IFF_BONDING && netdev->flags & IFF_MASTER) {
>> >>> > 		FCOE_NETDEV_DBG(netdev, "Bonded interfaces not supported\n");
>> >>> > 		return -EOPNOTSUPP;
>> >>> > 	}
>> >>> >-- 
>> >>> >1.7.3.4
>> >>> >
>> >>
>> >>James, feel free to pick up this patch. I don't have anything in my fcoe
>> >>tree right now that it would conflict with. I'll also put it in my tree
>> >>and resend if you don't put it into scsi-misc directly.
>> > 
>> > What's the status of this? Maybe this should rather go thru net-next
>> 
>> Sure, I can take this.  I'll look at it later.
>
>Hi Dave,
>
>   I'd rather have this patch go through scsi-misc. Most, if not all,
>libfc, libfcoe and fcoe patches have taken this path. The way it has
>been working is that I have been collecting fcoe patches and re-posting
>them to scsi-misc after I have reviewed them and done some basic
>testing.
>
>   Taking a patch like this through net{-next} could cause a merge
>problem at Linus' level if a later patch makes it though the normal
>process and conflicts. This is what I want to avoid.
>
>   This patch, although appreciated, isn't critical. I have collected it
>into my tree and will re-post it to scsi-misc. I see no reason to treat
>this patch differently from other patches.

Well I have another set of patches dependent on this one :(
>
>   Ultimately I just want things to go smoothly, so I'll leave it up to
>James and you to figure out what to do.
>
>Thanks, //Rob
>

^ permalink raw reply

* Re: Occasional link flap on Intel 82599 on boot in XAUI mode
From: Brent Cook @ 2011-03-14 19:21 UTC (permalink / raw)
  To: Skidmore, Donald C; +Cc: netdev@vger.kernel.org
In-Reply-To: <29F4ED941D916B48B88B4D2A4F3D1B9C01D30140EF@orsmsx509.amr.corp.intel.com>

On Friday 25 February 2011 12:11:11 Skidmore, Donald C wrote:
> >-----Original Message-----
> >From: netdev-owner@vger.kernel.org [mailto:netdev-owner@vger.kernel.org]
> >On Behalf Of Brent Cook
> >Sent: Friday, February 25, 2011 8:12 AM
> >To: netdev@vger.kernel.org
> >Subject: Occasional link flap on Intel 82599 on boot in XAUI mode
> >
> >We have a custom system with dual 82599's in XAUI mode. One has its pair
> >of ports connected to a 10G switch, the other has its pair of ports
> >connected to an FPGA.
> >
> >Occasionally, on any of the interfaces, we will see the links flapping
> >up and down when the system initially boots. This happens maybe one in
> >20 boots.
> >
> >Feb 23 14:58:10 mfg kernel: [  594.254977] ixgbe: eth1 NIC Link is Down
> >Feb 23 14:58:12 mfg kernel: [  596.230039] ixgbe: eth1 NIC Link is Up 10
> >Gbps, Flow Control: RX/TX
> >Feb 23 14:58:12 mfg kernel: [  596.256537] ixgbe: eth1 NIC Link is Down
> >Feb 23 14:58:16 mfg kernel: [  600.228096] ixgbe: eth1 NIC Link is Up 10
> >Gbps, Flow Control: RX/TX
> >Feb 23 14:58:16 mfg kernel: [  600.240135] ixgbe: eth1 NIC Link is Down
> >Feb 23 14:58:18 mfg kernel: [  602.227047] ixgbe: eth1 NIC Link is Up 10
> >Gbps, Flow Control: RX/TX
> >
> >Simply forcing a down/up on the interface seems to correct the problem:
> >
> >ip link set eth1 down
> >ip link set eth1 up
> >
> >Is anyone else using XAUI mode and has seen this? Here is the kernel
> >information that we are using currently:
> >
> 
> By any chance have you tried it with a newer driver?  Latest Source Forge is 3.2.9.
> 
> Also do you only see this link flap on boot, can you recreate the flap by unload and loading the ixgbe module?
> 

We updated to the latest source forge driver and had the same behavior. Reloading the driver does not reproduce the issue.

It seems to be related to the remote XAUI peer coming out of reset after the 82599. The FPGAs on one pair of links are loaded in software at boot time, and the switch reset is also delayed slightly compared to the 82599.  We fixed the issue by doing 'ethtool -r' once for each link at boot time.

Thanks
 - Brent

^ permalink raw reply

* Re: [GIT/PATCH v5] xen network backend driver
From: Ian Campbell @ 2011-03-14 19:30 UTC (permalink / raw)
  To: Ben Hutchings
  Cc: netdev@vger.kernel.org, xen-devel, Jeremy Fitzhardinge,
	Herbert Xu, Konrad Rzeszutek Wilk, Francois Romieu,
	Stephen Hemminger, Michał Mirosław
In-Reply-To: <1300128035.2584.6.camel@bwh-desktop>

On Mon, 2011-03-14 at 18:40 +0000, Ben Hutchings wrote:
> On Mon, 2011-03-14 at 14:27 +0000, Ian Campbell wrote:
> > The following patch is the fifth iteration of the Xen network backend
> > driver for upstream Linux.
> > 
> > This driver ("netback") is the host side counterpart to the frontend
> > driver in drivers/net/xen-netfront.c. The PV protocol is also
> > implemented by frontend drivers in other OSes too, such as the BSDs and
> > even Windows.
> > 
> > Changes since the fourth posting, due to review from Stephen Hemminger:
> >       * Treat dev->features as u32 (it effectively is already and
> >         becomes literally so in net-next -- there's no harm in netback
> >         switching earlier)
> >       * Use stats from struct netdevice instead of a local copy.
> [...]
> 
> All looks good (or good enough) to me.

Thanks.

> I would add Reviewed-by, but this isn't a single patch.

Sure.

> I know you have asked for this to be pulled, in order to retain its
> history, but I'm not sure that David Miller will do this unless the
> driver is at least buildable at each stage in the history.  (However,
> given that the driver wouldn't be enabled before it's added, I'm not
> sure it matters in this case.)

I think it should be, at least from the second commit onwards (modulo
any mistakes made in the past).

To be sure perhaps I should rebase to add a depends BROKEN to (or
otherwise nobble) the Kconfig entry at the beginning of time and only
remove it at the end of the series?

I'd definitely like to keep the cleanups I made while upstreaming
separated from the version imported from xen.git though, but I could
collapse things before that point if that would be preferred. The
upstream/dom0/backend/netback-base branch is the imported version,
everything after that until upstream/dom0/backend/netback is what I
really want to keep as separate commits, the stuff before is just (very)
nice to have.

Ian.


^ permalink raw reply

* Re: [GIT/PATCH v5] xen network backend driver
From: Ian Campbell @ 2011-03-14 19:33 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: netdev@vger.kernel.org, xen-devel, Jeremy Fitzhardinge,
	Herbert Xu, Konrad Rzeszutek Wilk, Francois Romieu, Ben Hutchings,
	Stephen Hemminger, Michał Mirosław
In-Reply-To: <1300129032.3423.170.camel@edumazet-laptop>

On Mon, 2011-03-14 at 18:57 +0000, Eric Dumazet wrote:
> Le lundi 14 mars 2011 à 14:27 +0000, Ian Campbell a écrit :

> > +	/* Statistics */
> > +	unsigned long rx_gso_checksum_fixup;
> 
> this is an "unsigned long" field

> > +static void xenvif_get_ethtool_stats(struct net_device *dev,
> > +				     struct ethtool_stats *stats, u64 * data)
> > +{
> > +	void *vif = netdev_priv(dev);
> > +	int i;
> > +
> > +	for (i = 0; i < ARRAY_SIZE(xenvif_stats); i++)
> > +		data[i] = *(int *)(vif + xenvif_stats[i].offset);
> 
> so use : data[i] = *(unsigned long *)(vif + xenvif_stats[i].offset);

Oops, will do, thanks.

Ian.



^ permalink raw reply

* Re: [PATCH][v2] dev : fix mtu check when TSO is enabled
From: Eric Dumazet @ 2011-03-14 20:02 UTC (permalink / raw)
  To: Daniel Lezcano; +Cc: davem, kaber, nightnord, netdev
In-Reply-To: <1300129859-9525-1-git-send-email-daniel.lezcano@free.fr>

Le lundi 14 mars 2011 à 20:10 +0100, Daniel Lezcano a écrit :
> In case the device where is coming from the packet has TSO enabled,
> we should not check the mtu size value as this one could be bigger
> than the expected value.
> 
> This is the case for the macvlan driver when the lower device has
> TSO enabled. The macvlan inherit this feature and forward the packets
> without fragmenting them. Then the packets go through dev_forward_skb
> and are dropped. This patch fix this by checking TSO is not enabled
> when we want to check the mtu size.
> 
> Signed-off-by: Daniel Lezcano <daniel.lezcano@free.fr>
> Cc: Eric Dumazet <eric.dumazet@gmail.com>
> Cc: Patrick McHardy <kaber@trash.net>
> Cc: Andrian Nord <nightnord@gmail.com>
> ---
>  net/core/dev.c |   24 ++++++++++++++++++++++--
>  1 files changed, 22 insertions(+), 2 deletions(-)
> 
> diff --git a/net/core/dev.c b/net/core/dev.c
> index 6561021..f1607a0 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -1503,6 +1503,27 @@ static inline void net_timestamp_check(struct sk_buff *skb)
>  		__net_timestamp(skb);
>  }
>  
> +static inline bool is_skb_forwardable(struct net_device *dev,
> +				      struct sk_buff *skb)
> +{
> +	unsigned int len;
> +
> +	if (!dev->flags & IFF_UP)
> +		return false;

	if (!(dev->flags & IFF_UP))
		return false;

> +
> +	/* we should not check the mtu size if TSO is enabled otherwise
> +	 * we may have a packet with a length bigger than the expected
> +	 * one as it was not segmented before */
> +	if (skb->dev && skb->dev->features & NETIF_F_TSO)
> +		return true;
> +
> +	len = dev->mtu + dev->hard_header_len + VLAN_HLEN;
> +	if (skb->len > len)
> +		return false;
> +


I suggest you reorder tests so that the first one is discriminant most
of the time

	maxlen = dev->mtu + dev->hard_header_len + VLAN_HLEN;
	if (skb->len < maxlen)
		return true;
	
	if (skb->dev && skb->dev->features & NETIF_F_TSO)
		return true;

	return false;

> +	return true;
> +}
> +
>  /**
>   * dev_forward_skb - loopback an skb to another netif
>   *
> @@ -1526,8 +1547,7 @@ int dev_forward_skb(struct net_device *dev, struct sk_buff *skb)
>  	skb_orphan(skb);
>  	nf_reset(skb);
>  
> -	if (unlikely(!(dev->flags & IFF_UP) ||
> -		     (skb->len > (dev->mtu + dev->hard_header_len + VLAN_HLEN)))) {
> +	if (unlikely(!is_skb_forwardable(dev, skb))) {
>  		atomic_long_inc(&dev->rx_dropped);
>  		kfree_skb(skb);
>  		return NET_RX_DROP;



^ permalink raw reply

* Re: [PATCH][v2] dev : fix mtu check when TSO is enabled
From: Daniel Lezcano @ 2011-03-14 20:07 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: davem, kaber, nightnord, netdev
In-Reply-To: <1300132932.2649.2.camel@edumazet-laptop>

On 03/14/2011 09:02 PM, Eric Dumazet wrote:
> Le lundi 14 mars 2011 à 20:10 +0100, Daniel Lezcano a écrit :
>> In case the device where is coming from the packet has TSO enabled,
>> we should not check the mtu size value as this one could be bigger
>> than the expected value.
>>
>> This is the case for the macvlan driver when the lower device has
>> TSO enabled. The macvlan inherit this feature and forward the packets
>> without fragmenting them. Then the packets go through dev_forward_skb
>> and are dropped. This patch fix this by checking TSO is not enabled
>> when we want to check the mtu size.
>>
>> Signed-off-by: Daniel Lezcano<daniel.lezcano@free.fr>
>> Cc: Eric Dumazet<eric.dumazet@gmail.com>
>> Cc: Patrick McHardy<kaber@trash.net>
>> Cc: Andrian Nord<nightnord@gmail.com>
>> ---
>>   net/core/dev.c |   24 ++++++++++++++++++++++--
>>   1 files changed, 22 insertions(+), 2 deletions(-)
>>
>> diff --git a/net/core/dev.c b/net/core/dev.c
>> index 6561021..f1607a0 100644
>> --- a/net/core/dev.c
>> +++ b/net/core/dev.c
>> @@ -1503,6 +1503,27 @@ static inline void net_timestamp_check(struct sk_buff *skb)
>>   		__net_timestamp(skb);
>>   }
>>
>> +static inline bool is_skb_forwardable(struct net_device *dev,
>> +				      struct sk_buff *skb)
>> +{
>> +	unsigned int len;
>> +
>> +	if (!dev->flags&  IFF_UP)
>> +		return false;
> 	if (!(dev->flags&  IFF_UP))
> 		return false;

Argh ! Hopefully you saw it.

>> +
>> +	/* we should not check the mtu size if TSO is enabled otherwise
>> +	 * we may have a packet with a length bigger than the expected
>> +	 * one as it was not segmented before */
>> +	if (skb->dev&&  skb->dev->features&  NETIF_F_TSO)
>> +		return true;
>> +
>> +	len = dev->mtu + dev->hard_header_len + VLAN_HLEN;
>> +	if (skb->len>  len)
>> +		return false;
>> +
> I suggest you reorder tests so that the first one is discriminant most
> of the time
>
> 	maxlen = dev->mtu + dev->hard_header_len + VLAN_HLEN;
> 	if (skb->len<  maxlen)
> 		return true;
> 	
> 	if (skb->dev&&  skb->dev->features&  NETIF_F_TSO)
> 		return true;
>
> 	return false;

Ok. Thanks !

   -- Daniel


^ permalink raw reply

* [PATCH] Fix overflow of name in struct net_device, replaced str* and sprintf with strn* and snprintf respectivly.
From: Sasikanth V @ 2011-03-14 20:07 UTC (permalink / raw)
  To: davem; +Cc: netdev, Sasikanth V


Signed-off-by: Sasikanth V <sasikanth.v19@gmail.com>
---
 net/core/dev.c |   14 +++++++-------
 1 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/net/core/dev.c b/net/core/dev.c
index 6561021..9d06c1e 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -510,7 +510,7 @@ int netdev_boot_setup_check(struct net_device *dev)
 
 	for (i = 0; i < NETDEV_BOOT_SETUP_MAX; i++) {
 		if (s[i].name[0] != '\0' && s[i].name[0] != ' ' &&
-		    !strcmp(dev->name, s[i].name)) {
+		    !strncmp(dev->name, s[i].name, IFNAMSIZ)) {
 			dev->irq 	= s[i].map.irq;
 			dev->base_addr 	= s[i].map.base_addr;
 			dev->mem_start 	= s[i].map.mem_start;
@@ -539,7 +539,7 @@ unsigned long netdev_boot_base(const char *prefix, int unit)
 	char name[IFNAMSIZ];
 	int i;
 
-	sprintf(name, "%s%d", prefix, unit);
+	snprintf(name, IFNAMSIZ, "%s%d", prefix, unit);
 
 	/*
 	 * If device already registered then return base of 1
@@ -549,7 +549,7 @@ unsigned long netdev_boot_base(const char *prefix, int unit)
 		return 1;
 
 	for (i = 0; i < NETDEV_BOOT_SETUP_MAX; i++)
-		if (!strcmp(name, s[i].name))
+		if (!strncmp(name, s[i].name, IFNAMSIZ))
 			return s[i].map.base_addr;
 	return 0;
 }
@@ -836,7 +836,7 @@ EXPORT_SYMBOL(dev_get_by_flags_rcu);
  */
 int dev_valid_name(const char *name)
 {
-	if (*name == '\0')
+	if (!name || *name == '\0')
 		return 0;
 	if (strlen(name) >= IFNAMSIZ)
 		return 0;
@@ -3834,7 +3834,7 @@ static int dev_ifname(struct net *net, struct ifreq __user *arg)
 		return -ENODEV;
 	}
 
-	strcpy(ifr.ifr_name, dev->name);
+	strncpy(ifr.ifr_name, dev->name, IFNAMSIZ);
 	rcu_read_unlock();
 
 	if (copy_to_user(arg, &ifr, sizeof(struct ifreq)))
@@ -4821,7 +4821,7 @@ int dev_ioctl(struct net *net, unsigned int cmd, void __user *arg)
 	if (copy_from_user(&ifr, arg, sizeof(struct ifreq)))
 		return -EFAULT;
 
-	ifr.ifr_name[IFNAMSIZ-1] = 0;
+	ifr.ifr_name[IFNAMSIZ-1] = '\0';
 
 	colon = strchr(ifr.ifr_name, ':');
 	if (colon)
@@ -5694,7 +5694,7 @@ struct net_device *alloc_netdev_mqs(int sizeof_priv, const char *name,
 		goto free_all;
 #endif
 
-	strcpy(dev->name, name);
+	strncpy(dev->name, name, IFNAMSIZ);
 	return dev;
 
 free_all:
-- 
1.7.3.4


^ permalink raw reply related

* Re: fcoe: correct checking for bonding
From: David Miller @ 2011-03-14 20:20 UTC (permalink / raw)
  To: jpirko
  Cc: robert.w.love, James.Bottomley, linux-scsi, devel, netdev, fubar,
	joe.eykholt
In-Reply-To: <20110314192201.GA2873@psychotron.redhat.com>

From: Jiri Pirko <jpirko@redhat.com>
Date: Mon, 14 Mar 2011 20:22:02 +0100

> Mon, Mar 14, 2011 at 08:04:07PM CET, robert.w.love@intel.com wrote:
>>On Sat, 2011-03-12 at 10:59 -0800, David Miller wrote:
>>> From: Jiri Pirko <jpirko@redhat.com>
>>> Date: Sat, 12 Mar 2011 13:01:10 +0100
>>> 
>>> > Thu, Mar 03, 2011 at 02:09:18AM CET, robert.w.love@intel.com wrote:
>>> >>On Wed, 2011-03-02 at 01:55 -0800, Jiri Pirko wrote:
>>> >>> Or perhaps this should be applied to net-next?
>>> >>> 
>>> >>I think this should go through scsi-misc as all the other
>>> >>libfc/libfcoe/fcoe patches do.
>>> >>
>>> >>> Wed, Mar 02, 2011 at 07:05:35AM CET, jpirko@redhat.com wrote:
>>> >>> >Check for bonding master and refuse to use that.
>>> >>> >
>>> >>> >Signed-off-by: Jiri Pirko <jpirko@redhat.com>
>>> >>> >---
>>> >>> > drivers/scsi/fcoe/fcoe.c |    4 +---
>>> >>> > 1 files changed, 1 insertions(+), 3 deletions(-)
>>> >>> >
>>> >>> >diff --git a/drivers/scsi/fcoe/fcoe.c b/drivers/scsi/fcoe/fcoe.c
>>> >>> >index 9f9600b..3becc6a 100644
>>> >>> >--- a/drivers/scsi/fcoe/fcoe.c
>>> >>> >+++ b/drivers/scsi/fcoe/fcoe.c
>>> >>> >@@ -285,9 +285,7 @@ static int fcoe_interface_setup(struct fcoe_interface *fcoe,
>>> >>> > 	}
>>> >>> > 
>>> >>> > 	/* Do not support for bonding device */
>>> >>> >-	if ((netdev->priv_flags & IFF_MASTER_ALB) ||
>>> >>> >-	    (netdev->priv_flags & IFF_SLAVE_INACTIVE) ||
>>> >>> >-	    (netdev->priv_flags & IFF_MASTER_8023AD)) {
>>> >>> >+	if (netdev->priv_flags & IFF_BONDING && netdev->flags & IFF_MASTER) {
>>> >>> > 		FCOE_NETDEV_DBG(netdev, "Bonded interfaces not supported\n");
>>> >>> > 		return -EOPNOTSUPP;
>>> >>> > 	}
>>> >>> >-- 
>>> >>> >1.7.3.4
>>> >>> >
>>> >>
>>> >>James, feel free to pick up this patch. I don't have anything in my fcoe
>>> >>tree right now that it would conflict with. I'll also put it in my tree
>>> >>and resend if you don't put it into scsi-misc directly.
>>> > 
>>> > What's the status of this? Maybe this should rather go thru net-next
>>> 
>>> Sure, I can take this.  I'll look at it later.
>>
>>Hi Dave,
>>
>>   I'd rather have this patch go through scsi-misc. Most, if not all,
>>libfc, libfcoe and fcoe patches have taken this path. The way it has
>>been working is that I have been collecting fcoe patches and re-posting
>>them to scsi-misc after I have reviewed them and done some basic
>>testing.
>>
>>   Taking a patch like this through net{-next} could cause a merge
>>problem at Linus' level if a later patch makes it though the normal
>>process and conflicts. This is what I want to avoid.
>>
>>   This patch, although appreciated, isn't critical. I have collected it
>>into my tree and will re-post it to scsi-misc. I see no reason to treat
>>this patch differently from other patches.
> 
> Well I have another set of patches dependent on this one :(

True, also I think Rob is overreacting.

Any merge problems created will be handled properly by Linus.

I recently changed the interface to ipv4 and ipv6 route lookups, and
this required all kinds of changes to stuff under Infiniband and elsewhere.
It's the only sane way to handle this kind of thing.

^ permalink raw reply

* Re: [GIT/PATCH v5] xen network backend driver
From: David Miller @ 2011-03-14 20:23 UTC (permalink / raw)
  To: Ian.Campbell
  Cc: bhutchings, netdev, xen-devel, jeremy, herbert, konrad.wilk,
	romieu, shemminger, mirqus
In-Reply-To: <1300131040.32696.23.camel@localhost.localdomain>

From: Ian Campbell <Ian.Campbell@eu.citrix.com>
Date: Mon, 14 Mar 2011 19:30:40 +0000

> On Mon, 2011-03-14 at 18:40 +0000, Ben Hutchings wrote:
>> I know you have asked for this to be pulled, in order to retain its
>> history, but I'm not sure that David Miller will do this unless the
>> driver is at least buildable at each stage in the history.  (However,
>> given that the driver wouldn't be enabled before it's added, I'm not
>> sure it matters in this case.)
> 
> I think it should be, at least from the second commit onwards (modulo
> any mistakes made in the past).

I don't want to retain all of that history, there are so many broken
points in the history that the only thing you can do is go back and
add Kconfig commits to depend the driver upon BROKEN, and that in effect
changes the history devalueing it even further.

I just want one patch that adds the driver in it's most current state,
and that's all.

Thanks.

^ permalink raw reply

* Re: [Bloat] shaper team forming up
From: Eric Dumazet @ 2011-03-14 20:24 UTC (permalink / raw)
  To: Jonathan Morton; +Cc: Dave Täht, bloat-devel, bloat, netdev
In-Reply-To: <5BC42741-852B-4699-BA5D-D70B8D610D96@gmail.com>

Le lundi 14 mars 2011 à 21:55 +0200, Jonathan Morton a écrit :
> On 14 Mar, 2011, at 9:26 pm, Dave Täht wrote:
> 
> > Over the weekend, Dan Siemons uncovered a possible bad interaction
> > between ECN and the default pfifo_fast qdisc in Linux.
> > 
> > http://www.coverfire.com/archives/2011/03/13/pfifo_fast-and-ecn/
> 
> This seems to be more complicated that it appears.  It looks as though
> Linux has re-used the LSB of the old TOS field for some "link local"
> flag which is used by routing.
> 
> It's not immediately obvious whether pfifo_fast is using this new
> interpretation though.  If it isn't, the fix should be to remove the
> RTO_ONLINK bit from the mask it's using on the tos field.  The other
> half of the mask correctly excludes the ECN bits from the field.
> 

CC netdev, where linux network dev can take a look.

I would say that this is a wrong analysis : 

1) ECN uses two low order bits of TOS byte

2) pfifo_fast uses skb->priority


skb->priority = rt_tos2priority(iph->tos);

#define IPTOS_TOS_MASK            0x1E
#define IPTOS_TOS(tos)            ((tos)&IPTOS_TOS_MASK)

static inline char rt_tos2priority(u8 tos)
{
	return ip_tos2prio[IPTOS_TOS(tos)>>1];
}

No interference between two mechanisms, unless sysadmin messed up things
(skb_edit)




^ 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