Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH v2 00/12] make rpc_pipefs be mountable multiple time
From: Kirill A. Shutemov @ 2011-01-07 11:12 UTC (permalink / raw)
  To: Al Viro
  Cc: Kirill A. Shutemov, Trond Myklebust, J. Bruce Fields, Neil Brown,
	Pavel Emelyanov, linux-nfs, David S. Miller, netdev, linux-kernel
In-Reply-To: <20110105114155.GN19804@ZenIV.linux.org.uk>

On Wed, Jan 05, 2011 at 11:41:55AM +0000, Al Viro wrote:
> On Wed, Dec 29, 2010 at 03:14:18PM +0200, Kirill A. Shutemov wrote:
> > Prepare nfs/sunrpc stack to use multiple instances of rpc_pipefs.
> 
> Won't that make sunrpc impossible to rmmod once you've got it in?
> Note that having a reference to vfsmount pins it down, which pins
> the superblock down, which pins the file_system_type down, which
> pins the damn module down.  So cleanup_sunrpc() won't be ever called,
> AFAICS...

Hm.. rpc_pipe_fs_type.owner = NULL seems fix the problem.
Is it valid solution in this case?

-- 
 Kirill A. Shutemov

^ permalink raw reply

* Re: [PATCH v2 00/12] make rpc_pipefs be mountable multiple time
From: Kirill A. Shutemov @ 2011-01-07 11:19 UTC (permalink / raw)
  To: Al Viro
  Cc: Trond Myklebust, J. Bruce Fields, Neil Brown, Pavel Emelyanov,
	linux-nfs, David S. Miller, netdev, linux-kernel
In-Reply-To: <20110107111222.GA19395@shutemov.name>

On Fri, Jan 07, 2011 at 01:12:22PM +0200, Kirill A. Shutemov wrote:
> On Wed, Jan 05, 2011 at 11:41:55AM +0000, Al Viro wrote:
> > On Wed, Dec 29, 2010 at 03:14:18PM +0200, Kirill A. Shutemov wrote:
> > > Prepare nfs/sunrpc stack to use multiple instances of rpc_pipefs.
> > 
> > Won't that make sunrpc impossible to rmmod once you've got it in?
> > Note that having a reference to vfsmount pins it down, which pins
> > the superblock down, which pins the file_system_type down, which
> > pins the damn module down.  So cleanup_sunrpc() won't be ever called,
> > AFAICS...
> 
> Hm.. rpc_pipe_fs_type.owner = NULL seems fix the problem.
> Is it valid solution in this case?

Please, ignore it. :)

-- 
 Kirill A. Shutemov

^ permalink raw reply

* Re: rndis gadget: Inconsistent locking
From: David Brownell @ 2011-01-07 11:20 UTC (permalink / raw)
  To: Neil Jones; +Cc: linux-usb, netdev
In-Reply-To: <AANLkTi=OS+jernOe8y1fbMbx5Rf+JRcvzqGY4Z2LKqsH@mail.gmail.com>



> Yes Michal Nazarewicz has seen this on a S3C UDC, 

Good to confirm that.  As I mentioned, other folk have seen much the same bug on other hardware; we
know it's neither a new nor a HW-specific issue.

Correction to an earlier comment of mine: 

https://patchwork.kernel.org/patch/195562/
does correctly identify the commit which
caused this bug in multiple drivers.

However, that patch won't apply to the latest
tree from Linus.

Michal, can you update and resubmit, so I can
at least ack your fix and xpedite its merge?

It'd be good  to have RNDIS work again, a fair
number of folk rely on it, and this seems to be
the main obstacle to continuing to do that.


- Dave






^ permalink raw reply

* Re: possible issue between bridge igmp/multicast  handling & bnx2x on kernel 2.6.34 and >
From: Eric Dumazet @ 2011-01-07 11:28 UTC (permalink / raw)
  To: Yann Dupont; +Cc: netdev
In-Reply-To: <4D26EDA4.7060502@univ-nantes.fr>

Le vendredi 07 janvier 2011 à 11:40 +0100, Yann Dupont a écrit :
> Le 04/01/2011 14:40, Yann Dupont a écrit :
> ...
> > We just added BCM57711 10G cards (bnx2x driver) on our blade servers 
> > (connected to 10G Power Connect M8024).
> > Since then, we are experiencing random lost of packets.
> >
> > Symptom : packets are lost on some vlans for a few seconds, then 
> > things go back to normal (and stops again a few minutes later)
> >
> 
> As I didn't had answer so far , I digged a little more and captured more 
> packets.
> I just noticed that an event trigger that problem : IPv6 neighbor 
> discovery packet .
> 
> This is , of course, a multicast packet.
> 
> Just saw that 2.6.36.3 should include this fix :
> 
> > From: David Stevens<dlstevens@us.ibm.com>
> >
> > [ Upstream commit 04bdf0c9a451863e50fff627713a900a2cabb998 ]
> >
> > This patch fixes a missing ntohs() for bridge IPv6 multicast snooping.
> But in fact , I just tested, and this doesn't cure the problem :(
> 
> This bug
> - only occurs with bnx2x with tagged vlans, attached to bridges. Other 
> interfaces (bnx2 , for exemple) works fine. bnx2x without bridges works 
> fine.
> - only happens when bridge is compiled with CONFIG_BRIDGE_IGMP_SNOOPING 
> (default setting)
> - is triggered by IPv6 neighbor discovery packet. Just after that 
> packet, others packets are discarded for some time.
> - packets originating from same vlans are not affected, only packets 
> previously routed are discarded. Examinating those packets, I don't 
> undersand why, apart TTL (and mac address), they seems similar .
> - has origin circa 2.6.33 :
> fe2d7c70b747d5d968f4e8fa210676d49d40059 is the first bad commit
> commit 3fe2d7c70b747d5d968f4e8fa210676d49d40059
> Author: Herbert Xu <herbert@gondor.apana.org.au>
> Date:   Sun Feb 28 00:49:38 2010 -0800
> 
>      bridge: Add multicast start/stop hooks
> 
>      This patch hooks up the bridge start/stop and add/delete/disable
>      port functions to the new multicast module.
> 
>      Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
>      Signed-off-by: David S. Miller <davem@davemloft.net>
> 
> 
> 
> What can I do to help fixing this bug ?
> regards,
> 

Please take a look at whole thread at
https://lkml.org/lkml/2010/8/13/200

I guess this is a similar problem.




^ permalink raw reply

* net-next-2.6 [PATCH 0/3] dccp: several sequence-number validation fixes
From: Gerrit Renker @ 2011-01-07 11:35 UTC (permalink / raw)
  To: davem; +Cc: dccp, netdev
In-Reply-To: <0110107112837.GA5315@gerrit.erg.abdn.ac.uk>

Hi Dave,

please find attached 3 bug fixes which recently came up on dccp@vger.

 Patch #1: fixes a bug which wrongly classified sequence-invalid packets. 
 Patch #2: fixes a bug in updating the Greatest Sequence number Received (GSR).
 Patch #3: fixes an inconsistency in setting the sequence window on 32/64 bit.

I have also placed this in into a fresh (today's) copy of net-next-2.6, on

    git://eden-feed.erg.abdn.ac.uk/net-next-2.6        [subtree 'dccp']

---
 Documentation/networking/dccp.txt |    1 +
 net/dccp/dccp.h                   |    3 ++-
 net/dccp/input.c                  |    2 +-
 net/dccp/sysctl.c                 |    4 +++-
 4 files changed, 7 insertions(+), 3 deletions(-)

^ permalink raw reply

* [PATCH 1/3] dccp: fix return value for sequence-invalid packets
From: Gerrit Renker @ 2011-01-07 11:35 UTC (permalink / raw)
  To: davem; +Cc: dccp, netdev, Samuel Jero
In-Reply-To: <1294400130-5604-1-git-send-email-gerrit@erg.abdn.ac.uk>

From: Samuel Jero <sj323707@ohio.edu>

Currently dccp_check_seqno returns 0 (indicating a valid packet) if the
acknowledgment number is out of bounds and the sync that RFC 4340 mandates at
this point is currently being rate-limited. This function should return -1,
indicating an invalid packet.

Signed-off-by: Samuel Jero <sj323707@ohio.edu>
Acked-by: Gerrit Renker <gerrit@erg.abdn.ac.uk>
---
 net/dccp/input.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

--- a/net/dccp/input.c
+++ b/net/dccp/input.c
@@ -260,7 +260,7 @@ static int dccp_check_seqno(struct sock *sk, struct sk_buff *skb)
 		 */
 		if (time_before(now, (dp->dccps_rate_last +
 				      sysctl_dccp_sync_ratelimit)))
-			return 0;
+			return -1;
 
 		DCCP_WARN("Step 6 failed for %s packet, "
 			  "(LSWL(%llu) <= P.seqno(%llu) <= S.SWH(%llu)) and "

^ permalink raw reply

* [PATCH 2/3] dccp: fix bug in updating the GSR
From: Gerrit Renker @ 2011-01-07 11:35 UTC (permalink / raw)
  To: davem; +Cc: dccp, netdev, Samuel Jero
In-Reply-To: <1294400130-5604-2-git-send-email-gerrit@erg.abdn.ac.uk>

From: Samuel Jero <sj323707@ohio.edu>

Currently dccp_check_seqno allows any valid packet to update the Greatest
Sequence Number Received, even if that packet's sequence number is less than
the current GSR. This patch adds a check to make sure that the new packet's
sequence number is greater than GSR.

Signed-off-by: Samuel Jero <sj323707@ohio.edu>
Signed-off-by: Gerrit Renker <gerrit@erg.abdn.ac.uk>
---
 net/dccp/dccp.h |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

--- a/net/dccp/dccp.h
+++ b/net/dccp/dccp.h
@@ -426,7 +426,8 @@ static inline void dccp_update_gsr(struct sock *sk, u64 seq)
 {
 	struct dccp_sock *dp = dccp_sk(sk);
 
-	dp->dccps_gsr = seq;
+	if (after48(seq, dp->dccps_gsr))
+		dp->dccps_gsr = seq;
 	/* Sequence validity window depends on remote Sequence Window (7.5.1) */
 	dp->dccps_swl = SUB48(ADD48(dp->dccps_gsr, 1), dp->dccps_r_seq_win / 4);
 	/*

^ permalink raw reply

* [PATCH 3/3] dccp: make upper bound for seq_window consistent on 32/64 bit
From: Gerrit Renker @ 2011-01-07 11:35 UTC (permalink / raw)
  To: davem; +Cc: dccp, netdev, Gerrit Renker
In-Reply-To: <1294400130-5604-3-git-send-email-gerrit@erg.abdn.ac.uk>

The 'seq_window' sysctl sets the initial value for the DCCP Sequence Window,
which may range from 32..2^46-1 (RFC 4340, 7.5.2). The patch sets the upper
bound consistently to 2^32-1 on both 32 and 64 bit systems, which should be
sufficient - with a RTT of 1sec and 1-byte packets, a seq_window of 2^32-1
corresponds to a link speed of 34 Gbps.

Signed-off-by: Gerrit Renker <gerrit@erg.abdn.ac.uk>
---
 Documentation/networking/dccp.txt |    1 +
 net/dccp/sysctl.c                 |    4 +++-
 2 files changed, 4 insertions(+), 1 deletions(-)

--- a/Documentation/networking/dccp.txt
+++ b/Documentation/networking/dccp.txt
@@ -167,6 +167,7 @@ rx_ccid = 2
 seq_window = 100
 	The initial sequence window (sec. 7.5.2) of the sender. This influences
 	the local ackno validity and the remote seqno validity windows (7.5.1).
+	Values in the range Wmin = 32 (RFC 4340, 7.5.2) up to 2^32-1 can be set.
 
 tx_qlen = 5
 	The size of the transmit buffer in packets. A value of 0 corresponds
--- a/net/dccp/sysctl.c
+++ b/net/dccp/sysctl.c
@@ -21,7 +21,8 @@
 /* Boundary values */
 static int		zero     = 0,
 			u8_max   = 0xFF;
-static unsigned long	seqw_min = 32;
+static unsigned long	seqw_min = DCCPF_SEQ_WMIN,
+			seqw_max = 0xFFFFFFFF;		/* maximum on 32 bit */
 
 static struct ctl_table dccp_default_table[] = {
 	{
@@ -31,6 +32,7 @@ static struct ctl_table dccp_default_table[] = {
 		.mode		= 0644,
 		.proc_handler	= proc_doulongvec_minmax,
 		.extra1		= &seqw_min,		/* RFC 4340, 7.5.2 */
+		.extra2		= &seqw_max,
 	},
 	{
 		.procname	= "rx_ccid",

^ permalink raw reply

* Re: [PATCH] net/r8169: Update the function of parsing firmware
From: Francois Romieu @ 2011-01-07 11:44 UTC (permalink / raw)
  To: Hayes Wang; +Cc: netdev, linux-kernel
In-Reply-To: <1294393509-3492-1-git-send-email-hayeswang@realtek.com>

Hayes Wang <hayeswang@realtek.com> :
> Update rtl_phy_write_fw function. The new function could
> parse the complex firmware which is used by RTL8111E and later.
> The new firmware may read data and do some operations, not just
> do writing only.
> 
> Signed-off-by: Hayes Wang <hayeswang@realtek.com>
> ---
>  drivers/net/r8169.c |  112 ++++++++++++++++++++++++++++++++++++++++++++-------
>  1 files changed, 97 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/net/r8169.c b/drivers/net/r8169.c
> index 27a7c20..2115424 100644
> --- a/drivers/net/r8169.c
> +++ b/drivers/net/r8169.c
> @@ -1632,39 +1632,121 @@ rtl_phy_write_fw(struct rtl8169_private *tp, const struct firmware *fw)
>  {
>  	__le32 *phytable = (__le32 *)fw->data;
>  	struct net_device *dev = tp->dev;
> -	size_t i;
> +	size_t index;

...

> +	u32 predata, count;
>  
>  	if (fw->size % sizeof(*phytable)) {
> -		netif_err(tp, probe, dev, "odd sized firmware %zd\n", fw->size);
> +		netif_err(tp, probe, dev, "odd sized firmware %zd\n",
> +			  fw->size);

It fits within 80 columns, please keep it as is.

>  		return;
>  	}
>  
> -	for (i = 0; i < fw->size / sizeof(*phytable); i++) {
> -		u32 action = le32_to_cpu(phytable[i]);
> +	for (index = 0; index < fw->size / sizeof(*phytable); index++) {
> +		u32 action = le32_to_cpu(phytable[index]);
>  
> -		if (!action)
> +		switch(action & 0xF0000000) {

Please don't add uppercase hex.

> +		case PHY_READ:
> +		case PHY_DATA_OR:
> +		case PHY_DATA_AND:
> +		case PHY_BJMPN:
> +		case PHY_READ_EFUSE:
> +		case PHY_CLEAR_READCOUNT:
> +		case PHY_WRITE:
> +		case PHY_READCOUNT_EQ_SKIP:
> +		case PHY_COMP_EQ_SKIPN:
> +		case PHY_COMP_NEQ_SKIPN:
> +		case PHY_WRITE_PREVIOUS:
> +		case PHY_SKIPN:
> +		case PHY_DELAY_MS:
>  			break;
[...]
> +	for (index = 0; index < fw->size / sizeof(*phytable); ) {
[...]
> +
> +		if (index < 0)
> +			BUG();

It could imho validate a bit harder.

-- 
Ueimor

^ permalink raw reply

* Re: genetlink misinterprets NEW as GET
From: Pablo Neira Ayuso @ 2011-01-07 12:12 UTC (permalink / raw)
  To: Jan Engelhardt
  Cc: Ben Pfaff, Netfilter Developer Mailing List,
	Linux Networking Developer Mailing List
In-Reply-To: <alpine.LNX.2.01.1101071033400.25363@obet.zrqbmnf.qr>

On 07/01/11 10:38, Jan Engelhardt wrote:
> 
> On Friday 2011-01-07 02:31, Pablo Neira Ayuso wrote:
>>>>> 	/* Modifiers to GET request */
>>>>> 	#define NLM_F_ROOT      0x100
>>>>> 	#define NLM_F_MATCH     0x200
>>>>> 	#define NLM_F_ATOMIC    0x400
>>>>> 	#define NLM_F_DUMP      (NLM_F_ROOT|NLM_F_MATCH)
>>> [...]
>>>>> [N.B.: I am also wondering whether
>>>>> 	(nlh->nlmsg_flags & NLM_F_DUMP) == NLM_F_DUMP
>>>>> may have been desired, because NLM_F_DUMP is composed of two bits.]
>>>>
>>>> Someone may include NLM_F_ATOMIC to a dump operation, in that case the
>>>> checking that you propose is not valid.
>>>
>>> Are you saying that NLM_F_MATCH and NLM_F_ATOMIC are mutually
>>> exclusive, and that NLM_F_ROOT|NLM_F_ATOMIC would also signal a
>>> dump operation?  Otherwise the test that Jan proposes looks valid
>>> to me.
>>
>> Indeed, Jan's test is fine to fix this. Please, send a patch to Davem asap.
> 
> But that would still mean that a user sending a
> NLM_F_REQUEST|NLM_F_REPLACE|NLM_F_EXCL message would be misinterpreted
> as NLM_F_DUMP.

That flag combination does not make sense to me. Valid combinations are:

NLM_F_REQUEST|NLM_F_CREATE : if it does not exist, create it, if it
exists, update it.
NLM_F_REQUEST|NLM_F_CREATE|NLM_F_EXCL: if it does not exist, create it,
if it exists, return -EEXIST.
NLM_F_REQUEST|NLM_F_REPLACE: if it does not exist, return -ENOENT, if it
exists, replace it.

^ permalink raw reply

* Re: [PATCH] ehea: Add some info messages and fix an issue
From: Breno Leitao @ 2011-01-07 12:14 UTC (permalink / raw)
  To: Anton Blanchard; +Cc: joe, netdev, davem
In-Reply-To: <20110107142430.42fda498@kryten>

Hi Anton,

On 01/07/2011 01:24 AM, Anton Blanchard wrote:
> It looks like you are now only initialising half the ring, but still
>  telling the hardware to use the whole ring. Once you get through the
>  entire ring once the errors go away.
You are correct. The problem in the past ehea_init_fill_rq1() wasn't
respecting the nr_rq1a parameter. So, every time ehea_init_fill_rqX()
was trying to allocated the entire skb arrary, and assume that the
entire array was allocated, which wasn't correct.

My patch just allocate the requested number of skbs (described in
nr_rq1a) in skb array, and tell hea that only that amount of skb were
allocated (via doorbell).

On the other side, ehea_proc_rwqes() tries to maximize the array usage,
meaning that if the array is not completely used, it'd try to allocate
an skb "on-the-fly", and this is expected (For example, when you
initialize the system on memory pressure)

That is why I sent another patch that turns this message a netif_info()
instead of a netif_error() (as it was in the past). The commit id for
this patch is 782615aea84e57dc7f2f922cea823df3de635a78

So, although this behaviour is completely expected, this code path
should only being executed on memory pressure. But I am suspecting this
code path is execute more often than I'd expect. Let me investigate this.

Thanks
Breno

^ permalink raw reply

* Re: [PATCH net-next-2.6 v2 1/2] can: add driver for Softing card
From: Kurt Van Dijck @ 2011-01-07 12:29 UTC (permalink / raw)
  To: Wolfgang Grandegger
  Cc: socketcan-core-0fE9KPoRgkgATYTw5x5z8w,
	netdev-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <4D25ED70.7000303-5Yr1BZd7O62+XT7JhA+gdA@public.gmane.org>

On Thu, Jan 06, 2011 at 05:27:28PM +0100, Wolfgang Grandegger wrote:
> >> - Please avoid alignment of expressions and structure members.
> 
I agree with most of them.
The alignment of register offset macro's is 1 thing I intend to
keep as is ...
Maybe I put a comment in front to explain why it's aligned...

Kurt
> 

^ permalink raw reply

* Re: [PATCH 3/5] NET: IPV4: ARP: allow to invalidate specific ARP entries
From: Maxim Levitsky @ 2011-01-07 12:47 UTC (permalink / raw)
  To: linux1394-devel
  Cc: Stefan Richter, netdev, David S. Miller, Alexey Kuznetsov,
	James Morris, Patrick McHardy
In-Reply-To: <1290996593-32416-4-git-send-email-maximlevitsky@gmail.com>

On Mon, 2010-11-29 at 04:09 +0200, Maxim Levitsky wrote:
> IPv4 over firewire needs to be able to remove ARP entries
> from the ARP cache that belong to nodes that are removed, because
> IPv4 over firewire uses ARP packets for private information
> about nodes.
> 
> This information becomes invalid as soon as node drops
> off the bus and when it reconnects, its only possible
> to start takling to is after it responded to an ARP packet.
> But ARP cache prevents such packets from being sent.
> 
> CC: netdev@vger.kernel.org
> CC: "David S. Miller" <davem@davemloft.net>
> CC: Alexey Kuznetsov <kuznet@ms2.inr.ac.ru>
> CC: James Morris <jmorris@namei.org>
> CC: Patrick McHardy <kaber@trash.net>

Anybody?

Best regards,
	Maxim Levitsky

> 
> 
> Signed-off-by: Maxim Levitsky <maximlevitsky@gmail.com>
> ---
>  include/net/arp.h |    1 +
>  net/ipv4/arp.c    |   29 ++++++++++++++++++-----------
>  2 files changed, 19 insertions(+), 11 deletions(-)
> 
> diff --git a/include/net/arp.h b/include/net/arp.h
> index f4cf6ce..91f0568 100644
> --- a/include/net/arp.h
> +++ b/include/net/arp.h
> @@ -25,5 +25,6 @@ extern struct sk_buff *arp_create(int type, int ptype, __be32 dest_ip,
>  				  const unsigned char *src_hw,
>  				  const unsigned char *target_hw);
>  extern void arp_xmit(struct sk_buff *skb);
> +int arp_invalidate(struct net_device *dev, __be32 ip);
>  
>  #endif	/* _ARP_H */
> diff --git a/net/ipv4/arp.c b/net/ipv4/arp.c
> index d8e540c..35b1272 100644
> --- a/net/ipv4/arp.c
> +++ b/net/ipv4/arp.c
> @@ -1142,6 +1142,23 @@ static int arp_req_get(struct arpreq *r, struct net_device *dev)
>  	return err;
>  }
>  
> +int arp_invalidate(struct net_device *dev, __be32 ip)
> +{
> +	int err = -ENXIO;
> +	struct neighbour *neigh = neigh_lookup(&arp_tbl, &ip, dev);
> +
> +	if (neigh) {
> +		if (neigh->nud_state & ~NUD_NOARP)
> +			err = neigh_update(neigh, NULL, NUD_FAILED,
> +					   NEIGH_UPDATE_F_OVERRIDE|
> +					   NEIGH_UPDATE_F_ADMIN);
> +		neigh_release(neigh);
> +	}
> +
> +	return err;
> +}
> +EXPORT_SYMBOL(arp_invalidate);
> +
>  static int arp_req_delete_public(struct net *net, struct arpreq *r,
>  		struct net_device *dev)
>  {
> @@ -1162,7 +1179,6 @@ static int arp_req_delete(struct net *net, struct arpreq *r,
>  {
>  	int err;
>  	__be32 ip;
> -	struct neighbour *neigh;
>  
>  	if (r->arp_flags & ATF_PUBL)
>  		return arp_req_delete_public(net, r, dev);
> @@ -1180,16 +1196,7 @@ static int arp_req_delete(struct net *net, struct arpreq *r,
>  		if (!dev)
>  			return -EINVAL;
>  	}
> -	err = -ENXIO;
> -	neigh = neigh_lookup(&arp_tbl, &ip, dev);
> -	if (neigh) {
> -		if (neigh->nud_state & ~NUD_NOARP)
> -			err = neigh_update(neigh, NULL, NUD_FAILED,
> -					   NEIGH_UPDATE_F_OVERRIDE|
> -					   NEIGH_UPDATE_F_ADMIN);
> -		neigh_release(neigh);
> -	}
> -	return err;
> +	return arp_invalidate(dev, ip);
>  }
>  
>  /*



^ permalink raw reply

* [PATCH] CAIF: Fix IPv6 support in receive path for GPRS/3G
From: Sjur Brændeland @ 2011-01-07 11:57 UTC (permalink / raw)
  To: davem, netdev; +Cc: Kumar Sanghvi, Sjur Braendeland

From: Kumar Sanghvi <kumar.sanghvi@stericsson.com>

Checks version field of IP in the receive path for GPRS/3G data
and appropriately sets the value of skb->protocol.

Signed-off-by: Sjur Braendeland <sjur.brandeland@stericsson.com>
---
 net/caif/chnl_net.c |   18 +++++++++++++++++-
 1 files changed, 17 insertions(+), 1 deletions(-)

diff --git a/net/caif/chnl_net.c b/net/caif/chnl_net.c
index 84a422c..fa9dab3 100644
--- a/net/caif/chnl_net.c
+++ b/net/caif/chnl_net.c
@@ -76,6 +76,8 @@ static int chnl_recv_cb(struct cflayer *layr, struct cfpkt *pkt)
 	struct chnl_net *priv  = container_of(layr, struct chnl_net, chnl);
 	int pktlen;
 	int err = 0;
+	const u8 *ip_version;
+	u8 buf;
 
 	priv = container_of(layr, struct chnl_net, chnl);
 
@@ -90,7 +92,21 @@ static int chnl_recv_cb(struct cflayer *layr, struct cfpkt *pkt)
 	 * send the packet to the net stack.
 	 */
 	skb->dev = priv->netdev;
-	skb->protocol = htons(ETH_P_IP);
+
+	/* check the version of IP */
+	ip_version = skb_header_pointer(skb, 0, 1, &buf);
+	if (!ip_version)
+		return -EINVAL;
+	switch (*ip_version >> 4) {
+	case 4:
+		skb->protocol = htons(ETH_P_IP);
+		break;
+	case 6:
+		skb->protocol = htons(ETH_P_IPV6);
+		break;
+	default:
+		return -EINVAL;
+	}
 
 	/* If we change the header in loop mode, the checksum is corrupted. */
 	if (priv->conn_req.protocol == CAIFPROTO_DATAGRAM_LOOP)
-- 
1.7.0.4


^ permalink raw reply related

* Re: [PATCH 3/5] NET: IPV4: ARP: allow to invalidate specific ARP entries
From: Eric Dumazet @ 2011-01-07 12:57 UTC (permalink / raw)
  To: Maxim Levitsky
  Cc: netdev, Morris, Patrick McHardy, Stefan Richter, James,
	Alexey Kuznetsov, linux1394-devel, David S. Miller
In-Reply-To: <1294404478.7674.0.camel@maxim-laptop>

Le vendredi 07 janvier 2011 à 14:47 +0200, Maxim Levitsky a écrit :
> On Mon, 2010-11-29 at 04:09 +0200, Maxim Levitsky wrote:
> > IPv4 over firewire needs to be able to remove ARP entries
> > from the ARP cache that belong to nodes that are removed, because
> > IPv4 over firewire uses ARP packets for private information
> > about nodes.
> > 
> > This information becomes invalid as soon as node drops
> > off the bus and when it reconnects, its only possible
> > to start takling to is after it responded to an ARP packet.
> > But ARP cache prevents such packets from being sent.
> > 
> > CC: netdev@vger.kernel.org
> > CC: "David S. Miller" <davem@davemloft.net>
> > CC: Alexey Kuznetsov <kuznet@ms2.inr.ac.ru>
> > CC: James Morris <jmorris@namei.org>
> > CC: Patrick McHardy <kaber@trash.net>
> 
> Anybody?
> 
> Best regards,
> 	Maxim Levitsky
> 
> > 
> > 
> > Signed-off-by: Maxim Levitsky <maximlevitsky@gmail.com>
> > ---
> >  include/net/arp.h |    1 +
> >  net/ipv4/arp.c    |   29 ++++++++++++++++++-----------
> >  2 files changed, 19 insertions(+), 11 deletions(-)
> > 
> > diff --git a/include/net/arp.h b/include/net/arp.h
> > index f4cf6ce..91f0568 100644
> > --- a/include/net/arp.h
> > +++ b/include/net/arp.h
> > @@ -25,5 +25,6 @@ extern struct sk_buff *arp_create(int type, int ptype, __be32 dest_ip,
> >  				  const unsigned char *src_hw,
> >  				  const unsigned char *target_hw);
> >  extern void arp_xmit(struct sk_buff *skb);
> > +int arp_invalidate(struct net_device *dev, __be32 ip);
> >  
> >  #endif	/* _ARP_H */
> > diff --git a/net/ipv4/arp.c b/net/ipv4/arp.c
> > index d8e540c..35b1272 100644
> > --- a/net/ipv4/arp.c
> > +++ b/net/ipv4/arp.c
> > @@ -1142,6 +1142,23 @@ static int arp_req_get(struct arpreq *r, struct net_device *dev)
> >  	return err;
> >  }
> >  
> > +int arp_invalidate(struct net_device *dev, __be32 ip)
> > +{
> > +	int err = -ENXIO;
> > +	struct neighbour *neigh = neigh_lookup(&arp_tbl, &ip, dev);
> > +
> > +	if (neigh) {
> > +		if (neigh->nud_state & ~NUD_NOARP)
> > +			err = neigh_update(neigh, NULL, NUD_FAILED,
> > +					   NEIGH_UPDATE_F_OVERRIDE|
> > +					   NEIGH_UPDATE_F_ADMIN);
> > +		neigh_release(neigh);
> > +	}
> > +
> > +	return err;
> > +}
> > +EXPORT_SYMBOL(arp_invalidate);
> > +
> >  static int arp_req_delete_public(struct net *net, struct arpreq *r,
> >  		struct net_device *dev)
> >  {
> > @@ -1162,7 +1179,6 @@ static int arp_req_delete(struct net *net, struct arpreq *r,
> >  {
> >  	int err;
> >  	__be32 ip;
> > -	struct neighbour *neigh;
> >  
> >  	if (r->arp_flags & ATF_PUBL)
> >  		return arp_req_delete_public(net, r, dev);
> > @@ -1180,16 +1196,7 @@ static int arp_req_delete(struct net *net, struct arpreq *r,
> >  		if (!dev)
> >  			return -EINVAL;
> >  	}
> > -	err = -ENXIO;
> > -	neigh = neigh_lookup(&arp_tbl, &ip, dev);
> > -	if (neigh) {
> > -		if (neigh->nud_state & ~NUD_NOARP)
> > -			err = neigh_update(neigh, NULL, NUD_FAILED,
> > -					   NEIGH_UPDATE_F_OVERRIDE|
> > -					   NEIGH_UPDATE_F_ADMIN);
> > -		neigh_release(neigh);
> > -	}
> > -	return err;
> > +	return arp_invalidate(dev, ip);
> >  }
> >  
> >  /*
> 

Hi Maxim

You were supposed to respin your patch after my commit :

(941666c2e3e0f9f6a1 net: RCU conversion of dev_getbyhwaddr() and
arp_ioctl())

Thanks



------------------------------------------------------------------------------
Gaining the trust of online customers is vital for the success of any company
that requires sensitive data to be transmitted over the Web.   Learn how to 
best implement a security strategy that keeps consumers' information secure 
and instills the confidence they need to proceed with transactions.
http://p.sf.net/sfu/oracle-sfdevnl 
_______________________________________________
mailing list linux1394-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux1394-devel

^ permalink raw reply

* [patch] Re: genetlink misinterprets NEW as GET
From: Jan Engelhardt @ 2011-01-07 13:15 UTC (permalink / raw)
  To: David S. Miller
  Cc: Pablo Neira Aysuo, Ben Pfaff, Netfilter Developer Mailing List,
	Linux Networking Developer Mailing List
In-Reply-To: <4D266CE5.4000309@netfilter.org>

On Friday 2011-01-07 02:31, Pablo Neira Ayuso wrote:
>>> On 04/01/11 03:14, Jan Engelhardt wrote:
>>>> 	/* Modifiers to GET request */
>>>> 	#define NLM_F_ROOT      0x100
>>>> 	#define NLM_F_MATCH     0x200
>>>> 	#define NLM_F_ATOMIC    0x400
>>>> 	#define NLM_F_DUMP      (NLM_F_ROOT|NLM_F_MATCH)
>> [...]
>>>> [N.B.: I am also wondering whether
>>>> 	(nlh->nlmsg_flags & NLM_F_DUMP) == NLM_F_DUMP
>>>> may have been desired, because NLM_F_DUMP is composed of two bits.]
>>>
>>> Someone may include NLM_F_ATOMIC to a dump operation, in that case the
>>> checking that you propose is not valid.
>> 
>> Are you saying that NLM_F_MATCH and NLM_F_ATOMIC are mutually
>> exclusive, and that NLM_F_ROOT|NLM_F_ATOMIC would also signal a
>> dump operation?  Otherwise the test that Jan proposes looks valid
>> to me.
>
>Indeed, Jan's test is fine to fix this. Please, send a patch to Davem asap.

Turns out genetlink isn't the only place where &NLM_F_DUMP is used 
without ==NLM_F_DUMP.
Thus I am adding it to other spots in net/ too.



parent c235848c5a76520b90cf31bfbcc17720b24745a2 (v2.6.37-rc1-230-gc235848)
commit eaab9042b29931730d6785bb3f27b174fb2f5518
Author: Jan Engelhardt <jengelh@medozas.de>
Date:   Fri Jan 7 13:53:49 2011 +0100

netlink: test for all flags of the NLM_F_DUMP composite

Due to NLM_F_DUMP is composed of two bits, NLM_F_ROOT | NLM_F_MATCH,
when doing "if (x & NLM_F_DUMP)", it tests for _either_ of the bits
being set. Because NLM_F_MATCH's value overlaps with NLM_F_EXCL,
non-dump requests with NLM_F_EXCL set are mistaken as dump requests.

Substitute the condition to test for _all_ bits being set.

Signed-off-by: Jan Engelhardt <jengelh@medozas.de>
---
 net/core/rtnetlink.c                 |    2 +-
 net/ipv4/inet_diag.c                 |    2 +-
 net/netfilter/nf_conntrack_netlink.c |    4 ++--
 net/netlink/genetlink.c              |    2 +-
 net/xfrm/xfrm_user.c                 |    2 +-
 5 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index 8121268..14dcb43 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -1657,7 +1657,7 @@ static int rtnetlink_rcv_msg(struct sk_buff *skb, struct nlmsghdr *nlh)
 	if (kind != 2 && security_netlink_recv(skb, CAP_NET_ADMIN))
 		return -EPERM;
 
-	if (kind == 2 && nlh->nlmsg_flags&NLM_F_DUMP) {
+	if (kind == 2 && (nlh->nlmsg_flags & NLM_F_DUMP) == NLM_F_DUMP) {
 		struct sock *rtnl;
 		rtnl_dumpit_func dumpit;
 
diff --git a/net/ipv4/inet_diag.c b/net/ipv4/inet_diag.c
index 2ada171..2746c1f 100644
--- a/net/ipv4/inet_diag.c
+++ b/net/ipv4/inet_diag.c
@@ -858,7 +858,7 @@ static int inet_diag_rcv_msg(struct sk_buff *skb, struct nlmsghdr *nlh)
 	    nlmsg_len(nlh) < hdrlen)
 		return -EINVAL;
 
-	if (nlh->nlmsg_flags & NLM_F_DUMP) {
+	if ((nlh->nlmsg_flags & NLM_F_DUMP) == NLM_F_DUMP) {
 		if (nlmsg_attrlen(nlh, hdrlen)) {
 			struct nlattr *attr;
 
diff --git a/net/netfilter/nf_conntrack_netlink.c b/net/netfilter/nf_conntrack_netlink.c
index 7f59be8..edc737e 100644
--- a/net/netfilter/nf_conntrack_netlink.c
+++ b/net/netfilter/nf_conntrack_netlink.c
@@ -925,7 +925,7 @@ ctnetlink_get_conntrack(struct sock *ctnl, struct sk_buff *skb,
 	u16 zone;
 	int err;
 
-	if (nlh->nlmsg_flags & NLM_F_DUMP)
+	if ((nlh->nlmsg_flags & NLM_F_DUMP) == NLM_F_DUMP)
 		return netlink_dump_start(ctnl, skb, nlh, ctnetlink_dump_table,
 					  ctnetlink_done);
 
@@ -1788,7 +1788,7 @@ ctnetlink_get_expect(struct sock *ctnl, struct sk_buff *skb,
 	u16 zone;
 	int err;
 
-	if (nlh->nlmsg_flags & NLM_F_DUMP) {
+	if ((nlh->nlmsg_flags & NLM_F_DUMP) == NLM_F_DUMP) {
 		return netlink_dump_start(ctnl, skb, nlh,
 					  ctnetlink_exp_dump_table,
 					  ctnetlink_exp_done);
diff --git a/net/netlink/genetlink.c b/net/netlink/genetlink.c
index 1781d99..f83cb37 100644
--- a/net/netlink/genetlink.c
+++ b/net/netlink/genetlink.c
@@ -519,7 +519,7 @@ static int genl_rcv_msg(struct sk_buff *skb, struct nlmsghdr *nlh)
 	    security_netlink_recv(skb, CAP_NET_ADMIN))
 		return -EPERM;
 
-	if (nlh->nlmsg_flags & NLM_F_DUMP) {
+	if ((nlh->nlmsg_flags & NLM_F_DUMP) == NLM_F_DUMP) {
 		if (ops->dumpit == NULL)
 			return -EOPNOTSUPP;
 
diff --git a/net/xfrm/xfrm_user.c b/net/xfrm/xfrm_user.c
index 8bae6b2..6770895 100644
--- a/net/xfrm/xfrm_user.c
+++ b/net/xfrm/xfrm_user.c
@@ -2174,7 +2174,7 @@ static int xfrm_user_rcv_msg(struct sk_buff *skb, struct nlmsghdr *nlh)
 
 	if ((type == (XFRM_MSG_GETSA - XFRM_MSG_BASE) ||
 	     type == (XFRM_MSG_GETPOLICY - XFRM_MSG_BASE)) &&
-	    (nlh->nlmsg_flags & NLM_F_DUMP)) {
+	    (nlh->nlmsg_flags & NLM_F_DUMP) == NLM_F_DUMP) {
 		if (link->dump == NULL)
 			return -EINVAL;
 
-- 
# Created with git-export-patch

^ permalink raw reply related

* Re: [PATCH 3/5] NET: IPV4: ARP: allow to invalidate specific ARP entries
From: Maxim Levitsky @ 2011-01-07 13:15 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: linux1394-devel, Stefan Richter, netdev, David S. Miller,
	Alexey Kuznetsov, James Morris, Patrick McHardy
In-Reply-To: <1294405062.3306.11.camel@edumazet-laptop>

On Fri, 2011-01-07 at 13:57 +0100, Eric Dumazet wrote:
> Le vendredi 07 janvier 2011 à 14:47 +0200, Maxim Levitsky a écrit :
> > On Mon, 2010-11-29 at 04:09 +0200, Maxim Levitsky wrote:
> > > IPv4 over firewire needs to be able to remove ARP entries
> > > from the ARP cache that belong to nodes that are removed, because
> > > IPv4 over firewire uses ARP packets for private information
> > > about nodes.
> > > 
> > > This information becomes invalid as soon as node drops
> > > off the bus and when it reconnects, its only possible
> > > to start takling to is after it responded to an ARP packet.
> > > But ARP cache prevents such packets from being sent.
> > > 
> > > CC: netdev@vger.kernel.org
> > > CC: "David S. Miller" <davem@davemloft.net>
> > > CC: Alexey Kuznetsov <kuznet@ms2.inr.ac.ru>
> > > CC: James Morris <jmorris@namei.org>
> > > CC: Patrick McHardy <kaber@trash.net>
> > 
> > Anybody?
> > 
> > Best regards,
> > 	Maxim Levitsky
> > 
> > > 
> > > 
> > > Signed-off-by: Maxim Levitsky <maximlevitsky@gmail.com>
> > > ---
> > >  include/net/arp.h |    1 +
> > >  net/ipv4/arp.c    |   29 ++++++++++++++++++-----------
> > >  2 files changed, 19 insertions(+), 11 deletions(-)
> > > 
> > > diff --git a/include/net/arp.h b/include/net/arp.h
> > > index f4cf6ce..91f0568 100644
> > > --- a/include/net/arp.h
> > > +++ b/include/net/arp.h
> > > @@ -25,5 +25,6 @@ extern struct sk_buff *arp_create(int type, int ptype, __be32 dest_ip,
> > >  				  const unsigned char *src_hw,
> > >  				  const unsigned char *target_hw);
> > >  extern void arp_xmit(struct sk_buff *skb);
> > > +int arp_invalidate(struct net_device *dev, __be32 ip);
> > >  
> > >  #endif	/* _ARP_H */
> > > diff --git a/net/ipv4/arp.c b/net/ipv4/arp.c
> > > index d8e540c..35b1272 100644
> > > --- a/net/ipv4/arp.c
> > > +++ b/net/ipv4/arp.c
> > > @@ -1142,6 +1142,23 @@ static int arp_req_get(struct arpreq *r, struct net_device *dev)
> > >  	return err;
> > >  }
> > >  
> > > +int arp_invalidate(struct net_device *dev, __be32 ip)
> > > +{
> > > +	int err = -ENXIO;
> > > +	struct neighbour *neigh = neigh_lookup(&arp_tbl, &ip, dev);
> > > +
> > > +	if (neigh) {
> > > +		if (neigh->nud_state & ~NUD_NOARP)
> > > +			err = neigh_update(neigh, NULL, NUD_FAILED,
> > > +					   NEIGH_UPDATE_F_OVERRIDE|
> > > +					   NEIGH_UPDATE_F_ADMIN);
> > > +		neigh_release(neigh);
> > > +	}
> > > +
> > > +	return err;
> > > +}
> > > +EXPORT_SYMBOL(arp_invalidate);
> > > +
> > >  static int arp_req_delete_public(struct net *net, struct arpreq *r,
> > >  		struct net_device *dev)
> > >  {
> > > @@ -1162,7 +1179,6 @@ static int arp_req_delete(struct net *net, struct arpreq *r,
> > >  {
> > >  	int err;
> > >  	__be32 ip;
> > > -	struct neighbour *neigh;
> > >  
> > >  	if (r->arp_flags & ATF_PUBL)
> > >  		return arp_req_delete_public(net, r, dev);
> > > @@ -1180,16 +1196,7 @@ static int arp_req_delete(struct net *net, struct arpreq *r,
> > >  		if (!dev)
> > >  			return -EINVAL;
> > >  	}
> > > -	err = -ENXIO;
> > > -	neigh = neigh_lookup(&arp_tbl, &ip, dev);
> > > -	if (neigh) {
> > > -		if (neigh->nud_state & ~NUD_NOARP)
> > > -			err = neigh_update(neigh, NULL, NUD_FAILED,
> > > -					   NEIGH_UPDATE_F_OVERRIDE|
> > > -					   NEIGH_UPDATE_F_ADMIN);
> > > -		neigh_release(neigh);
> > > -	}
> > > -	return err;
> > > +	return arp_invalidate(dev, ip);
> > >  }
> > >  
> > >  /*
> > 
> 
> Hi Maxim
> 
> You were supposed to respin your patch after my commit :
> 
> (941666c2e3e0f9f6a1 net: RCU conversion of dev_getbyhwaddr() and
> arp_ioctl())
> 
> Thanks


Will do very soon.

Best regards,
	Maxim Levitsky


^ permalink raw reply

* Re: POLLPRI/poll() behavior change since 2.6.31
From: Eric Dumazet @ 2011-01-07 13:31 UTC (permalink / raw)
  To: Davide Libenzi; +Cc: Leonardo Chiquitto, netdev, David S. Miller
In-Reply-To: <alpine.DEB.2.00.1101061429080.4928@davide-lnx1>

Le jeudi 06 janvier 2011 à 14:40 -0800, Davide Libenzi a écrit :
> On Thu, 6 Jan 2011, Eric Dumazet wrote:
> 
> > Hmm, this is because 	sock_def_readable() uses :
> > 
> > wake_up_interruptible_sync_poll(&wq->wait, POLLIN | POLLRDNORM |
> > POLLRDBAND);
> > 
> > So POLLPRI bit is not signaled. 
> > 
> > I would just add POLLPRI flag in sock_def_readable()
> > 
> > (Alternatively, define a tcp_def_readable() function to pass POLLPRI
> > only if TCP_URG is set, but is it worth the pain for a seldom used
> > feature ?)
> 
> It would be kinda cleaner though, /me thinks.
> 

Yep, we'll do this in net-next-2.6 for 2.6.39 :)

Thanks !



^ permalink raw reply

* Re: [patch] Re: genetlink misinterprets NEW as GET
From: Pablo Neira Ayuso @ 2011-01-07 13:33 UTC (permalink / raw)
  To: Jan Engelhardt
  Cc: David S. Miller, Ben Pfaff, Netfilter Developer Mailing List,
	Linux Networking Developer Mailing List
In-Reply-To: <alpine.LNX.2.01.1101071357420.27454@obet.zrqbmnf.qr>

On 07/01/11 14:15, Jan Engelhardt wrote:
> On Friday 2011-01-07 02:31, Pablo Neira Ayuso wrote:
>>>> On 04/01/11 03:14, Jan Engelhardt wrote:
>>>>> 	/* Modifiers to GET request */
>>>>> 	#define NLM_F_ROOT      0x100
>>>>> 	#define NLM_F_MATCH     0x200
>>>>> 	#define NLM_F_ATOMIC    0x400
>>>>> 	#define NLM_F_DUMP      (NLM_F_ROOT|NLM_F_MATCH)
>>> [...]
>>>>> [N.B.: I am also wondering whether
>>>>> 	(nlh->nlmsg_flags & NLM_F_DUMP) == NLM_F_DUMP
>>>>> may have been desired, because NLM_F_DUMP is composed of two bits.]
>>>>
>>>> Someone may include NLM_F_ATOMIC to a dump operation, in that case the
>>>> checking that you propose is not valid.
>>>
>>> Are you saying that NLM_F_MATCH and NLM_F_ATOMIC are mutually
>>> exclusive, and that NLM_F_ROOT|NLM_F_ATOMIC would also signal a
>>> dump operation?  Otherwise the test that Jan proposes looks valid
>>> to me.
>>
>> Indeed, Jan's test is fine to fix this. Please, send a patch to Davem asap.
> 
> Turns out genetlink isn't the only place where &NLM_F_DUMP is used 
> without ==NLM_F_DUMP.
> Thus I am adding it to other spots in net/ too.
> 
> 
> 
> parent c235848c5a76520b90cf31bfbcc17720b24745a2 (v2.6.37-rc1-230-gc235848)
> commit eaab9042b29931730d6785bb3f27b174fb2f5518
> Author: Jan Engelhardt <jengelh@medozas.de>
> Date:   Fri Jan 7 13:53:49 2011 +0100
> 
> netlink: test for all flags of the NLM_F_DUMP composite
> 
> Due to NLM_F_DUMP is composed of two bits, NLM_F_ROOT | NLM_F_MATCH,
> when doing "if (x & NLM_F_DUMP)", it tests for _either_ of the bits
> being set. Because NLM_F_MATCH's value overlaps with NLM_F_EXCL,
> non-dump requests with NLM_F_EXCL set are mistaken as dump requests.
> 
> Substitute the condition to test for _all_ bits being set.
> 
> Signed-off-by: Jan Engelhardt <jengelh@medozas.de>

Acked-by: Pablo Neira Ayuso <pablo@netfilter.org>

^ permalink raw reply

* Re: [PATCH] sh: sh_eth: Add support ethtool
From: Ben Hutchings @ 2011-01-07 14:35 UTC (permalink / raw)
  To: Nobuhiro Iwamatsu; +Cc: netdev, linux-sh, Yoshihiro Shimoda
In-Reply-To: <1294385126-3098-1-git-send-email-nobuhiro.iwamatsu.yj@renesas.com>

On Fri, 2011-01-07 at 16:25 +0900, Nobuhiro Iwamatsu wrote:
> This commit supports following functions.
>  - get_drvinfo
>  - get_settings
>  - set_settings
>  - nway_reset
>  - get_msglevel
>  - set_msglevel
>  - get_link
>  - get_strings
>  - get_ethtool_stats
>  - get_sset_count
> 
> About other function, the device does not support.
> 
> Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> Signed-off-by: Nobuhiro Iwamatsu <nobuhiro.iwamatsu.yj@renesas.com>
> ---
>  drivers/net/sh_eth.c |  190 ++++++++++++++++++++++++++++++++++++++++++++++----
>  1 files changed, 176 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/net/sh_eth.c b/drivers/net/sh_eth.c
> index 819c175..10493e8 100644
> --- a/drivers/net/sh_eth.c
> +++ b/drivers/net/sh_eth.c
> @@ -32,6 +32,7 @@
>  #include <linux/io.h>
>  #include <linux/pm_runtime.h>
>  #include <linux/slab.h>
> +#include <linux/ethtool.h>
>  #include <asm/cacheflush.h>
>  
>  #include "sh_eth.h"
> @@ -573,7 +574,7 @@ static int sh_eth_ring_init(struct net_device *ndev)
>  	}
>  
>  	/* Allocate all Rx descriptors. */
> -	rx_ringsize = sizeof(struct sh_eth_rxdesc) * RX_RING_SIZE;
> +	rx_ringsize = sizeof(struct sh_eth_rxdesc) *RX_RING_SIZE;

Please don't delete spaces just because checkpatch.pl is too stupid to
recognise a multiplication.

Also, don't mix formatting cleanup with actual feature changes.

[...]
> +static const char sh_eth_gstrings_stats[][ETH_GSTRING_LEN] = {
> +	"rx_packets", "tx_packets", "rx_bytes", "tx_bytes", "rx_errors",
> +	"tx_errors", "rx_dropped", "tx_dropped", "multicast", "collisions",
> +	"rx_length_errors", "rx_over_errors", "rx_crc_errors",
> +	"rx_frame_errors", "rx_fifo_errors", "rx_missed_errors",
> +	"tx_aborted_errors", "tx_carrier_errors", "tx_fifo_errors",
> +	"tx_heartbeat_errors", "tx_window_errors",
> +	/* device-specific stats */
> +	"rx_current", "tx_current",
> +	"rx_dirty", "tx_dirty",
> +};
> +#define SH_ETH_NET_STATS_LEN  21
> +#define SH_ETH_STATS_LEN  ARRAY_SIZE(sh_eth_gstrings_stats)
> +
> +static int sh_eth_get_sset_count(struct net_device *netdev, int sset)
> +{
> +	switch (sset) {
> +	case ETH_SS_STATS:
> +		return SH_ETH_STATS_LEN;
> +	default:
> +		return -EOPNOTSUPP;
> +	}
> +}
> +
> +static void sh_eth_get_ethtool_stats(struct net_device *ndev,
> +			struct ethtool_stats *stats, u64 *data)
> +{
> +	struct sh_eth_private *mdp = netdev_priv(ndev);
> +	int i;
> +
> +	for (i = 0; i < SH_ETH_NET_STATS_LEN; i++)
> +		data[i] = ((unsigned long *)&ndev->stats)[i];
[...]

There is no need to duplicate net_device_stats here.

Ben.

-- 
Ben Hutchings, Senior Software Engineer, Solarflare Communications
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] Madge Ambassador ATM Adapter driver: Always release_firmware() in ucode_init() and don't leak memory.
From: chas williams - CONTRACTOR @ 2011-01-07 15:02 UTC (permalink / raw)
  To: Jesper Juhl; +Cc: linux-atm-general, netdev, linux-kernel
In-Reply-To: <alpine.LNX.2.00.1101062203160.13988@swampdragon.chaosbits.net>

instead of duplicating the same section again and again, could you
write something like:

	errmsg = "no start record";
	goto fail;

	...

	errmsg = "record to long"
	goto fail;

	.... whatever ...

	return 0;

fail:
	release_firmware(fw)
	PRINTK(KERN_ERR, "Bad microcode data (%s)\n", errmsg);
	return -EINVAL;
}

On Thu, 6 Jan 2011 22:06:37 +0100 (CET)
Jesper Juhl <jj@chaosbits.net> wrote:

> 
> Failure to call release_firmware() will result in memory leak in 
> drivers/atm/ambassador.c::ucode_init().
> This patch makes sure we always call release_firmware() when needed, thus 
> removing the leak(s).
> 
> Signed-off-by: Jesper Juhl <jj@chaosbits.net>
> ---
>  ambassador.c |    5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
>   Compile tested only since I have no way to actually test this.
> 
> diff --git a/drivers/atm/ambassador.c b/drivers/atm/ambassador.c
> index ffe9b65..ab56539 100644
> --- a/drivers/atm/ambassador.c
> +++ b/drivers/atm/ambassador.c
> @@ -1927,7 +1927,7 @@ static int __devinit ucode_init (loader_block * lb, amb_dev * dev) {
>    unsigned long start_address;no start record
>    const struct ihex_binrec *rec;
>    int res;
> -  
> +
>    res = request_ihex_firmware(&fw, "atmsar11.fw", &dev->pci_dev->dev);
>    if (res) {
>      PRINTK (KERN_ERR, "Cannot load microcode data");
> @@ -1937,6 +1937,7 @@ static int __devinit ucode_init (loader_block * lb, amb_dev * dev) {
>    /* First record contains just the start address */
>    rec = (const struct ihex_binrec *)fw->data;
>    if (be16_to_cpu(rec->len) != sizeof(__be32) || be32_to_cpu(rec->addr)) {
> +    release_firmware(fw);
>      PRINTK (KERN_ERR, "Bad microcode data (no start record)");
>      return -EINVAL;
>    }
> @@ -1950,10 +1951,12 @@ static int __devinit ucode_init (loader_block * lb, amb_dev * dev) {
>      PRINTD (DBG_LOAD, "starting region (%x, %u)", be32_to_cpu(rec->addr),
>  	    be16_to_cpu(rec->len));
>      if (be16_to_cpu(rec->len) > 4 * MAX_TRANSFER_DATA) {
> +	    release_firmware(fw);
>  	    PRINTK (KERN_ERR, "Bad microcode data (record too long)");
>  	    return -EINVAL;
>      }
>      if (be16_to_cpu(rec->len) & 3) {
> +	    release_firmware(fw);
>  	    PRINTK (KERN_ERR, "Bad microcode data (odd number of bytes)");
>  	    return -EINVAL;
>      }
> 
> 
> 

^ permalink raw reply

* Re: [PATCH] net/r8169: Update the function of parsing firmware
From: Ben Hutchings @ 2011-01-07 15:17 UTC (permalink / raw)
  To: Hayes Wang; +Cc: romieu, netdev, linux-kernel
In-Reply-To: <1294393509-3492-1-git-send-email-hayeswang@realtek.com>

On Fri, 2011-01-07 at 17:45 +0800, Hayes Wang wrote:
> Update rtl_phy_write_fw function. The new function could
> parse the complex firmware which is used by RTL8111E and later.
> The new firmware may read data and do some operations, not just
> do writing only.
> 
> Signed-off-by: Hayes Wang <hayeswang@realtek.com>
> ---
>  drivers/net/r8169.c |  112 ++++++++++++++++++++++++++++++++++++++++++++-------
>  1 files changed, 97 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/net/r8169.c b/drivers/net/r8169.c
> index 27a7c20..2115424 100644
> --- a/drivers/net/r8169.c
> +++ b/drivers/net/r8169.c
[...] 
> -	while (i-- != 0) {
> -		u32 action = le32_to_cpu(*phytable);
> -		u32 data = action & 0x0000ffff;
> -		u32 reg = (action & 0x0fff0000) >> 16;
> +	predata = 0;
> +	count = 0;
> +
> +	for (index = 0; index < fw->size / sizeof(*phytable); ) {
> +		u32 action = le32_to_cpu(phytable[index]);
> +		u32 data = action & 0x0000FFFF;
> +		u32 regno = (action & 0x0FFF0000) >> 16;
> +
> +		if (!action)
> +			break;
>  
> -		switch(action & 0xf0000000) {
> +		switch(action & 0xF0000000) {
[...]
> +		case PHY_BJMPN:
> +			index -= regno;
> +			break;
[...]

I'm concerned that this is being extended from a firmware upload
interface to a quite general interpreter for PHY initialisation.  I
realise that this will make it easier to fix PHY firmware bugs in
future but it also allows you to accidentally introduce infinite loops.
The initialisation programs will obviously not be subject to the same
sort of review on netdev that new C code is.

> +		case PHY_DELAY_MS:
> +			mdelay(data);
> +			index++;
> +			break;

Why mdelay() and not msleep()?  This is not an atomic context.

> +		case PHY_READ_MAC_BYTE:
> +		case PHY_WRITE_MAC_BYTE:
> +		case PHY_WRITE_ERI_WORD:
>  		default:
>  			BUG();
>  		}
> +
> +		if (index < 0)
> +			BUG();
[...]

index is unsigned so it can't be < 0.  It looks like the loop condition
should catch an out-of-range index, but really the range-checking should
be done in the first loop.

Ben.

-- 
Ben Hutchings
We get into the habit of living before acquiring the habit of thinking.
                                                              - Albert Camus

^ permalink raw reply

* Re: via-velocity: corrupted mac, deadlock on link
From: Janne Karhunen @ 2011-01-07 15:31 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: netdev
In-Reply-To: <20110106125822.70a85e0a@nehalam>

On Thu, Jan 6, 2011 at 10:58 PM, Stephen Hemminger
<shemminger@vyatta.com> wrote:

> To debug, remove the udev rule that does renaming
>  cd /lib/udev
>  mkdir save; mv rules.d/75-persistent-net-generator-rules save
> and remove any rules
>  rm /etc/udev/70-persistent-net.rules
>
> Then after bootup you can debug problem.

Yeah.. before wasting anyones time with this, will have the
board replaced just in case. Looks suspicious enough already.
Thanks.


-- 
// Janne

^ permalink raw reply

* Re: [net-next 08/12] ixgb: convert to new VLAN model
From: Jesse Gross @ 2011-01-07 15:41 UTC (permalink / raw)
  To: jeffrey.t.kirsher; +Cc: davem, Emil Tantilov, netdev, gosp, bphilips
In-Reply-To: <1294360199-9860-9-git-send-email-jeffrey.t.kirsher@intel.com>

On Thu, Jan 6, 2011 at 7:29 PM,  <jeffrey.t.kirsher@intel.com> wrote:
> +static int ixgb_set_flags(struct net_device *netdev, u32 data)
> +{
> +       struct ixgb_adapter *adapter = netdev_priv(netdev);
> +       bool need_reset;
> +       int rc;
> +
> +       /*
> +        * TX vlan insertion does not work per HW design when Rx stripping is
> +        * disabled.  Disable txvlan when rxvlan is off.
> +        */
> +       if ((data & ETH_FLAG_RXVLAN) != (netdev->features & NETIF_F_HW_VLAN_RX))
> +               data ^= ETH_FLAG_TXVLAN;

Does this really do the right thing?  If the RX vlan setting is
changed, it will do the opposite of what the user requested for TX
vlan?

So if I start with both on (the default) and turn them both off in one
command (a valid setting), I will get RX off and TX on (an invalid
setting).

Why not:

if (!(data & ETH_FLAG_RXVLAN))
        data &= ~ETH_FLAG_TXVLAN;

^ permalink raw reply

* Re: [PATCH V8 09/13] posix clocks: introduce dynamic clocks
From: Richard Cochran @ 2011-01-07 16:41 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: linux-kernel, linux-api, netdev, Alan Cox, Christoph Lameter,
	David Miller, John Stultz, Krzysztof Halasa, Peter Zijlstra,
	Rodolfo Giometti, Thomas Gleixner
In-Reply-To: <201101062056.22423.arnd@arndb.de>

On Thu, Jan 06, 2011 at 08:56:22PM +0100, Arnd Bergmann wrote:
> It should be just a trivial change and just affect how easy it is for
> other people to understand the code if they are already familiar
> with other kernel code.
 
Okay, I'll get right on it.

> Overall, your series looks really good now, it would be nice if this
> could still make it into 2.6.38.

Yes, that would be great.

Thanks,
Richard

^ 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