Netdev List
 help / color / mirror / Atom feed
* RE: hv_netvsc: WARNING in softirq.c
From: Haiyang Zhang @ 2013-04-05 14:13 UTC (permalink / raw)
  To: Richard Genoud
  Cc: KY Srinivasan, devel@linuxdriverproject.org,
	netdev@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <CACQ1gAgh7QX=cDYsrw9J5qYNKYpgQ841QijA145TDO_Ae5EsQQ@mail.gmail.com>



> -----Original Message-----
> From: Richard Genoud [mailto:richard.genoud@gmail.com]
> Sent: Friday, April 05, 2013 7:00 AM
> To: Haiyang Zhang
> Cc: KY Srinivasan; devel@linuxdriverproject.org; netdev@vger.kernel.org;
> linux-kernel@vger.kernel.org
> Subject: Re: hv_netvsc: WARNING in softirq.c
> 
> Ok, it has been a little bit more than 2 weeks now, and I did not see this
> warning any more, just the
> "hv_vmbus: child device vmbus_0_8 unregistered" sometimes.
> 
> so it seems to be fixed !
> 
> Thanks !
> 
> 
> Reported-by: Richard Genoud <richard.genoud@gmail.com>
> Tested-by: Richard Genoud <richard.genoud@gmail.com>
> 

Thank you! I will submit the patch soon.

- Haiyang

^ permalink raw reply

* Re: [PATCH 1/5 v2] mv643xx_eth: add Device Tree bindings
From: Florian Fainelli @ 2013-04-05 14:23 UTC (permalink / raw)
  To: Sebastian Hesselbarth
  Cc: thomas.petazzoni, moinejf, Simon Baatz, andrew, netdev,
	devicetree-discuss, rob.herring, grant.likely, jogo,
	linux-arm-kernel, jm, davem, buytenh, jason
In-Reply-To: <CABJ1b_TdzE_Q9qLsdSGZy8byFpCTtVBUy5FtxiZAx7QqgG-rVg@mail.gmail.com>

Hello Sebastian,

Le 04/05/13 15:58, Sebastian Hesselbarth a écrit :
> On Fri, Apr 5, 2013 at 11:56 AM, Florian Fainelli <florian@openwrt.org> wrote:
>> [snip]
>
> Florian,
>
> took me a while to try you patches out on Dove but now I fixed all
> issues. I will
> comment on all related patches but first I want to comment here.
>
> One general note for Dove related patches: You didn't remove the registration of
> ge platform_device from mach-dove/board-dt.c. That will lead to double
> registration
> of mdio and mv643xx_eth/shared, so you'll never be sure if DT or non-DT code is
> executed. I haven't checked mach-kirkwood/board-dt.c or orion5x code.

This was intentional, this patchset is just preparatory in the sense 
that it does no conversion of the existing users of the mv643xx_eth 
platform driver over DT (have some patches to that though). I wanted to 
resume the discussion on these bindings first, then proceed with the 
conversion.

>
>>>>          if (!mv643xx_eth_version_printed++)
>>>>                  pr_notice("MV-643xx 10/100/1000 ethernet driver version
>>>> %s\n",
>>>
>>>
>>> This is not related to your change, but there is a problem in this
>>> function that has already been discussed in the past if I remember
>>> correctly: The respective clock needs to be enabled here (at least
>>> on Kirkwood), since accesses to the hardware are done below.
>>> Enabling the clock only in mv643xx_eth_probe() is too late.
>>>
>>> As said, this is not a problem introduced by your changes (and which
>>> is currently circumvented by enabling the respective clocks in
>>> kirkwood_legacy_clk_init() and kirkwood_ge0x_init()), but we might
>>> want to fix this now to get rid of unconditionally enabling the GE
>>> clocks in the DT case.
>>
>>
>> I think there may have been some confusion between the "ethernet-group"
>> clock and the actual Ethernet port inside the "ethernet-group". The
>> mv643xx_eth driver assumes we have a per-port clock gating scheme, while I
>> think we have a per "ethernet-group" clock gating scheme instead. Like you
>> said, I think this should be addressed separately.
>
> IMHO, there should be a clocks property where ever you try to access registers,
> i.e. in all three "parts" mv643xx_eth_shared (group), mv643xx_eth
> (port) and mdio.
> Since port depends on shared it would be ok to have it per group but that may
> collide with other SoCs than Dove/Kirkwood that have per port clocks.

Ok, which means that we should also teach mv643xx_eth_shared_probe() 
about it, as well as the orion-mdio driver. I don't have any particular 
objections since it should just make things safer with respect to clocking.

>
> Is that separation (group/port) really required for any SoC?

Probably not, it was not clear when I looked at mv78xx0 if it uses two 
ports per group or 4 groups and 1 port. Anyway, since we are re-using 
the existing Device Tree binding definition and that the hardware 
present itself as ethernet groups and ports, I don't see any problem 
with keeping that difference since it allows for fine-grained 
representation of the hardware.

>
>>
>> [snip]
>>>
>>> You don't change the clk initialization here:
>>>
>>> #if defined(CONFIG_HAVE_CLK)
>>>          mp->clk = clk_get(&pdev->dev, (pdev->id ? "1" : "0"));
>>>          if (!IS_ERR(mp->clk)) {
>>>                  clk_prepare_enable(mp->clk);
>>>                  mp->t_clk = clk_get_rate(mp->clk);
>>>          }
>>> #endif
>>>
>>> Which, if I understand correctly, works in the DT case because you
>>> assign "clock-names" to the clocks in the DTS. However, I wonder
>>> whether this works for any but the first Ethernet device.
>
> Yes, it does. Assigned clocks from clocks property get a clock alias for
> that device name (node name). Using anything else than NULL here is
> IMHO just wrong. We should rather provide proper clock aliases for non-DT case.
>
>>> In the old platform device setup, the pdev->id was set when
>>> initialiazing the platform_device structure in common.c.  Where is
>>> this done in the DT case?
>>
>> Looks like you are right, in the DT case, I assume that we should lookup the
>> clock using NULL instead of "1" or "0" so we match any clock instead of a
>> specific one.
>
> Yes.
>
>> [snip]
>>>
>>>
>>> In phy_scan(), the phy is searched like this:
>>>
>>>                  snprintf(phy_id, sizeof(phy_id), PHY_ID_FMT,
>>>                                  "orion-mdio-mii", addr);
>>>
>>>                  phydev = phy_connect(mp->dev, phy_id,
>>> mv643xx_eth_adjust_link,
>>>                                  PHY_INTERFACE_MODE_GMII);
>>>
>>> But "orion-mdio-mii:xx" is the name of the PHY if MDIO is setup via a
>>> platform_device. I could not get this to work if the MDIO device is
>>> setup via DT. Am I doing something wrong?
>>
>> I just missed updating this part of the code to probe for PHYs. The board I
>> tested with uses a "PHY_NONE" configuration. I will add the missing bits for
>> of_phy_connect() to be called here.
>
> I don't think that the ethernet controller should probe the PHY's on mdio-bus
> at all. At least not for DT enabled platforms. I had a look at DT and non-DT
> mdio-bus sources, and realized that there is a bus scan for non-DT only.
> of_mdiobus_register requires you to set (and know) the PHY address.

One reason the Ethernet controller could do the probing is in the case 
we need to apply quirks (e.g: using phydev->flags) for instance. This 
can be done even after the MDIO bus driver did probe PHY devices though.

>
> I prepared a patch for of_mdio_register that will allow you to probe mdio and
> assign phy addresses to each node found. Currently, the heuristic for probing
> is: assign each phy node the next probed phy_addr starting with 0. But that
> will not allow to e.g. set some PHY addresses and probe the rest.

Ok, we just need to make sure that this does not break any specific use 
case, I don't think it does, since it seems to be more accurate or 
equivalent to Ethernet driver doing the probing.

>
> We had a similar discussion whether to probe or not for DT nodes, and I guess
> there also will be some discussion about the above patch. OTOH we could just
> (again) ask users of every kirkwood/orion5x/dove board to tell their
> phy addresses
> and fail to probe the phy for new boards...
>
> I will prepare a proper patch soon and post it on the corresponding lists.

Cool, thanks!

>
>>> Additionally, in phy_scan() there is this:
>>>
>>>          if (phy_addr == MV643XX_ETH_PHY_ADDR_DEFAULT) {
>>>                  start = phy_addr_get(mp) & 0x1f;
>>>                  num = 32;
>>>          } else {
>>>          ...
>>>
>>> MV643XX_ETH_PHY_ADDR_DEFAULT is defined as 0. However, many Kirkwood
>>> devices use "MV643XX_ETH_PHY_ADDR(0)".  If the module probe is
>>> deferred in mv643xx_eth because the MDIO driver is not yet loaded,
>>> all 32 PHY addresses are scanned without success.  This is not needed
>>> and clutters the log.
>>
>>
>> Ok, I am not sure how we can circumvent the log cluttering that happens,
>> what would be your suggestion?
>
> My suggestion is to change MV643XX_ETH_PHY_ADDR_DEFAULT from a valid
> phy address (0)
> to something invalid (32). I understand that using 0 helps if you
> don't want to set it in mv643xx's platform_data
> but it is always difficult to rely on that if 0 is a valid number.
>
> Changing the above to 32 should just work because most (all?) boards
> using phy_scan should also
> already use MV643XX_ETH_PHY_ADDR_DEFAULT. I also suggest to rename
> current define to a
> better name, e.g. MV643XX_ETH_PHY_ADDR_AUTOSCAN.

Sounds good to me.
--
Florian

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: be2net: GRO for non-inet protocols
From: Eric Dumazet @ 2013-04-05 15:28 UTC (permalink / raw)
  To: Erik Hugne; +Cc: sathya.perla, subbu.seetharaman, ajit.khaparde, netdev
In-Reply-To: <20130405132007.GF7551@eerihug-hybrid.ki.sw.ericsson.se>

On Fri, 2013-04-05 at 15:20 +0200, Erik Hugne wrote:
> I'm adding GRO support for the TIPC protocol and
> tried to test it on a pair of HP G7 blades with 
> Emulex Corporation OneConnect 10Gb NIC (be3) (rev 01) NICs.
> 
> However, my GRO callback is never invoked on this setup, and 
> i suspect the be2net driver is to blame.
> I had a brief look at the driver, first suspecting that the
> do_gro check only passed for inet protocols (tcpf must be set 
> in the be_rx_compl_info struct).
> However, removing this check did not change the behavior.

Try following instead :

diff --git a/drivers/net/ethernet/emulex/benet/be_main.c b/drivers/net/ethernet/emulex/benet/be_main.c
index 536afa2..1b9e467 100644
--- a/drivers/net/ethernet/emulex/benet/be_main.c
+++ b/drivers/net/ethernet/emulex/benet/be_main.c
@@ -1355,7 +1355,7 @@ static void skb_fill_rx_data(struct be_rx_obj *rxo, struct sk_buff *skb,
 }
 
 /* Process the RX completion indicated by rxcp when GRO is disabled */
-static void be_rx_compl_process(struct be_rx_obj *rxo,
+static void be_rx_compl_process(struct be_rx_obj *rxo, struct napi_struct *napi,
 				struct be_rx_compl_info *rxcp)
 {
 	struct be_adapter *adapter = rxo->adapter;
@@ -1385,7 +1385,7 @@ static void be_rx_compl_process(struct be_rx_obj *rxo,
 	if (rxcp->vlanf)
 		__vlan_hwaccel_put_tag(skb, rxcp->vlan_tag);
 
-	netif_receive_skb(skb);
+	napi_gro_receive(&napi, skb);
 }
 
 /* Process the RX completion indicated by rxcp when GRO is enabled */
@@ -2113,7 +2113,7 @@ static int be_process_rx(struct be_rx_obj *rxo, struct napi_struct *napi,
 		if (do_gro(rxcp))
 			be_rx_compl_process_gro(rxo, napi, rxcp);
 		else
-			be_rx_compl_process(rxo, rxcp);
+			be_rx_compl_process(rxo, napi, rxcp);
 loop_continue:
 		be_rx_stats_update(rxo, rxcp);
 	}

^ permalink raw reply related

* Re: be2net: GRO for non-inet protocols
From: Eric Dumazet @ 2013-04-05 15:31 UTC (permalink / raw)
  To: Erik Hugne; +Cc: sathya.perla, subbu.seetharaman, ajit.khaparde, netdev
In-Reply-To: <1365175702.3405.2.camel@edumazet-glaptop>

On Fri, 2013-04-05 at 08:28 -0700, Eric Dumazet wrote:

>  /* Process the RX completion indicated by rxcp when GRO is disabled */
> -static void be_rx_compl_process(struct be_rx_obj *rxo,
> +static void be_rx_compl_process(struct be_rx_obj *rxo, struct napi_struct *napi,
>  				struct be_rx_compl_info *rxcp)
>  {
>  	struct be_adapter *adapter = rxo->adapter;
> @@ -1385,7 +1385,7 @@ static void be_rx_compl_process(struct be_rx_obj *rxo,
>  	if (rxcp->vlanf)
>  		__vlan_hwaccel_put_tag(skb, rxcp->vlan_tag);
>  
> -	netif_receive_skb(skb);
> +	napi_gro_receive(&napi, skb);

That would be : napi_gro_receive(napi, skb);

^ permalink raw reply

* Re: [Linux-zigbee-devel] [PATCH 2/2] at86rf230: change irq handling to prevent lockups with edge type irq
From: Sascha Herrmann @ 2013-04-05 15:59 UTC (permalink / raw)
  To: Werner Almesberger, netdev, linux-zigbee-devel
In-Reply-To: <20130405105158.GD29789@ws>




>I wrote:
>> To achieve perfection, at86rf230_probe could try all four
>> possible trigger modes, pick one the platform supports, and
>> set TRX_CTRL_1.IRQ_POLARITY accordingly.
>
>Thinking of it, probing by trying request_irq has an unpleasant ring
>to it. Perhaps a better way would be to leave this decision to the
>platform code and do one of these:
>
>1) pass irqflags and the polarity in the platform data, or
>
>2) pass irqflags and extract the polarity from the irqflags, or
>
>3) set up the trigger mode outside the driver and pass only the
>   polarity,
>
>where 1) with (irqflags & IRQF_TRIGGER_MASK) == 0 includes
>case 3).

The change with this patch is mostly about the trigger type of the interrupt. The trigger level isn't configurable in the rf230, as far as I understand the datasheet. I posted a version of this patch with the option to configure the interrupt type (edge or level) for discussion on the linux-zigbee list some time ago. The result tended toward hardcoding the interrupt type in the driver [1]. My assumption is, that every platform would support edge type interrupts.

If it is preferred to have the interrupt type configurable I now would prefer to implement seperate isr and irqwork functions for each irq type. I fear the sollution to read the interrupt status register in a loop (as suggested in your earlier message) would leave chances for race conditions or spurious interrupts, depending on wheter interrupts are enabled before or after reading the status register. 

Hard coding the interrupt type would have the pro of keeping the driver complexity low. Surely for this option, the assumption that (at least nearly) every platform supports edge type interrupts should hold.

[1] http://www.mail-archive.com/linux-zigbee-devel@lists.sourceforge.net/msg01385.html

Thanks,
Sascha

^ permalink raw reply

* Re: [PATCH iproute2 2/2] ip: ipv6: add tokenized interface identifier support
From: Stephen Hemminger @ 2013-04-05 16:39 UTC (permalink / raw)
  To: Daniel Borkmann; +Cc: davem, netdev, Hannes Frederic Sowa, YOSHIFUJI Hideaki
In-Reply-To: <1365086258-4512-3-git-send-email-dborkman@redhat.com>

On Thu,  4 Apr 2013 16:37:38 +0200
Daniel Borkmann <dborkman@redhat.com> wrote:

> This is experimental support for tokenized IIDs, that enable
> administrators to assign well-known host-part addresses to nodes
> whilst still obtaining global network prefix from Router
> Advertisements. It is currently in IETF RFC draft status [1].
> 
> Example commands with iproute2:
> 
> Setting a device token:
>   # ip token set ::1a:2b:3c:4d/64 dev eth1
> 
> Getting a device token:
>   # ip token get dev eth1
>   token ::1a:2b:3c:4d dev eth1
> 
> Listing all tokens:
>   # ip token list  (or: ip token)
>   token :: dev eth0
>   token ::1a:2b:3c:4d dev eth1
> 
>  [1] http://tools.ietf.org/html/draft-chown-6man-tokenised-ipv6-identifiers-02
> 
> Cc: Hannes Frederic Sowa <hannes@stressinduktion.org>
> Cc: YOSHIFUJI Hideaki <yoshfuji@linux-ipv6.org>
> Signed-off-by: Daniel Borkmann <dborkman@redhat.com>

Deferred.
Please resubmit during 3.10 merge window.

^ permalink raw reply

* [PATCH 1/3] netfilter: ipv4: propagate routing errors from ip_route_me_harder()
From: Patrick McHardy @ 2013-04-05 16:41 UTC (permalink / raw)
  To: pablo; +Cc: netfilter-devel, netdev
In-Reply-To: <1365180072-6182-1-git-send-email-kaber@trash.net>

Propagate routing errors from ip_route_me_harder() when dropping a packet
using NF_DROP_ERR(). This makes userspace get the proper error instead of
EPERM for everything.

Example:

# ip r a unreachable default table 100
# ip ru add fwmark 0x1 lookup 100
# iptables -t mangle -A OUTPUT -d 8.8.8.8 -j MARK --set-mark 0x1

Current behaviour:

PING 8.8.8.8 (8.8.8.8) 56(84) bytes of data.
ping: sendmsg: Operation not permitted
ping: sendmsg: Operation not permitted
ping: sendmsg: Operation not permitted

New behaviour:

PING 8.8.8.8 (8.8.8.8) 56(84) bytes of data.
ping: sendmsg: Network is unreachable
ping: sendmsg: Network is unreachable
ping: sendmsg: Network is unreachable
ping: sendmsg: Network is unreachable

Signed-off-by: Patrick McHardy <kaber@trash.net>
---
 net/ipv4/netfilter.c                | 8 ++++----
 net/ipv4/netfilter/iptable_mangle.c | 9 ++++++---
 net/ipv4/netfilter/iptable_nat.c    | 6 ++++--
 3 files changed, 14 insertions(+), 9 deletions(-)

diff --git a/net/ipv4/netfilter.c b/net/ipv4/netfilter.c
index 4c0cf63..8b201e8 100644
--- a/net/ipv4/netfilter.c
+++ b/net/ipv4/netfilter.c
@@ -40,14 +40,14 @@ int ip_route_me_harder(struct sk_buff *skb, unsigned int addr_type)
 	fl4.flowi4_flags = flags;
 	rt = ip_route_output_key(net, &fl4);
 	if (IS_ERR(rt))
-		return -1;
+		return PTR_ERR(rt);
 
 	/* Drop old route. */
 	skb_dst_drop(skb);
 	skb_dst_set(skb, &rt->dst);
 
 	if (skb_dst(skb)->error)
-		return -1;
+		return skb_dst(skb)->error;
 
 #ifdef CONFIG_XFRM
 	if (!(IPCB(skb)->flags & IPSKB_XFRM_TRANSFORMED) &&
@@ -56,7 +56,7 @@ int ip_route_me_harder(struct sk_buff *skb, unsigned int addr_type)
 		skb_dst_set(skb, NULL);
 		dst = xfrm_lookup(net, dst, flowi4_to_flowi(&fl4), skb->sk, 0);
 		if (IS_ERR(dst))
-			return -1;
+			return PTR_ERR(dst);;
 		skb_dst_set(skb, dst);
 	}
 #endif
@@ -66,7 +66,7 @@ int ip_route_me_harder(struct sk_buff *skb, unsigned int addr_type)
 	if (skb_headroom(skb) < hh_len &&
 	    pskb_expand_head(skb, HH_DATA_ALIGN(hh_len - skb_headroom(skb)),
 				0, GFP_ATOMIC))
-		return -1;
+		return -ENOMEM;
 
 	return 0;
 }
diff --git a/net/ipv4/netfilter/iptable_mangle.c b/net/ipv4/netfilter/iptable_mangle.c
index 85d88f2..cba5658 100644
--- a/net/ipv4/netfilter/iptable_mangle.c
+++ b/net/ipv4/netfilter/iptable_mangle.c
@@ -44,6 +44,7 @@ ipt_mangle_out(struct sk_buff *skb, const struct net_device *out)
 	u_int8_t tos;
 	__be32 saddr, daddr;
 	u_int32_t mark;
+	int err;
 
 	/* root is playing with raw sockets. */
 	if (skb->len < sizeof(struct iphdr) ||
@@ -66,9 +67,11 @@ ipt_mangle_out(struct sk_buff *skb, const struct net_device *out)
 		if (iph->saddr != saddr ||
 		    iph->daddr != daddr ||
 		    skb->mark != mark ||
-		    iph->tos != tos)
-			if (ip_route_me_harder(skb, RTN_UNSPEC))
-				ret = NF_DROP;
+		    iph->tos != tos) {
+			err = ip_route_me_harder(skb, RTN_UNSPEC);
+			if (err < 0)
+				ret = NF_DROP_ERR(err);
+		}
 	}
 
 	return ret;
diff --git a/net/ipv4/netfilter/iptable_nat.c b/net/ipv4/netfilter/iptable_nat.c
index eeaff7e..c2937c8 100644
--- a/net/ipv4/netfilter/iptable_nat.c
+++ b/net/ipv4/netfilter/iptable_nat.c
@@ -213,6 +213,7 @@ nf_nat_ipv4_local_fn(unsigned int hooknum,
 	const struct nf_conn *ct;
 	enum ip_conntrack_info ctinfo;
 	unsigned int ret;
+	int err;
 
 	/* root is playing with raw sockets. */
 	if (skb->len < sizeof(struct iphdr) ||
@@ -226,8 +227,9 @@ nf_nat_ipv4_local_fn(unsigned int hooknum,
 
 		if (ct->tuplehash[dir].tuple.dst.u3.ip !=
 		    ct->tuplehash[!dir].tuple.src.u3.ip) {
-			if (ip_route_me_harder(skb, RTN_UNSPEC))
-				ret = NF_DROP;
+			err = ip_route_me_harder(skb, RTN_UNSPEC);
+			if (err < 0)
+				ret = NF_DROP_ERR(err);
 		}
 #ifdef CONFIG_XFRM
 		else if (!(IPCB(skb)->flags & IPSKB_XFRM_TRANSFORMED) &&
-- 
1.8.1.4


^ permalink raw reply related

* [PATCH 3/3] netfilter: nat: propagate errors from xfrm_me_harder()
From: Patrick McHardy @ 2013-04-05 16:41 UTC (permalink / raw)
  To: pablo; +Cc: netfilter-devel, netdev
In-Reply-To: <1365180072-6182-1-git-send-email-kaber@trash.net>

Propagate errors from ip_xfrm_me_harder() instead of returning EPERM in
all cases.

Signed-off-by: Patrick McHardy <kaber@trash.net>
---
 net/ipv4/netfilter/iptable_nat.c  | 17 +++++++++++------
 net/ipv6/netfilter/ip6table_nat.c | 17 +++++++++++------
 net/netfilter/nf_nat_core.c       |  9 +++++----
 3 files changed, 27 insertions(+), 16 deletions(-)

diff --git a/net/ipv4/netfilter/iptable_nat.c b/net/ipv4/netfilter/iptable_nat.c
index c2937c8..6383273 100644
--- a/net/ipv4/netfilter/iptable_nat.c
+++ b/net/ipv4/netfilter/iptable_nat.c
@@ -176,6 +176,7 @@ nf_nat_ipv4_out(unsigned int hooknum,
 #ifdef CONFIG_XFRM
 	const struct nf_conn *ct;
 	enum ip_conntrack_info ctinfo;
+	int err;
 #endif
 	unsigned int ret;
 
@@ -195,9 +196,11 @@ nf_nat_ipv4_out(unsigned int hooknum,
 		     ct->tuplehash[!dir].tuple.dst.u3.ip) ||
 		    (ct->tuplehash[dir].tuple.dst.protonum != IPPROTO_ICMP &&
 		     ct->tuplehash[dir].tuple.src.u.all !=
-		     ct->tuplehash[!dir].tuple.dst.u.all))
-			if (nf_xfrm_me_harder(skb, AF_INET) < 0)
-				ret = NF_DROP;
+		     ct->tuplehash[!dir].tuple.dst.u.all)) {
+			err = nf_xfrm_me_harder(skb, AF_INET);
+			if (err < 0)
+				ret = NF_DROP_ERR(err);
+		}
 	}
 #endif
 	return ret;
@@ -235,9 +238,11 @@ nf_nat_ipv4_local_fn(unsigned int hooknum,
 		else if (!(IPCB(skb)->flags & IPSKB_XFRM_TRANSFORMED) &&
 			 ct->tuplehash[dir].tuple.dst.protonum != IPPROTO_ICMP &&
 			 ct->tuplehash[dir].tuple.dst.u.all !=
-			 ct->tuplehash[!dir].tuple.src.u.all)
-			if (nf_xfrm_me_harder(skb, AF_INET) < 0)
-				ret = NF_DROP;
+			 ct->tuplehash[!dir].tuple.src.u.all) {
+			err = nf_xfrm_me_harder(skb, AF_INET);
+			if (err < 0)
+				ret = NF_DROP_ERR(err);
+		}
 #endif
 	}
 	return ret;
diff --git a/net/ipv6/netfilter/ip6table_nat.c b/net/ipv6/netfilter/ip6table_nat.c
index 97e2edd..6383f90 100644
--- a/net/ipv6/netfilter/ip6table_nat.c
+++ b/net/ipv6/netfilter/ip6table_nat.c
@@ -179,6 +179,7 @@ nf_nat_ipv6_out(unsigned int hooknum,
 #ifdef CONFIG_XFRM
 	const struct nf_conn *ct;
 	enum ip_conntrack_info ctinfo;
+	int err;
 #endif
 	unsigned int ret;
 
@@ -197,9 +198,11 @@ nf_nat_ipv6_out(unsigned int hooknum,
 				      &ct->tuplehash[!dir].tuple.dst.u3) ||
 		    (ct->tuplehash[dir].tuple.dst.protonum != IPPROTO_ICMPV6 &&
 		     ct->tuplehash[dir].tuple.src.u.all !=
-		     ct->tuplehash[!dir].tuple.dst.u.all))
-			if (nf_xfrm_me_harder(skb, AF_INET6) < 0)
-				ret = NF_DROP;
+		     ct->tuplehash[!dir].tuple.dst.u.all)) {
+			err = nf_xfrm_me_harder(skb, AF_INET6);
+			if (err < 0)
+				ret = NF_DROP_ERR(err);
+		}
 	}
 #endif
 	return ret;
@@ -236,9 +239,11 @@ nf_nat_ipv6_local_fn(unsigned int hooknum,
 		else if (!(IP6CB(skb)->flags & IP6SKB_XFRM_TRANSFORMED) &&
 			 ct->tuplehash[dir].tuple.dst.protonum != IPPROTO_ICMPV6 &&
 			 ct->tuplehash[dir].tuple.dst.u.all !=
-			 ct->tuplehash[!dir].tuple.src.u.all)
-			if (nf_xfrm_me_harder(skb, AF_INET6))
-				ret = NF_DROP;
+			 ct->tuplehash[!dir].tuple.src.u.all) {
+			err = nf_xfrm_me_harder(skb, AF_INET6);
+			if (err < 0)
+				ret = NF_DROP_ERR(err);
+		}
 #endif
 	}
 	return ret;
diff --git a/net/netfilter/nf_nat_core.c b/net/netfilter/nf_nat_core.c
index 8d5769c..346f871 100644
--- a/net/netfilter/nf_nat_core.c
+++ b/net/netfilter/nf_nat_core.c
@@ -87,9 +87,10 @@ int nf_xfrm_me_harder(struct sk_buff *skb, unsigned int family)
 	struct flowi fl;
 	unsigned int hh_len;
 	struct dst_entry *dst;
+	int err;
 
-	if (xfrm_decode_session(skb, &fl, family) < 0)
-		return -1;
+	err = xfrm_decode_session(skb, &fl, family);
+		return err;
 
 	dst = skb_dst(skb);
 	if (dst->xfrm)
@@ -98,7 +99,7 @@ int nf_xfrm_me_harder(struct sk_buff *skb, unsigned int family)
 
 	dst = xfrm_lookup(dev_net(dst->dev), dst, &fl, skb->sk, 0);
 	if (IS_ERR(dst))
-		return -1;
+		return PTR_ERR(dst);
 
 	skb_dst_drop(skb);
 	skb_dst_set(skb, dst);
@@ -107,7 +108,7 @@ int nf_xfrm_me_harder(struct sk_buff *skb, unsigned int family)
 	hh_len = skb_dst(skb)->dev->hard_header_len;
 	if (skb_headroom(skb) < hh_len &&
 	    pskb_expand_head(skb, hh_len - skb_headroom(skb), 0, GFP_ATOMIC))
-		return -1;
+		return -ENOMEM;
 	return 0;
 }
 EXPORT_SYMBOL(nf_xfrm_me_harder);
-- 
1.8.1.4


^ permalink raw reply related

* [PATCH 0/3] netfilter: propagate routing and XFRM errors
From: Patrick McHardy @ 2013-04-05 16:41 UTC (permalink / raw)
  To: pablo; +Cc: netfilter-devel, netdev

These patches fix a long standing annoyance: netfilter will return
EPERM for all errors during rerouting or XFRM policy lookups.

These patches change {ip,ip6}_route_me_harder() and nf_xfrm_me_harder()
to return errno codes and use NF_DROP_ERR() to propagate those back
to userspace.

Please apply.

^ permalink raw reply

* [PATCH 2/3] netfilter: ipv6: propagate routing errors from ip6_route_me_harder()
From: Patrick McHardy @ 2013-04-05 16:41 UTC (permalink / raw)
  To: pablo; +Cc: netfilter-devel, netdev
In-Reply-To: <1365180072-6182-1-git-send-email-kaber@trash.net>

Propagate routing errors from ip_route_me_harder() when dropping a packet
using NF_DROP_ERR(). This makes userspace get the proper error instead of
EPERM for everything.

# ip -6 r a unreachable default table 100
# ip -6 ru add fwmark 0x1 lookup 100
# ip6tables -t mangle -A OUTPUT -d 2001:4860:4860::8888 -j MARK --set-mark 0x1

Old behaviour:

PING 2001:4860:4860::8888(2001:4860:4860::8888) 56 data bytes
ping: sendmsg: Operation not permitted
ping: sendmsg: Operation not permitted
ping: sendmsg: Operation not permitted

New behaviour:

PING 2001:4860:4860::8888(2001:4860:4860::8888) 56 data bytes
ping: sendmsg: Network is unreachable
ping: sendmsg: Network is unreachable
ping: sendmsg: Network is unreachable

Signed-off-by: Patrick McHardy <kaber@trash.net>
---
 net/ipv6/netfilter.c                 | 6 +++---
 net/ipv6/netfilter/ip6table_mangle.c | 9 ++++++---
 net/ipv6/netfilter/ip6table_nat.c    | 6 ++++--
 3 files changed, 13 insertions(+), 8 deletions(-)

diff --git a/net/ipv6/netfilter.c b/net/ipv6/netfilter.c
index 429089c..fc5fbd7 100644
--- a/net/ipv6/netfilter.c
+++ b/net/ipv6/netfilter.c
@@ -29,7 +29,7 @@ int ip6_route_me_harder(struct sk_buff *skb)
 		IP6_INC_STATS(net, ip6_dst_idev(dst), IPSTATS_MIB_OUTNOROUTES);
 		LIMIT_NETDEBUG(KERN_DEBUG "ip6_route_me_harder: No more route.\n");
 		dst_release(dst);
-		return -EINVAL;
+		return dst->error;
 	}
 
 	/* Drop old route. */
@@ -43,7 +43,7 @@ int ip6_route_me_harder(struct sk_buff *skb)
 		skb_dst_set(skb, NULL);
 		dst = xfrm_lookup(net, dst, flowi6_to_flowi(&fl6), skb->sk, 0);
 		if (IS_ERR(dst))
-			return -1;
+			return PTR_ERR(dst);
 		skb_dst_set(skb, dst);
 	}
 #endif
@@ -53,7 +53,7 @@ int ip6_route_me_harder(struct sk_buff *skb)
 	if (skb_headroom(skb) < hh_len &&
 	    pskb_expand_head(skb, HH_DATA_ALIGN(hh_len - skb_headroom(skb)),
 			     0, GFP_ATOMIC))
-		return -1;
+		return -ENOMEM;
 
 	return 0;
 }
diff --git a/net/ipv6/netfilter/ip6table_mangle.c b/net/ipv6/netfilter/ip6table_mangle.c
index 6134a1e..e075399 100644
--- a/net/ipv6/netfilter/ip6table_mangle.c
+++ b/net/ipv6/netfilter/ip6table_mangle.c
@@ -38,7 +38,7 @@ ip6t_mangle_out(struct sk_buff *skb, const struct net_device *out)
 	struct in6_addr saddr, daddr;
 	u_int8_t hop_limit;
 	u_int32_t flowlabel, mark;
-
+	int err;
 #if 0
 	/* root is playing with raw sockets. */
 	if (skb->len < sizeof(struct iphdr) ||
@@ -65,8 +65,11 @@ ip6t_mangle_out(struct sk_buff *skb, const struct net_device *out)
 	     !ipv6_addr_equal(&ipv6_hdr(skb)->daddr, &daddr) ||
 	     skb->mark != mark ||
 	     ipv6_hdr(skb)->hop_limit != hop_limit ||
-	     flowlabel != *((u_int32_t *)ipv6_hdr(skb))))
-		return ip6_route_me_harder(skb) == 0 ? ret : NF_DROP;
+	     flowlabel != *((u_int32_t *)ipv6_hdr(skb)))) {
+		err = ip6_route_me_harder(skb);
+		if (err < 0)
+			ret = NF_DROP_ERR(err);
+	}
 
 	return ret;
 }
diff --git a/net/ipv6/netfilter/ip6table_nat.c b/net/ipv6/netfilter/ip6table_nat.c
index e0e788d..97e2edd 100644
--- a/net/ipv6/netfilter/ip6table_nat.c
+++ b/net/ipv6/netfilter/ip6table_nat.c
@@ -215,6 +215,7 @@ nf_nat_ipv6_local_fn(unsigned int hooknum,
 	const struct nf_conn *ct;
 	enum ip_conntrack_info ctinfo;
 	unsigned int ret;
+	int err;
 
 	/* root is playing with raw sockets. */
 	if (skb->len < sizeof(struct ipv6hdr))
@@ -227,8 +228,9 @@ nf_nat_ipv6_local_fn(unsigned int hooknum,
 
 		if (!nf_inet_addr_cmp(&ct->tuplehash[dir].tuple.dst.u3,
 				      &ct->tuplehash[!dir].tuple.src.u3)) {
-			if (ip6_route_me_harder(skb))
-				ret = NF_DROP;
+			err = ip6_route_me_harder(skb);
+			if (err < 0)
+				ret = NF_DROP_ERR(err);
 		}
 #ifdef CONFIG_XFRM
 		else if (!(IP6CB(skb)->flags & IP6SKB_XFRM_TRANSFORMED) &&
-- 
1.8.1.4

^ permalink raw reply related

* Re: Active URB submitted twice in pegasus driver
From: Petko Manolov @ 2013-04-05 17:01 UTC (permalink / raw)
  To: Sarah Sharp
  Cc: linux-usb-u79uwXL29TY76Z2rM5mHXA, Greg KH,
	netdev-u79uwXL29TY76Z2rM5mHXA, Stephen Hemminger
In-Reply-To: <alpine.DEB.2.02.1303262348460.3959@fry>

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1200 bytes --]

On Wed, 27 Mar 2013, Petko Manolov wrote:

> On Tue, 26 Mar 2013, Sarah Sharp wrote:
>
>> ctrl_callback is still reading the URB status, and using it in the
>> switch statement.  It's also using the urb->context as well, to dig out
>> a pointer (pegasus_t) that the pegasus_set_multicast already has access
>> to.  What happens if an URB to get the registers completes at the same
>> time pegasus_set_multicast calls ctrl_callback?  If the URB failed for
>> some reason (e.g. bad cable, stall), you'll miss that if statement.  I
>> don't think that's what you intended to do.
>> 
>> I think the fix should be to just to move the if block into a new
>> function, and call it both in ctrl_callback() and
>> pegasus_set_multicast().
>
> The _real_ fix would be to rework the whole callback mechanism, but this is a 
> different story.  It is in my todo list.

Attached you'll find two patches against Linus' 3.9.0-rc5.  These should 
effectively cure the broken control requests callback logic.  The patch is 
so big because i also cleaned up a lot of old cruft.

The new driver actually compiles and runs fine with Pegasus-II based 
device.  Please let me know if you run into problems.


cheers,
Petko

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Type: TEXT/x-diff; name=pegasus.h.diff, Size: 1246 bytes --]

diff --git a/drivers/net/usb/pegasus.h b/drivers/net/usb/pegasus.h
index 65b78b3..aada2fa 100644
--- a/drivers/net/usb/pegasus.h
+++ b/drivers/net/usb/pegasus.h
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 1999-2003 Petko Manolov - Petkan (petkan@users.sourceforge.net)
+ * Copyright (c) 1999-2013 Petko Manolov - Petkan (petkan@nucleusys.com)
  *
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License version 2 as published
@@ -34,8 +34,6 @@
 #define	CTRL_URB_SLEEP		0x00000020
 #define	PEGASUS_UNPLUG		0x00000040
 #define	PEGASUS_RX_URB_FAIL	0x00000080
-#define	ETH_REGS_CHANGE		0x40000000
-#define	ETH_REGS_CHANGED	0x80000000
 
 #define	RX_MULTICAST		2
 #define	RX_PROMISCUOUS		4
@@ -96,12 +94,9 @@ typedef struct pegasus {
 	int			intr_interval;
 	struct tasklet_struct	rx_tl;
 	struct delayed_work	carrier_check;
-	struct urb		*ctrl_urb, *rx_urb, *tx_urb, *intr_urb;
-	struct sk_buff		*rx_pool[RX_SKBS];
+	struct urb		*rx_urb, *tx_urb, *intr_urb;
 	struct sk_buff		*rx_skb;
 	struct usb_ctrlrequest	dr;
-	wait_queue_head_t	ctrl_wait;
-	spinlock_t		rx_pool_lock;
 	int			chip;
 	unsigned char		intr_buff[8];
 	__u8			tx_buff[PEGASUS_MTU];

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: Type: TEXT/x-diff; name=pegasus.c.diff, Size: 21453 bytes --]

diff --git a/drivers/net/usb/pegasus.c b/drivers/net/usb/pegasus.c
index 73051d1..64b7a04 100644
--- a/drivers/net/usb/pegasus.c
+++ b/drivers/net/usb/pegasus.c
@@ -1,5 +1,5 @@
 /*
- *  Copyright (c) 1999-2005 Petko Manolov (petkan@users.sourceforge.net)
+ *  Copyright (c) 1999-2013 Petko Manolov (petkan@nucleusys.com)
  *
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License version 2 as
@@ -26,6 +26,9 @@
  *		v0.5.1	ethtool support added
  *		v0.5.5	rx socket buffers are in a pool and the their allocation
  *			is out of the interrupt routine.
+ *		...
+ *		v0.9.1	simplified [get|set]_register(s), async update registers
+ *			logic revisited, receive skb_pool removed.
  */
 
 #include <linux/sched.h>
@@ -45,8 +48,8 @@
 /*
  * Version Information
  */
-#define DRIVER_VERSION "v0.6.14 (2006/09/27)"
-#define DRIVER_AUTHOR "Petko Manolov <petkan@users.sourceforge.net>"
+#define DRIVER_VERSION "v0.9.1 (2013/04/04)"
+#define DRIVER_AUTHOR "Petko Manolov <petkan@nucleusys.com>"
 #define DRIVER_DESC "Pegasus/Pegasus II USB Ethernet driver"
 
 static const char driver_name[] = "pegasus";
@@ -108,88 +111,43 @@ MODULE_PARM_DESC(msg_level, "Override default message level");
 MODULE_DEVICE_TABLE(usb, pegasus_ids);
 static const struct net_device_ops pegasus_netdev_ops;
 
-static int update_eth_regs_async(pegasus_t *);
-/* Aargh!!! I _really_ hate such tweaks */
-static void ctrl_callback(struct urb *urb)
+
+static void async_ctrl_callback(struct urb *urb)
 {
 	pegasus_t *pegasus = urb->context;
 	int status = urb->status;
 
 	if (!pegasus)
-		return;
+		goto out;
 
 	switch (status) {
 	case 0:
-		if (pegasus->flags & ETH_REGS_CHANGE) {
-			pegasus->flags &= ~ETH_REGS_CHANGE;
-			pegasus->flags |= ETH_REGS_CHANGED;
-			update_eth_regs_async(pegasus);
-			return;
-		}
 		break;
 	case -EINPROGRESS:
-		return;
+		goto out;
 	case -ENOENT:
 		break;
 	default:
 		if (net_ratelimit())
 			netif_dbg(pegasus, drv, pegasus->net,
-				  "%s, status %d\n", __func__, status);
+			          "%s, status %d\n", __func__, status);
 		break;
 	}
-	pegasus->flags &= ~ETH_REGS_CHANGED;
-	wake_up(&pegasus->ctrl_wait);
+out:
+	usb_free_urb(urb);
 }
 
 static int get_registers(pegasus_t *pegasus, __u16 indx, __u16 size,
 			 void *data)
 {
 	int ret;
-	char *buffer;
-	DECLARE_WAITQUEUE(wait, current);
-
-	buffer = kmalloc(size, GFP_KERNEL);
-	if (!buffer)
-		return -ENOMEM;
-
-	add_wait_queue(&pegasus->ctrl_wait, &wait);
-	set_current_state(TASK_UNINTERRUPTIBLE);
-	while (pegasus->flags & ETH_REGS_CHANGED)
-		schedule();
-	remove_wait_queue(&pegasus->ctrl_wait, &wait);
-	set_current_state(TASK_RUNNING);
-
-	pegasus->dr.bRequestType = PEGASUS_REQT_READ;
-	pegasus->dr.bRequest = PEGASUS_REQ_GET_REGS;
-	pegasus->dr.wValue = cpu_to_le16(0);
-	pegasus->dr.wIndex = cpu_to_le16(indx);
-	pegasus->dr.wLength = cpu_to_le16(size);
-	pegasus->ctrl_urb->transfer_buffer_length = size;
-
-	usb_fill_control_urb(pegasus->ctrl_urb, pegasus->usb,
-			     usb_rcvctrlpipe(pegasus->usb, 0),
-			     (char *) &pegasus->dr,
-			     buffer, size, ctrl_callback, pegasus);
 
-	add_wait_queue(&pegasus->ctrl_wait, &wait);
-	set_current_state(TASK_UNINTERRUPTIBLE);
-
-	/* using ATOMIC, we'd never wake up if we slept */
-	if ((ret = usb_submit_urb(pegasus->ctrl_urb, GFP_ATOMIC))) {
-		set_current_state(TASK_RUNNING);
-		if (ret == -ENODEV)
-			netif_device_detach(pegasus->net);
-		if (net_ratelimit())
-			netif_err(pegasus, drv, pegasus->net,
-				  "%s, status %d\n", __func__, ret);
-		goto out;
-	}
-
-	schedule();
-out:
-	remove_wait_queue(&pegasus->ctrl_wait, &wait);
-	memcpy(data, buffer, size);
-	kfree(buffer);
+	ret = usb_control_msg(pegasus->usb, usb_rcvctrlpipe(pegasus->usb, 0),
+	                      PEGASUS_REQ_GET_REGS, PEGASUS_REQT_READ, 0,
+	                      indx, data, size, 1000);
+	if (ret < 0)
+		netif_dbg(pegasus, drv, pegasus->net,
+		          "%s returned %d\n", __func__, ret);
 
 	return ret;
 }
@@ -198,126 +156,55 @@ static int set_registers(pegasus_t *pegasus, __u16 indx, __u16 size,
 			 void *data)
 {
 	int ret;
-	char *buffer;
-	DECLARE_WAITQUEUE(wait, current);
-
-	buffer = kmemdup(data, size, GFP_KERNEL);
-	if (!buffer) {
-		netif_warn(pegasus, drv, pegasus->net,
-			   "out of memory in %s\n", __func__);
-		return -ENOMEM;
-	}
-
-	add_wait_queue(&pegasus->ctrl_wait, &wait);
-	set_current_state(TASK_UNINTERRUPTIBLE);
-	while (pegasus->flags & ETH_REGS_CHANGED)
-		schedule();
-	remove_wait_queue(&pegasus->ctrl_wait, &wait);
-	set_current_state(TASK_RUNNING);
-
-	pegasus->dr.bRequestType = PEGASUS_REQT_WRITE;
-	pegasus->dr.bRequest = PEGASUS_REQ_SET_REGS;
-	pegasus->dr.wValue = cpu_to_le16(0);
-	pegasus->dr.wIndex = cpu_to_le16(indx);
-	pegasus->dr.wLength = cpu_to_le16(size);
-	pegasus->ctrl_urb->transfer_buffer_length = size;
-
-	usb_fill_control_urb(pegasus->ctrl_urb, pegasus->usb,
-			     usb_sndctrlpipe(pegasus->usb, 0),
-			     (char *) &pegasus->dr,
-			     buffer, size, ctrl_callback, pegasus);
-
-	add_wait_queue(&pegasus->ctrl_wait, &wait);
-	set_current_state(TASK_UNINTERRUPTIBLE);
-
-	if ((ret = usb_submit_urb(pegasus->ctrl_urb, GFP_ATOMIC))) {
-		if (ret == -ENODEV)
-			netif_device_detach(pegasus->net);
-		netif_err(pegasus, drv, pegasus->net,
-			  "%s, status %d\n", __func__, ret);
-		goto out;
-	}
-
-	schedule();
-out:
-	remove_wait_queue(&pegasus->ctrl_wait, &wait);
-	kfree(buffer);
 
+	ret = usb_control_msg(pegasus->usb, usb_sndctrlpipe(pegasus->usb, 0),
+	                      PEGASUS_REQ_SET_REGS, PEGASUS_REQT_WRITE, 0,
+	                      indx, data, size, 100);
+	if (ret < 0)
+		netif_dbg(pegasus, drv, pegasus->net,
+		          "%s returned %d\n", __func__, ret);
 	return ret;
 }
 
 static int set_register(pegasus_t *pegasus, __u16 indx, __u8 data)
 {
 	int ret;
-	char *tmp;
-	DECLARE_WAITQUEUE(wait, current);
-
-	tmp = kmemdup(&data, 1, GFP_KERNEL);
-	if (!tmp) {
-		netif_warn(pegasus, drv, pegasus->net,
-			   "out of memory in %s\n", __func__);
-		return -ENOMEM;
-	}
-	add_wait_queue(&pegasus->ctrl_wait, &wait);
-	set_current_state(TASK_UNINTERRUPTIBLE);
-	while (pegasus->flags & ETH_REGS_CHANGED)
-		schedule();
-	remove_wait_queue(&pegasus->ctrl_wait, &wait);
-	set_current_state(TASK_RUNNING);
 
-	pegasus->dr.bRequestType = PEGASUS_REQT_WRITE;
-	pegasus->dr.bRequest = PEGASUS_REQ_SET_REG;
-	pegasus->dr.wValue = cpu_to_le16(data);
-	pegasus->dr.wIndex = cpu_to_le16(indx);
-	pegasus->dr.wLength = cpu_to_le16(1);
-	pegasus->ctrl_urb->transfer_buffer_length = 1;
-
-	usb_fill_control_urb(pegasus->ctrl_urb, pegasus->usb,
-			     usb_sndctrlpipe(pegasus->usb, 0),
-			     (char *) &pegasus->dr,
-			     tmp, 1, ctrl_callback, pegasus);
-
-	add_wait_queue(&pegasus->ctrl_wait, &wait);
-	set_current_state(TASK_UNINTERRUPTIBLE);
-
-	if ((ret = usb_submit_urb(pegasus->ctrl_urb, GFP_ATOMIC))) {
-		if (ret == -ENODEV)
-			netif_device_detach(pegasus->net);
-		if (net_ratelimit())
-			netif_err(pegasus, drv, pegasus->net,
-				  "%s, status %d\n", __func__, ret);
-		goto out;
-	}
-
-	schedule();
-out:
-	remove_wait_queue(&pegasus->ctrl_wait, &wait);
-	kfree(tmp);
+	ret = usb_control_msg(pegasus->usb, usb_sndctrlpipe(pegasus->usb, 0),
+	                      PEGASUS_REQ_SET_REG, PEGASUS_REQT_WRITE, data,
+	                      indx, &data, 1, 1000);
+	if (ret < 0)
+		netif_dbg(pegasus, drv, pegasus->net,
+		          "%s returned %d\n", __func__, ret);
 
 	return ret;
 }
 
 static int update_eth_regs_async(pegasus_t *pegasus)
 {
-	int ret;
+	int ret = -ENOMEM;
+	struct urb *async_urb;
+
+	if ((async_urb = usb_alloc_urb(0, GFP_ATOMIC)) == NULL)
+		return ret;
 
 	pegasus->dr.bRequestType = PEGASUS_REQT_WRITE;
 	pegasus->dr.bRequest = PEGASUS_REQ_SET_REGS;
 	pegasus->dr.wValue = cpu_to_le16(0);
 	pegasus->dr.wIndex = cpu_to_le16(EthCtrl0);
 	pegasus->dr.wLength = cpu_to_le16(3);
-	pegasus->ctrl_urb->transfer_buffer_length = 3;
+	async_urb->transfer_buffer_length = 3;
 
-	usb_fill_control_urb(pegasus->ctrl_urb, pegasus->usb,
-			     usb_sndctrlpipe(pegasus->usb, 0),
-			     (char *) &pegasus->dr,
-			     pegasus->eth_regs, 3, ctrl_callback, pegasus);
+	usb_fill_control_urb(async_urb, pegasus->usb,
+	                     usb_sndctrlpipe(pegasus->usb, 0),
+	                     (char *) &pegasus->dr, pegasus->eth_regs,
+	                     3, async_ctrl_callback, pegasus);
 
-	if ((ret = usb_submit_urb(pegasus->ctrl_urb, GFP_ATOMIC))) {
+	if ((ret = usb_submit_urb(async_urb, GFP_ATOMIC))) {
 		if (ret == -ENODEV)
 			netif_device_detach(pegasus->net);
 		netif_err(pegasus, drv, pegasus->net,
-			  "%s, status %d\n", __func__, ret);
+		          "%s returned %d\n", __func__, ret);
 	}
 
 	return ret;
@@ -342,8 +229,10 @@ static int read_mii_word(pegasus_t *pegasus, __u8 phy, __u8 indx, __u16 *regd)
 			break;
 	}
 
-	if (i >= REG_TIMEOUT)
+	if (i >= REG_TIMEOUT) {
+		ret = -EBUSY;
 		goto fail;
+	}
 
 	ret = get_registers(pegasus, PhyData, 2, &regdi);
 	*regd = le16_to_cpu(regdi);
@@ -480,7 +369,7 @@ fail:
 	netif_warn(pegasus, drv, pegasus->net, "%s failed\n", __func__);
 	return -ETIMEDOUT;
 }
-#endif				/* PEGASUS_WRITE_EEPROM */
+#endif	/* PEGASUS_WRITE_EEPROM */
 
 static inline void get_node_id(pegasus_t *pegasus, __u8 *id)
 {
@@ -543,12 +432,11 @@ static inline int reset_mac(pegasus_t *pegasus)
 	return 0;
 }
 
-static int enable_net_traffic(struct net_device *dev, struct usb_device *usb)
+static void enable_net_traffic(struct net_device *dev, struct usb_device *usb)
 {
 	__u16 linkpart;
 	__u8 data[4];
 	pegasus_t *pegasus = netdev_priv(dev);
-	int ret;
 
 	read_mii_word(pegasus, pegasus->phy, MII_LPA, &linkpart);
 	data[0] = 0xc9;
@@ -562,7 +450,7 @@ static int enable_net_traffic(struct net_device *dev, struct usb_device *usb)
 	data[2] = loopback ? 0x09 : 0x01;
 
 	memcpy(pegasus->eth_regs, data, sizeof(data));
-	ret = set_registers(pegasus, EthCtrl0, 3, data);
+	set_registers(pegasus, EthCtrl0, 3, data);
 
 	if (usb_dev_id[pegasus->dev_index].vendor == VENDOR_LINKSYS ||
 	    usb_dev_id[pegasus->dev_index].vendor == VENDOR_LINKSYS2 ||
@@ -571,53 +459,6 @@ static int enable_net_traffic(struct net_device *dev, struct usb_device *usb)
 		read_mii_word(pegasus, 0, 0x1b, &auxmode);
 		write_mii_word(pegasus, 0, 0x1b, auxmode | 4);
 	}
-
-	return ret;
-}
-
-static void fill_skb_pool(pegasus_t *pegasus)
-{
-	int i;
-
-	for (i = 0; i < RX_SKBS; i++) {
-		if (pegasus->rx_pool[i])
-			continue;
-		pegasus->rx_pool[i] = dev_alloc_skb(PEGASUS_MTU + 2);
-		/*
-		 ** we give up if the allocation fail. the tasklet will be
-		 ** rescheduled again anyway...
-		 */
-		if (pegasus->rx_pool[i] == NULL)
-			return;
-		skb_reserve(pegasus->rx_pool[i], 2);
-	}
-}
-
-static void free_skb_pool(pegasus_t *pegasus)
-{
-	int i;
-
-	for (i = 0; i < RX_SKBS; i++) {
-		if (pegasus->rx_pool[i]) {
-			dev_kfree_skb(pegasus->rx_pool[i]);
-			pegasus->rx_pool[i] = NULL;
-		}
-	}
-}
-
-static inline struct sk_buff *pull_skb(pegasus_t * pegasus)
-{
-	int i;
-	struct sk_buff *skb;
-
-	for (i = 0; i < RX_SKBS; i++) {
-		if (likely(pegasus->rx_pool[i] != NULL)) {
-			skb = pegasus->rx_pool[i];
-			pegasus->rx_pool[i] = NULL;
-			return skb;
-		}
-	}
-	return NULL;
 }
 
 static void read_bulk_callback(struct urb *urb)
@@ -643,7 +484,7 @@ static void read_bulk_callback(struct urb *urb)
 		netif_dbg(pegasus, rx_err, net, "reset MAC\n");
 		pegasus->flags &= ~PEGASUS_RX_BUSY;
 		break;
-	case -EPIPE:		/* stall, or disconnect from TT */
+	case -EPIPE:	/* stall, or disconnect from TT */
 		/* FIXME schedule work to clear the halt */
 		netif_warn(pegasus, rx_err, net, "no rx stall recovery\n");
 		return;
@@ -663,7 +504,7 @@ static void read_bulk_callback(struct urb *urb)
 	rx_status = buf[count - 2];
 	if (rx_status & 0x1e) {
 		netif_dbg(pegasus, rx_err, net,
-			  "RX packet error %x\n", rx_status);
+		          "RX packet error %x\n", rx_status);
 		pegasus->stats.rx_errors++;
 		if (rx_status & 0x06)	/* long or runt	*/
 			pegasus->stats.rx_length_errors++;
@@ -704,9 +545,8 @@ static void read_bulk_callback(struct urb *urb)
 	if (pegasus->flags & PEGASUS_UNPLUG)
 		return;
 
-	spin_lock(&pegasus->rx_pool_lock);
-	pegasus->rx_skb = pull_skb(pegasus);
-	spin_unlock(&pegasus->rx_pool_lock);
+	pegasus->rx_skb = __netdev_alloc_skb_ip_align(pegasus->net,
+	                                              PEGASUS_MTU, GFP_ATOMIC);
 
 	if (pegasus->rx_skb == NULL)
 		goto tl_sched;
@@ -734,24 +574,23 @@ tl_sched:
 static void rx_fixup(unsigned long data)
 {
 	pegasus_t *pegasus;
-	unsigned long flags;
 	int status;
 
 	pegasus = (pegasus_t *) data;
 	if (pegasus->flags & PEGASUS_UNPLUG)
 		return;
 
-	spin_lock_irqsave(&pegasus->rx_pool_lock, flags);
-	fill_skb_pool(pegasus);
 	if (pegasus->flags & PEGASUS_RX_URB_FAIL)
 		if (pegasus->rx_skb)
 			goto try_again;
 	if (pegasus->rx_skb == NULL)
-		pegasus->rx_skb = pull_skb(pegasus);
+		pegasus->rx_skb = __netdev_alloc_skb_ip_align(pegasus->net,
+		                                              PEGASUS_MTU,
+		                                              GFP_ATOMIC);
 	if (pegasus->rx_skb == NULL) {
 		netif_warn(pegasus, rx_err, pegasus->net, "low on memory\n");
 		tasklet_schedule(&pegasus->rx_tl);
-		goto done;
+		return;
 	}
 	usb_fill_bulk_urb(pegasus->rx_urb, pegasus->usb,
 			  usb_rcvbulkpipe(pegasus->usb, 1),
@@ -767,8 +606,6 @@ try_again:
 	} else {
 		pegasus->flags &= ~PEGASUS_RX_URB_FAIL;
 	}
-done:
-	spin_unlock_irqrestore(&pegasus->rx_pool_lock, flags);
 }
 
 static void write_bulk_callback(struct urb *urb)
@@ -860,7 +697,7 @@ static void intr_callback(struct urb *urb)
 		netif_device_detach(pegasus->net);
 	if (res)
 		netif_err(pegasus, timer, net,
-			  "can't resubmit interrupt urb, %d\n", res);
+		          "can't resubmit interrupt urb, %d\n", res);
 }
 
 static void pegasus_tx_timeout(struct net_device *net)
@@ -890,10 +727,10 @@ static netdev_tx_t pegasus_start_xmit(struct sk_buff *skb,
 	if ((res = usb_submit_urb(pegasus->tx_urb, GFP_ATOMIC))) {
 		netif_warn(pegasus, tx_err, net, "fail tx, %d\n", res);
 		switch (res) {
-		case -EPIPE:		/* stall, or disconnect from TT */
+		case -EPIPE:	/* stall, or disconnect from TT */
 			/* cleanup should already have been scheduled */
 			break;
-		case -ENODEV:		/* disconnect() upcoming */
+		case -ENODEV:	/* disconnect() upcoming */
 		case -EPERM:
 			netif_device_detach(pegasus->net);
 			break;
@@ -932,8 +769,8 @@ static inline void get_interrupt_interval(pegasus_t *pegasus)
 	if (pegasus->usb->speed != USB_SPEED_HIGH) {
 		if (interval < 0x80) {
 			netif_info(pegasus, timer, pegasus->net,
-				   "intr interval changed from %ums to %ums\n",
-				   interval, 0x80);
+			           "intr interval changed from %ums to %ums\n",
+			           interval, 0x80);
 			interval = 0x80;
 			data = (data & 0x00FF) | ((u16)interval << 8);
 #ifdef PEGASUS_WRITE_EEPROM
@@ -951,7 +788,6 @@ static void set_carrier(struct net_device *net)
 
 	if (read_mii_word(pegasus, pegasus->phy, MII_BMSR, &tmp))
 		return;
-
 	if (tmp & BMSR_LSTATUS)
 		netif_carrier_on(net);
 	else
@@ -963,7 +799,6 @@ static void free_all_urbs(pegasus_t *pegasus)
 	usb_free_urb(pegasus->intr_urb);
 	usb_free_urb(pegasus->tx_urb);
 	usb_free_urb(pegasus->rx_urb);
-	usb_free_urb(pegasus->ctrl_urb);
 }
 
 static void unlink_all_urbs(pegasus_t *pegasus)
@@ -971,30 +806,23 @@ static void unlink_all_urbs(pegasus_t *pegasus)
 	usb_kill_urb(pegasus->intr_urb);
 	usb_kill_urb(pegasus->tx_urb);
 	usb_kill_urb(pegasus->rx_urb);
-	usb_kill_urb(pegasus->ctrl_urb);
 }
 
 static int alloc_urbs(pegasus_t *pegasus)
 {
-	pegasus->ctrl_urb = usb_alloc_urb(0, GFP_KERNEL);
-	if (!pegasus->ctrl_urb)
-		return 0;
 	pegasus->rx_urb = usb_alloc_urb(0, GFP_KERNEL);
 	if (!pegasus->rx_urb) {
-		usb_free_urb(pegasus->ctrl_urb);
 		return 0;
 	}
 	pegasus->tx_urb = usb_alloc_urb(0, GFP_KERNEL);
 	if (!pegasus->tx_urb) {
 		usb_free_urb(pegasus->rx_urb);
-		usb_free_urb(pegasus->ctrl_urb);
 		return 0;
 	}
 	pegasus->intr_urb = usb_alloc_urb(0, GFP_KERNEL);
 	if (!pegasus->intr_urb) {
 		usb_free_urb(pegasus->tx_urb);
 		usb_free_urb(pegasus->rx_urb);
-		usb_free_urb(pegasus->ctrl_urb);
 		return 0;
 	}
 
@@ -1004,15 +832,12 @@ static int alloc_urbs(pegasus_t *pegasus)
 static int pegasus_open(struct net_device *net)
 {
 	pegasus_t *pegasus = netdev_priv(net);
-	int res;
+	int res=-ENOMEM;
 
 	if (pegasus->rx_skb == NULL)
-		pegasus->rx_skb = pull_skb(pegasus);
-	/*
-	 ** Note: no point to free the pool.  it is empty :-)
-	 */
+		pegasus->rx_skb = __netdev_alloc_skb_ip_align(pegasus->net, PEGASUS_MTU, GFP_KERNEL);
 	if (!pegasus->rx_skb)
-		return -ENOMEM;
+		goto exit;
 
 	res = set_registers(pegasus, EthID, 6, net->dev_addr);
 
@@ -1038,18 +863,9 @@ static int pegasus_open(struct net_device *net)
 		usb_kill_urb(pegasus->rx_urb);
 		goto exit;
 	}
-	if ((res = enable_net_traffic(net, pegasus->usb))) {
-		netif_dbg(pegasus, ifup, net,
-			  "can't enable_net_traffic() - %d\n", res);
-		res = -EIO;
-		usb_kill_urb(pegasus->rx_urb);
-		usb_kill_urb(pegasus->intr_urb);
-		free_skb_pool(pegasus);
-		goto exit;
-	}
+	enable_net_traffic(net, pegasus->usb);
 	set_carrier(net);
 	netif_start_queue(net);
-	netif_dbg(pegasus, ifup, net, "open\n");
 	res = 0;
 exit:
 	return res;
@@ -1113,8 +929,7 @@ pegasus_set_wol(struct net_device *dev, struct ethtool_wolinfo *wol)
 
 	ret = set_register(pegasus, WakeupControl, reg78);
 	if (!ret)
-		ret = device_set_wakeup_enable(&pegasus->usb->dev,
-						wol->wolopts);
+		ret = device_set_wakeup_enable(&pegasus->usb->dev, wol->wolopts);
 	return ret;
 }
 
@@ -1214,16 +1029,13 @@ static void pegasus_set_multicast(struct net_device *net)
 	} else if (!netdev_mc_empty(net) || (net->flags & IFF_ALLMULTI)) {
 		pegasus->eth_regs[EthCtrl0] |= RX_MULTICAST;
 		pegasus->eth_regs[EthCtrl2] &= ~RX_PROMISCUOUS;
-		netif_dbg(pegasus, link, net, "set allmulti\n");
+		netif_info(pegasus, link, net, "set allmulti\n");
 	} else {
 		pegasus->eth_regs[EthCtrl0] &= ~RX_MULTICAST;
 		pegasus->eth_regs[EthCtrl2] &= ~RX_PROMISCUOUS;
+		netif_info(pegasus, link, net, "general mode\n");
 	}
-
-	pegasus->ctrl_urb->status = 0;
-
-	pegasus->flags |= ETH_REGS_CHANGE;
-	ctrl_callback(pegasus->ctrl_urb);
+	update_eth_regs_async(pegasus);
 }
 
 static __u8 mii_phy_probe(pegasus_t *pegasus)
@@ -1281,10 +1093,9 @@ static void check_carrier(struct work_struct *work)
 {
 	pegasus_t *pegasus = container_of(work, pegasus_t, carrier_check.work);
 	set_carrier(pegasus->net);
-	if (!(pegasus->flags & PEGASUS_UNPLUG)) {
+	if (!(pegasus->flags & PEGASUS_UNPLUG))
 		queue_delayed_work(pegasus_workqueue, &pegasus->carrier_check,
-			CARRIER_CHECK_DELAY);
-	}
+		                   CARRIER_CHECK_DELAY);
 }
 
 static int pegasus_blacklisted(struct usb_device *udev)
@@ -1303,7 +1114,8 @@ static int pegasus_blacklisted(struct usb_device *udev)
 	return 0;
 }
 
-/* we rely on probe() and remove() being serialized so we
+/*
+ * we rely on probe() and remove() being serialized so we
  * don't need extra locking on pegasus_count.
  */
 static void pegasus_dec_workqueue(void)
@@ -1340,7 +1152,6 @@ static int pegasus_probe(struct usb_interface *intf,
 
 	pegasus = netdev_priv(net);
 	pegasus->dev_index = dev_index;
-	init_waitqueue_head(&pegasus->ctrl_wait);
 
 	if (!alloc_urbs(pegasus)) {
 		dev_err(&intf->dev, "can't allocate %s\n", "urbs");
@@ -1364,7 +1175,6 @@ static int pegasus_probe(struct usb_interface *intf,
 	pegasus->mii.mdio_write = mdio_write;
 	pegasus->mii.phy_id_mask = 0x1f;
 	pegasus->mii.reg_num_mask = 0x1f;
-	spin_lock_init(&pegasus->rx_pool_lock);
 	pegasus->msg_enable = netif_msg_init(msg_level, NETIF_MSG_DRV
 				| NETIF_MSG_PROBE | NETIF_MSG_LINK);
 
@@ -1376,7 +1186,6 @@ static int pegasus_probe(struct usb_interface *intf,
 		goto out2;
 	}
 	set_ethernet_addr(pegasus);
-	fill_skb_pool(pegasus);
 	if (pegasus->features & PEGASUS_II) {
 		dev_info(&intf->dev, "setup Pegasus II specific registers\n");
 		setup_pegasus_II(pegasus);
@@ -1396,15 +1205,12 @@ static int pegasus_probe(struct usb_interface *intf,
 	queue_delayed_work(pegasus_workqueue, &pegasus->carrier_check,
 				CARRIER_CHECK_DELAY);
 
-	dev_info(&intf->dev, "%s, %s, %pM\n",
-		 net->name,
-		 usb_dev_id[dev_index].name,
-		 net->dev_addr);
+	dev_info(&intf->dev, "%s, %s, %pM\n", net->name,
+	         usb_dev_id[dev_index].name, net->dev_addr);
 	return 0;
 
 out3:
 	usb_set_intfdata(intf, NULL);
-	free_skb_pool(pegasus);
 out2:
 	free_all_urbs(pegasus);
 out1:
@@ -1429,7 +1235,6 @@ static void pegasus_disconnect(struct usb_interface *intf)
 	unregister_netdev(pegasus->net);
 	unlink_all_urbs(pegasus);
 	free_all_urbs(pegasus);
-	free_skb_pool(pegasus);
 	if (pegasus->rx_skb != NULL) {
 		dev_kfree_skb(pegasus->rx_skb);
 		pegasus->rx_skb = NULL;

^ permalink raw reply related

* Re: PROBLEM: IPv6 TCP-Connections resetting
From: Neal Cardwell @ 2013-04-05 17:54 UTC (permalink / raw)
  To: Tetja Rediske; +Cc: LKML, Netdev
In-Reply-To: <20130405174828.310a02c3@trediske.ws.office.manitu.net>

On Fri, Apr 5, 2013 at 11:48 AM, Tetja Rediske <tetja@tetja.de> wrote:
> I tracked it down with 'git bisect' to commit:
>
>   093d04d42fa094f6740bb188f0ad0c215ff61e2c
...

Thanks for the  detailed report!

> 11:52:04.634656 IP6 fe80::92e2:baff:fe00:c120 > 2a00:1828:1000:1102::2:
> ICMP6, redirect, 2a00:1828:0:1::10 to 2a00:1828:0:1::10, length 136

Would you be able to re-run your tests with a tcpdump command line like:

  tcpdump -v -n -X -s 1600 icmp6

And report the full dump of this first ICMP6 packet in the exchange?

It seems that perhaps the parsing/validation of this packet is failing
somehow, so it would be nice to know exactly what that packet looked
like.

Also, are both sides of your test running 3.9.0-rc5+?

And can you please cc netdev@vger.kernel.org if you follow up with more details?

Thanks!
neal

^ permalink raw reply

* Re: PROBLEM: IPv6 TCP-Connections resetting
From: Eric Dumazet @ 2013-04-05 17:58 UTC (permalink / raw)
  To: Tetja Rediske; +Cc: linux-kernel, Duan Jiong, netdev
In-Reply-To: <20130405174828.310a02c3@trediske.ws.office.manitu.net>

On Fri, 2013-04-05 at 17:48 +0200, Tetja Rediske wrote:
> Hi,
> 

CC netdev and Duan Jiong (author of bad commit)

> [1.] One line summary of the problem:
> 
> IPv6 TCP-Connections resetting 
> 
> [2.] Full description of the problem/report:
> 
> In the last weeks we updated some of our systems to a 3.8.4 Kernel.
> Since then sometimes we can't connect to services running IPv6,
> Apache and Openssh tested. 
> 
> We got this on different machines with x86 and x86_64 Kernels. On
> x86_64 it is more random, but on x86 i can reproduce it permanently
> (Just opening any TCP Connection 1st time or after some short delay).
> Connecting quick after the reset again will work as expected. It will
> also work, if you keep another connection open.
> 
> Before I got to the Kernel, I just kept an strace on an userspace
> process, but it did not notice the connection attempt. After this I
> monitored the connection with tcpdump, but nothing unusual.
> 
> Then I did a rollback to the older Kernel and it worked as expected.
> 
> I tracked it down with 'git bisect' to commit:
> 
>   093d04d42fa094f6740bb188f0ad0c215ff61e2c
> 
> I also tested latest git state available.
> 
> [3.] Keywords (i.e., modules, networking, kernel):
> 
> networking, IPv6
> 
> [4.] Kernel information
> [4.1.] Kernel version (from /proc/version): 
> 
>   since commit: 093d04d42fa094f6740bb188f0ad0c215ff61e2c
> 
> [4.2.] Kernel .config file:
> [5.] Most recent kernel version which did not have the bug:
> 
> none
> 
> [6.] Output of Oops.. message (if applicable) with symbolic information
>      resolved (see Documentation/oops-tracing.txt)
> [7.] A small shell script or example program which triggers the
>      problem (if possible)
> [8.] Environment
> [8.1.] Software (add the output of the ver_linux script here)
> 
> Different systems, mostly reproduced on this one:
> 
> Linux dns03.tetja.de 3.9.0-rc5+ #10 SMP Fri Apr 5 16:55:54 CEST 2013
> i686 AMD Athlon(tm) 64 X2 Dual Core Processor 5600+ AuthenticAMD
> GNU/Linux
> 
> Gnu C                  4.4.5
> Gnu make               3.82
> binutils               2.22
> util-linux             2.22.2
> mount                  debug
> module-init-tools      12
> e2fsprogs              1.42
> jfsutils               1.1.15
> reiserfsprogs          3.6.21
> xfsprogs               3.1.10
> Linux C Library        2.15
> Dynamic linker (ldd)   2.15
> Procps                 3.3.4
> Net-tools              1.60_p20120127084908
> Kbd                    1.15.3wip
> Sh-utils               8.20
> Modules Loaded
> 
> Connections looking like this on booth sites:
> 
> 11:52:04.634315 IP6 2a00:1828:0:1::10.51808 >
> 2a00:1828:1000:1102::2.80: Flags [S], seq 103067898, win 5760, options
> [mss 1440,sackOK,TS val 232579708 ecr 0,nop,wscale 7], length 0
> 
> 11:52:04.634354 IP6 2a00:1828:1000:1102::2.80 >
> 2a00:1828:0:1::10.51808: Flags [S.], seq 3352491415, ack 103067899, win
> 14280, options [mss 1440,sackOK,TS val 174797959 ecr
> 232579708,nop,wscale 7], length 0 
> 
> 11:52:04.634656 IP6 fe80::92e2:baff:fe00:c120 > 2a00:1828:1000:1102::2:
> ICMP6, redirect, 2a00:1828:0:1::10 to 2a00:1828:0:1::10, length 136
> 
> 11:52:04.634715 IP6 2a00:1828:0:1::10.51808 >
> 2a00:1828:1000:1102::2.80: Flags [.], ack 1, win 45, options
> [nop,nop,TS val 232579708 ecr 174797959], length 0
> 
> 11:52:04.634726 IP6 2a00:1828:1000:1102::2.80 >
> 2a00:1828:0:1::10.51808: Flags [R], seq 3352491416, win 0, length 0
> 
> 11:52:04.635027 IP6 2a00:1828:0:1::10.51808 >
> 2a00:1828:1000:1102::2.80: Flags [P.], seq 1:359, ack 1, win 45,
> options [nop,nop,TS val 232579708 ecr 174797959], length 358
> 
> 11:52:04.635037 IP6 2a00:1828:1000:1102::2.80 >
> 2a00:1828:0:1::10.51808: Flags [R], seq 3352491416, win 0, length 0
> 
> 11:52:04.635071 IP6 fe80::92e2:baff:fe00:c120 > 2a00:1828:1000:1102::2:
> ICMP6, redirect, 2a00:1828:0:1::10 to 2a00:1828:0:1::10, length 112
> 
> 11:52:04.635246 IP6 fe80::92e2:baff:fe00:c120 > 2a00:1828:1000:1102::2:
> ICMP6, redirect, 2a00:1828:0:1::10 to 2a00:1828:0:1::10, length 112
> 
> Kind Regards and keep up the good work! :)
> 
> Tetja Rediske

^ permalink raw reply

* Re: [PATCH 1/5 v2] mv643xx_eth: add Device Tree bindings
From: Jason Gunthorpe @ 2013-04-05 18:04 UTC (permalink / raw)
  To: Sebastian Hesselbarth
  Cc: thomas.petazzoni, moinejf, Simon Baatz, andrew, netdev,
	devicetree-discuss, rob.herring, davem, grant.likely, jogo,
	buytenh, jm, Florian Fainelli, linux-arm-kernel, jason
In-Reply-To: <CABJ1b_TdzE_Q9qLsdSGZy8byFpCTtVBUy5FtxiZAx7QqgG-rVg@mail.gmail.com>

On Fri, Apr 05, 2013 at 03:58:03PM +0200, Sebastian Hesselbarth wrote:

> I don't think that the ethernet controller should probe the PHY's on mdio-bus
> at all. At least not for DT enabled platforms. I had a look at DT and non-DT
> mdio-bus sources, and realized that there is a bus scan for non-DT only.
> of_mdiobus_register requires you to set (and know) the PHY address.

DT platforms should have the option to use the standard phy-phandle
connection:


		mdio@72004 {
			#address-cells = <1>;
			#size-cells = <0>;
			compatible = "marvell,orion-mdio";
			reg = <0x72004 0x84>;
			status = "disabled";

+                        PHY1: ethernet-phy@1 {
+                                reg = <1>;
+                                device_type = "ethernet-phy";
+                        };
		};

		ethernet-group@72000 {
			#address-cells = <1>;
			#size-cells = <0>;
			compatible = "marvell,mv643xx-eth-block";
			reg = <0x72000 0x4000>;
			tx-csum-limit = <1600>;
			status = "disabled";

			egiga0: ethernet@0 {
				device_type = "network";
				compatible = "marvell,mv643xx-eth";
				reg = <0>;
				interrupts = <29>;
				clocks = <&gate_clk 2>;
+	                        phy-handle = <&PHY1>;
			};
		};

When phy-handle is present the ethernet driver should not probe/scan for
phys.

There is standard code to handle all of this - an important gain is
that the phy driver now has access to a DT node and can apply
phy-specific properties.

> We had a similar discussion whether to probe or not for DT nodes,
> and I guess there also will be some discussion about the above
> patch. OTOH we could just (again) ask users of every
> kirkwood/orion5x/dove board to tell their phy addresses and fail to
> probe the phy for new boards...

Maybe print a warning and call the no-DT phy probe code if phy-handle
is nor present?

Not sure this should be in the common code, phy probing is sketchy, it
shouldn't be encouraged, IMHO..

Jason

^ permalink raw reply

* [PATCH] netfilter: nf_conntrack_sip: don't drop packets with offsets pointing outside the packet
From: Patrick McHardy @ 2013-04-05 18:13 UTC (permalink / raw)
  To: pablo; +Cc: netfilter-devel, netdev

Some Cisco phones create huge messages that are spread over multiple packets.
After calculating the offset of the SIP body, it is validated to be within
the packet and the packet is dropped otherwise. This breaks operation of
these phones. Since connection tracking is supposed to be passive, just let
those packets pass unmodified and untracked.

Signed-off-by: Patrick McHardy <kaber@trash.net>
---
 net/netfilter/nf_conntrack_sip.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/net/netfilter/nf_conntrack_sip.c b/net/netfilter/nf_conntrack_sip.c
index 0e7d423..e0c4373 100644
--- a/net/netfilter/nf_conntrack_sip.c
+++ b/net/netfilter/nf_conntrack_sip.c
@@ -1593,10 +1593,8 @@ static int sip_help_tcp(struct sk_buff *skb, unsigned int protoff,
 		end += strlen("\r\n\r\n") + clen;
 
 		msglen = origlen = end - dptr;
-		if (msglen > datalen) {
-			nf_ct_helper_log(skb, ct, "incomplete/bad SIP message");
-			return NF_DROP;
-		}
+		if (msglen > datalen)
+			return NF_ACCEPT;
 
 		ret = process_sip_msg(skb, ct, protoff, dataoff,
 				      &dptr, &msglen);
-- 
1.8.1.4


^ permalink raw reply related

* [PATCH] netfilter: don't reset nf_trace in nf_reset()
From: Patrick McHardy @ 2013-04-05 18:42 UTC (permalink / raw)
  To: pablo; +Cc: netfilter-devel, netdev, gaofeng

Commit 130549fe added code to reset nf_trace in nf_reset(). This is wrong
and unnecessary.

nf_reset() is used in the following cases:

- when passing packets up the the socket layer, at which point we want to
  release all netfilter references that might keep modules pinned while
  the packet is queued. nf_trace doesn't matter anymore at this point.

- when encapsulating or decapsulating IPsec packets. We want to continue
  tracing these packets after IPsec processing.

- when passing packets through virtual network devices. Only devices on
  that encapsulate in IPv4/v6 matter since otherwise nf_trace is not
  used anymore. Its not entirely clear whether those packets should
  be traced after that, however we've always done that.

- when passing packets through virtual network devices that make the
  packet cross network namespace boundaries. This is the only cases
  where we clearly want to reset nf_trace and is also what the
  original patch intended to fix.

Add a new function nf_reset_trace() and use it in dev_forward_skb() to
fix this properly.

Signed-off-by: Patrick McHardy <kaber@trash.net>
---
 include/linux/skbuff.h | 4 ++++
 net/core/dev.c         | 1 +
 2 files changed, 5 insertions(+)

diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 72b3967..b8292d8 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -2641,6 +2641,10 @@ static inline void nf_reset(struct sk_buff *skb)
 	nf_bridge_put(skb->nf_bridge);
 	skb->nf_bridge = NULL;
 #endif
+}
+
+static inline void nf_reset_trace(struct sk_buff *skb)
+{
 #if IS_ENABLED(CONFIG_NETFILTER_XT_TARGET_TRACE)
 	skb->nf_trace = 0;
 #endif
diff --git a/net/core/dev.c b/net/core/dev.c
index 13e6447..e7d68ed 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -1639,6 +1639,7 @@ int dev_forward_skb(struct net_device *dev, struct sk_buff *skb)
 	skb->mark = 0;
 	secpath_reset(skb);
 	nf_reset(skb);
+	nf_reset_trace(skb);
 	return netif_rx(skb);
 }
 EXPORT_SYMBOL_GPL(dev_forward_skb);
-- 
1.8.1.4

^ permalink raw reply related

* Re: [PATCH] netfilter: don't reset nf_trace in nf_reset()
From: David Miller @ 2013-04-05 18:56 UTC (permalink / raw)
  To: kaber; +Cc: pablo, netfilter-devel, netdev, gaofeng
In-Reply-To: <1365187325-25147-1-git-send-email-kaber@trash.net>

From: Patrick McHardy <kaber@trash.net>
Date: Fri,  5 Apr 2013 20:42:05 +0200

> Commit 130549fe added code to reset nf_trace in nf_reset(). This is wrong
> and unnecessary.
> 
> nf_reset() is used in the following cases:
> 
> - when passing packets up the the socket layer, at which point we want to
>   release all netfilter references that might keep modules pinned while
>   the packet is queued. nf_trace doesn't matter anymore at this point.
> 
> - when encapsulating or decapsulating IPsec packets. We want to continue
>   tracing these packets after IPsec processing.
> 
> - when passing packets through virtual network devices. Only devices on
>   that encapsulate in IPv4/v6 matter since otherwise nf_trace is not
>   used anymore. Its not entirely clear whether those packets should
>   be traced after that, however we've always done that.
> 
> - when passing packets through virtual network devices that make the
>   packet cross network namespace boundaries. This is the only cases
>   where we clearly want to reset nf_trace and is also what the
>   original patch intended to fix.
> 
> Add a new function nf_reset_trace() and use it in dev_forward_skb() to
> fix this properly.
> 
> Signed-off-by: Patrick McHardy <kaber@trash.net>

I'll apply this directly because I'm trying to get a pull request out
to Linus.

^ permalink raw reply

* Re: [PATCH] netfilter: don't reset nf_trace in nf_reset()
From: Sergei Shtylyov @ 2013-04-05 19:14 UTC (permalink / raw)
  To: Patrick McHardy; +Cc: pablo, netfilter-devel, netdev, gaofeng
In-Reply-To: <1365187325-25147-1-git-send-email-kaber@trash.net>

On 04/05/2013 10:42 PM, Patrick McHardy wrote:

> Commit 130549fe

    Please also specify the summary line of this commit in parens.
David M. also seems to require it to be enclosed in quotes inside parens...

> added code to reset nf_trace in nf_reset(). This is wrong
> and unnecessary.
>
> nf_reset() is used in the following cases:
>
> - when passing packets up the the socket layer, at which point we want to
>    release all netfilter references that might keep modules pinned while
>    the packet is queued. nf_trace doesn't matter anymore at this point.
>
> - when encapsulating or decapsulating IPsec packets. We want to continue
>    tracing these packets after IPsec processing.
>
> - when passing packets through virtual network devices. Only devices on
>    that encapsulate in IPv4/v6 matter since otherwise nf_trace is not
>    used anymore. Its not entirely clear whether those packets should
>    be traced after that, however we've always done that.
>
> - when passing packets through virtual network devices that make the
>    packet cross network namespace boundaries. This is the only cases
>    where we clearly want to reset nf_trace and is also what the
>    original patch intended to fix.
>
> Add a new function nf_reset_trace() and use it in dev_forward_skb() to
> fix this properly.
>
> Signed-off-by: Patrick McHardy <kaber@trash.net>
>

WBR, Sergei

^ permalink raw reply

* Re: [PATCH] netfilter: don't reset nf_trace in nf_reset()
From: Patrick McHardy @ 2013-04-05 19:20 UTC (permalink / raw)
  To: Sergei Shtylyov; +Cc: pablo, netfilter-devel, netdev, gaofeng
In-Reply-To: <515F2296.6070001@cogentembedded.com>



Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> schrieb:

>On 04/05/2013 10:42 PM, Patrick McHardy wrote:
>
>> Commit 130549fe
>
>    Please also specify the summary line of this commit in parens.
>David M. also seems to require it to be enclosed in quotes inside
>parens...

I thought I did. Well, apparently I forgot.


^ permalink raw reply

* [PATCH 0/3] nfc: standardize logging styles
From: Joe Perches @ 2013-04-05 19:27 UTC (permalink / raw)
  To: Paul Bolle
  Cc: Lauro Ramos Venancio, Aloisio Almeida Jr, Samuel Ortiz,
	linux-wireless, linux-nfc, linux-kernel, netdev
In-Reply-To: <1365180413.2075.5.camel@joe-AO722>

Fix some defects in the logging too

Joe Perches (3):
  nfc: Replace nfc_dev_dbg with dev_dbg, remove function tracing
  nfc: Convert nfc_dev_info and nfc_dev_err to nfc_<level>
  nfc: Standardize logging style

 drivers/nfc/microread/i2c.c       |  32 ++---
 drivers/nfc/microread/mei.c       |  20 +--
 drivers/nfc/microread/microread.c |   7 +-
 drivers/nfc/nfcwilink.c           |  95 ++++++-------
 drivers/nfc/pn533.c               | 271 ++++++++++++++++----------------------
 drivers/nfc/pn544/i2c.c           |  38 +++---
 drivers/nfc/pn544/pn544.c         |  13 +-
 include/net/nfc/nfc.h             |   5 +-
 8 files changed, 196 insertions(+), 285 deletions(-)

-- 
1.8.1.2.459.gbcd45b4.dirty

^ permalink raw reply

* [PATCH 1/3] nfc: Replace nfc_dev_dbg with dev_dbg, remove function tracing
From: Joe Perches @ 2013-04-05 19:27 UTC (permalink / raw)
  To: Paul Bolle
  Cc: Lauro Ramos Venancio, Aloisio Almeida Jr, Samuel Ortiz,
	David S. Miller, linux-wireless, linux-nfc, linux-kernel, netdev
In-Reply-To: <cover.1365189658.git.joe@perches.com>

Use the generic kernel function instead of a home-grown
one that does the same thing.  Remove empty function
tracing style uses as the kernel function tracer does
that too.

Add \n to uses not at the macro.  Don't add \n where
the nfc_dev_dbg macro mistakenly had them already.

Signed-off-by: Joe Perches <joe@perches.com>
---
 drivers/nfc/nfcwilink.c |  51 +++++++--------------
 drivers/nfc/pn533.c     | 118 ++++++++++++++++--------------------------------
 include/net/nfc/nfc.h   |   1 -
 3 files changed, 56 insertions(+), 114 deletions(-)

diff --git a/drivers/nfc/nfcwilink.c b/drivers/nfc/nfcwilink.c
index 3b731ac..b2f4040 100644
--- a/drivers/nfc/nfcwilink.c
+++ b/drivers/nfc/nfcwilink.c
@@ -146,8 +146,6 @@ static int nfcwilink_get_bts_file_name(struct nfcwilink *drv, char *file_name)
 	unsigned long comp_ret;
 	int rc;
 
-	nfc_dev_dbg(&drv->pdev->dev, "get_bts_file_name entry");
-
 	skb = nfcwilink_skb_alloc(sizeof(struct nci_vs_nfcc_info_cmd),
 					GFP_KERNEL);
 	if (!skb) {
@@ -172,17 +170,16 @@ static int nfcwilink_get_bts_file_name(struct nfcwilink *drv, char *file_name)
 
 	comp_ret = wait_for_completion_timeout(&drv->completed,
 				msecs_to_jiffies(NFCWILINK_CMD_TIMEOUT));
-	nfc_dev_dbg(&drv->pdev->dev, "wait_for_completion_timeout returned %ld",
-			comp_ret);
+	dev_dbg(&drv->pdev->dev, "wait_for_completion_timeout returned %ld\n",
+		comp_ret);
 	if (comp_ret == 0) {
-		nfc_dev_err(&drv->pdev->dev,
-				"timeout on wait_for_completion_timeout");
+		dev_err(&drv->pdev->dev,
+			"timeout on wait_for_completion_timeout\n");
 		return -ETIMEDOUT;
 	}
 
-	nfc_dev_dbg(&drv->pdev->dev, "nci_vs_nfcc_info_rsp: plen %d, status %d",
-			drv->nfcc_info.plen,
-			drv->nfcc_info.status);
+	dev_dbg(&drv->pdev->dev, "nci_vs_nfcc_info_rsp: plen %d, status %d\n",
+		drv->nfcc_info.plen, drv->nfcc_info.status);
 
 	if ((drv->nfcc_info.plen != 5) || (drv->nfcc_info.status != 0)) {
 		nfc_dev_err(&drv->pdev->dev,
@@ -209,8 +206,6 @@ static int nfcwilink_send_bts_cmd(struct nfcwilink *drv, __u8 *data, int len)
 	unsigned long comp_ret;
 	int rc;
 
-	nfc_dev_dbg(&drv->pdev->dev, "send_bts_cmd entry");
-
 	/* verify valid cmd for the NFC channel */
 	if ((len <= sizeof(struct nfcwilink_hdr)) ||
 		(len > BTS_FILE_CMD_MAX_LEN) ||
@@ -242,8 +237,8 @@ static int nfcwilink_send_bts_cmd(struct nfcwilink *drv, __u8 *data, int len)
 
 	comp_ret = wait_for_completion_timeout(&drv->completed,
 				msecs_to_jiffies(NFCWILINK_CMD_TIMEOUT));
-	nfc_dev_dbg(&drv->pdev->dev, "wait_for_completion_timeout returned %ld",
-			comp_ret);
+	dev_dbg(&drv->pdev->dev, "wait_for_completion_timeout returned %ld\n",
+		comp_ret);
 	if (comp_ret == 0) {
 		nfc_dev_err(&drv->pdev->dev,
 				"timeout on wait_for_completion_timeout");
@@ -261,8 +256,6 @@ static int nfcwilink_download_fw(struct nfcwilink *drv)
 	__u8 *ptr;
 	int len, rc;
 
-	nfc_dev_dbg(&drv->pdev->dev, "download_fw entry");
-
 	set_bit(NFCWILINK_FW_DOWNLOAD, &drv->flags);
 
 	rc = nfcwilink_get_bts_file_name(drv, file_name);
@@ -284,8 +277,8 @@ static int nfcwilink_download_fw(struct nfcwilink *drv)
 	ptr = (__u8 *)fw->data;
 
 	if ((len == 0) || (ptr == NULL)) {
-		nfc_dev_dbg(&drv->pdev->dev,
-				"request_firmware returned size %d", len);
+		dev_dbg(&drv->pdev->dev,
+			"request_firmware returned size %d\n", len);
 		goto release_fw;
 	}
 
@@ -306,8 +299,8 @@ static int nfcwilink_download_fw(struct nfcwilink *drv)
 		action_len =
 			__le16_to_cpu(((struct bts_file_action *)ptr)->len);
 
-		nfc_dev_dbg(&drv->pdev->dev, "bts_file_action type %d, len %d",
-				action_type, action_len);
+		dev_dbg(&drv->pdev->dev, "bts_file_action type %d, len %d\n",
+			action_type, action_len);
 
 		switch (action_type) {
 		case BTS_FILE_ACTION_TYPE_SEND_CMD:
@@ -337,8 +330,6 @@ static void nfcwilink_register_complete(void *priv_data, char data)
 {
 	struct nfcwilink *drv = priv_data;
 
-	nfc_dev_dbg(&drv->pdev->dev, "register_complete entry");
-
 	/* store ST registration status */
 	drv->st_register_cb_status = data;
 
@@ -360,7 +351,7 @@ static long nfcwilink_receive(void *priv_data, struct sk_buff *skb)
 		return -EFAULT;
 	}
 
-	nfc_dev_dbg(&drv->pdev->dev, "receive entry, len %d", skb->len);
+	dev_dbg(&drv->pdev->dev, "receive entry, len %d\n", skb->len);
 
 	/* strip the ST header
 	(apart for the chnl byte, which is not received in the hdr) */
@@ -402,8 +393,6 @@ static int nfcwilink_open(struct nci_dev *ndev)
 	unsigned long comp_ret;
 	int rc;
 
-	nfc_dev_dbg(&drv->pdev->dev, "open entry");
-
 	if (test_and_set_bit(NFCWILINK_RUNNING, &drv->flags)) {
 		rc = -EBUSY;
 		goto exit;
@@ -421,9 +410,9 @@ static int nfcwilink_open(struct nci_dev *ndev)
 			&drv->completed,
 			msecs_to_jiffies(NFCWILINK_REGISTER_TIMEOUT));
 
-			nfc_dev_dbg(&drv->pdev->dev,
-			"wait_for_completion_timeout returned %ld",
-			comp_ret);
+			dev_dbg(&drv->pdev->dev,
+				"wait_for_completion_timeout returned %ld\n",
+				comp_ret);
 
 			if (comp_ret == 0) {
 				/* timeout */
@@ -466,8 +455,6 @@ static int nfcwilink_close(struct nci_dev *ndev)
 	struct nfcwilink *drv = nci_get_drvdata(ndev);
 	int rc;
 
-	nfc_dev_dbg(&drv->pdev->dev, "close entry");
-
 	if (!test_and_clear_bit(NFCWILINK_RUNNING, &drv->flags))
 		return 0;
 
@@ -487,7 +474,7 @@ static int nfcwilink_send(struct sk_buff *skb)
 	struct nfcwilink_hdr hdr = {NFCWILINK_CHNL, NFCWILINK_OPCODE, 0x0000};
 	long len;
 
-	nfc_dev_dbg(&drv->pdev->dev, "send entry, len %d", skb->len);
+	dev_dbg(&drv->pdev->dev, "send entry, len %d\n", skb->len);
 
 	if (!test_bit(NFCWILINK_RUNNING, &drv->flags)) {
 		kfree_skb(skb);
@@ -524,8 +511,6 @@ static int nfcwilink_probe(struct platform_device *pdev)
 	int rc;
 	__u32 protocols;
 
-	nfc_dev_dbg(&pdev->dev, "probe entry");
-
 	drv = devm_kzalloc(&pdev->dev, sizeof(struct nfcwilink), GFP_KERNEL);
 	if (!drv) {
 		rc = -ENOMEM;
@@ -576,8 +561,6 @@ static int nfcwilink_remove(struct platform_device *pdev)
 	struct nfcwilink *drv = dev_get_drvdata(&pdev->dev);
 	struct nci_dev *ndev;
 
-	nfc_dev_dbg(&pdev->dev, "remove entry");
-
 	if (!drv)
 		return -EFAULT;
 
diff --git a/drivers/nfc/pn533.c b/drivers/nfc/pn533.c
index f0f6763..e2956a6 100644
--- a/drivers/nfc/pn533.c
+++ b/drivers/nfc/pn533.c
@@ -527,9 +527,9 @@ static void pn533_recv_response(struct urb *urb)
 		break; /* success */
 	case -ECONNRESET:
 	case -ENOENT:
-		nfc_dev_dbg(&dev->interface->dev,
-			    "The urb has been canceled (status %d)",
-			    urb->status);
+		dev_dbg(&dev->interface->dev,
+			"The urb has been canceled (status %d)\n",
+			urb->status);
 		dev->wq_in_error = urb->status;
 		goto sched_wq;
 	case -ESHUTDOWN:
@@ -542,7 +542,7 @@ static void pn533_recv_response(struct urb *urb)
 
 	in_frame = dev->in_urb->transfer_buffer;
 
-	nfc_dev_dbg(&dev->interface->dev, "Received a frame.");
+	dev_dbg(&dev->interface->dev, "Received a frame\n");
 	print_hex_dump(KERN_DEBUG, "PN533 RX: ", DUMP_PREFIX_NONE, 16, 1,
 		       in_frame, dev->ops->rx_frame_size(in_frame), false);
 
@@ -583,9 +583,9 @@ static void pn533_recv_ack(struct urb *urb)
 		break; /* success */
 	case -ECONNRESET:
 	case -ENOENT:
-		nfc_dev_dbg(&dev->interface->dev,
-			    "The urb has been stopped (status %d)",
-			    urb->status);
+		dev_dbg(&dev->interface->dev,
+			"The urb has been stopped (status %d)\n",
+			urb->status);
 		dev->wq_in_error = urb->status;
 		goto sched_wq;
 	case -ESHUTDOWN:
@@ -631,8 +631,6 @@ static int pn533_send_ack(struct pn533 *dev, gfp_t flags)
 	/* spec 7.1.1.3:  Preamble, SoPC (2), ACK Code (2), Postamble */
 	int rc;
 
-	nfc_dev_dbg(&dev->interface->dev, "%s", __func__);
-
 	dev->out_urb->transfer_buffer = ack;
 	dev->out_urb->transfer_buffer_length = sizeof(ack);
 	rc = usb_submit_urb(dev->out_urb, flags);
@@ -739,7 +737,7 @@ static int __pn533_send_async(struct pn533 *dev, u8 cmd_code,
 	struct pn533_send_async_complete_arg *arg;
 	int rc = 0;
 
-	nfc_dev_dbg(&dev->interface->dev, "Sending command 0x%x", cmd_code);
+	dev_dbg(&dev->interface->dev, "Sending command 0x%x\n", cmd_code);
 
 	arg = kzalloc(sizeof(*arg), GFP_KERNEL);
 	if (!arg)
@@ -764,8 +762,8 @@ static int __pn533_send_async(struct pn533 *dev, u8 cmd_code,
 		goto unlock;
 	}
 
-	nfc_dev_dbg(&dev->interface->dev, "%s Queueing command 0x%x", __func__,
-		    cmd_code);
+	dev_dbg(&dev->interface->dev, "%s Queueing command 0x%x\n",
+		__func__, cmd_code);
 
 	cmd = kzalloc(sizeof(struct pn533_cmd), GFP_KERNEL);
 	if (!cmd) {
@@ -971,9 +969,9 @@ static void pn533_send_complete(struct urb *urb)
 		break; /* success */
 	case -ECONNRESET:
 	case -ENOENT:
-		nfc_dev_dbg(&dev->interface->dev,
-			    "The urb has been stopped (status %d)",
-			    urb->status);
+		dev_dbg(&dev->interface->dev,
+			"The urb has been stopped (status %d)\n",
+			urb->status);
 		break;
 	case -ESHUTDOWN:
 	default:
@@ -1235,8 +1233,8 @@ static int pn533_target_found(struct pn533 *dev, u8 tg, u8 *tgdata,
 	struct nfc_target nfc_tgt;
 	int rc;
 
-	nfc_dev_dbg(&dev->interface->dev, "%s - modulation=%d", __func__,
-		    dev->poll_mod_curr);
+	dev_dbg(&dev->interface->dev, "%s - modulation=%d\n",
+		__func__, dev->poll_mod_curr);
 
 	if (tg != 1)
 		return -EPROTO;
@@ -1267,14 +1265,14 @@ static int pn533_target_found(struct pn533 *dev, u8 tg, u8 *tgdata,
 		return rc;
 
 	if (!(nfc_tgt.supported_protocols & dev->poll_protocols)) {
-		nfc_dev_dbg(&dev->interface->dev,
-			    "The Tg found doesn't have the desired protocol");
+		dev_dbg(&dev->interface->dev,
+			"The Tg found doesn't have the desired protocol\n");
 		return -EAGAIN;
 	}
 
-	nfc_dev_dbg(&dev->interface->dev,
-		    "Target found - supported protocols: 0x%x",
-		    nfc_tgt.supported_protocols);
+	dev_dbg(&dev->interface->dev,
+		"Target found - supported protocols: 0x%x\n",
+		nfc_tgt.supported_protocols);
 
 	dev->tgt_available_prots = nfc_tgt.supported_protocols;
 
@@ -1331,8 +1329,6 @@ static int pn533_start_poll_complete(struct pn533 *dev, struct sk_buff *resp)
 	u8 nbtg, tg, *tgdata;
 	int rc, tgdata_len;
 
-	nfc_dev_dbg(&dev->interface->dev, "%s", __func__);
-
 	nbtg = resp->data[0];
 	tg = resp->data[1];
 	tgdata = &resp->data[2];
@@ -1412,8 +1408,6 @@ static int pn533_tm_get_data_complete(struct pn533 *dev, void *arg,
 {
 	u8 status;
 
-	nfc_dev_dbg(&dev->interface->dev, "%s", __func__);
-
 	if (IS_ERR(resp))
 		return PTR_ERR(resp);
 
@@ -1437,8 +1431,6 @@ static void pn533_wq_tg_get_data(struct work_struct *work)
 	struct sk_buff *skb;
 	int rc;
 
-	nfc_dev_dbg(&dev->interface->dev, "%s", __func__);
-
 	skb = pn533_alloc_skb(dev, 0);
 	if (!skb)
 		return;
@@ -1459,16 +1451,14 @@ static int pn533_init_target_complete(struct pn533 *dev, struct sk_buff *resp)
 	size_t gb_len;
 	int rc;
 
-	nfc_dev_dbg(&dev->interface->dev, "%s", __func__);
-
 	if (resp->len < ATR_REQ_GB_OFFSET + 1)
 		return -EINVAL;
 
 	mode = resp->data[0];
 	cmd = &resp->data[1];
 
-	nfc_dev_dbg(&dev->interface->dev, "Target mode 0x%x len %d\n",
-		    mode, resp->len);
+	dev_dbg(&dev->interface->dev, "Target mode 0x%x len %d\n",
+		mode, resp->len);
 
 	if ((mode & PN533_INIT_TARGET_RESP_FRAME_MASK) ==
 	    PN533_INIT_TARGET_RESP_ACTIVE)
@@ -1498,8 +1488,6 @@ static void pn533_listen_mode_timer(unsigned long data)
 {
 	struct pn533 *dev = (struct pn533 *)data;
 
-	nfc_dev_dbg(&dev->interface->dev, "Listen mode timeout");
-
 	/* An ack will cancel the last issued command (poll) */
 	pn533_send_ack(dev, GFP_ATOMIC);
 
@@ -1516,8 +1504,6 @@ static int pn533_poll_complete(struct pn533 *dev, void *arg,
 	struct pn533_poll_modulations *cur_mod;
 	int rc;
 
-	nfc_dev_dbg(&dev->interface->dev, "%s", __func__);
-
 	if (IS_ERR(resp)) {
 		rc = PTR_ERR(resp);
 
@@ -1587,8 +1573,8 @@ static int pn533_send_poll_frame(struct pn533 *dev)
 
 	mod = dev->poll_mod_active[dev->poll_mod_curr];
 
-	nfc_dev_dbg(&dev->interface->dev, "%s mod len %d\n",
-		    __func__, mod->len);
+	dev_dbg(&dev->interface->dev, "%s mod len %d\n",
+		__func__, mod->len);
 
 	if (mod->len == 0) {  /* Listen mode */
 		cmd_code = PN533_CMD_TG_INIT_AS_TARGET;
@@ -1621,9 +1607,9 @@ static void pn533_wq_poll(struct work_struct *work)
 
 	cur_mod = dev->poll_mod_active[dev->poll_mod_curr];
 
-	nfc_dev_dbg(&dev->interface->dev,
-		    "%s cancel_listen %d modulation len %d",
-		    __func__, dev->cancel_listen, cur_mod->len);
+	dev_dbg(&dev->interface->dev,
+		"%s cancel_listen %d modulation len %d\n",
+		__func__, dev->cancel_listen, cur_mod->len);
 
 	if (dev->cancel_listen == 1) {
 		dev->cancel_listen = 0;
@@ -1645,9 +1631,9 @@ static int pn533_start_poll(struct nfc_dev *nfc_dev,
 {
 	struct pn533 *dev = nfc_get_drvdata(nfc_dev);
 
-	nfc_dev_dbg(&dev->interface->dev,
-		    "%s: im protocols 0x%x tm protocols 0x%x",
-		    __func__, im_protocols, tm_protocols);
+	dev_dbg(&dev->interface->dev,
+		"%s: im protocols 0x%x tm protocols 0x%x\n",
+		__func__, im_protocols, tm_protocols);
 
 	if (dev->tgt_active_prot) {
 		nfc_dev_err(&dev->interface->dev,
@@ -1679,13 +1665,11 @@ static void pn533_stop_poll(struct nfc_dev *nfc_dev)
 {
 	struct pn533 *dev = nfc_get_drvdata(nfc_dev);
 
-	nfc_dev_dbg(&dev->interface->dev, "%s", __func__);
-
 	del_timer(&dev->listen_timer);
 
 	if (!dev->poll_mod_count) {
-		nfc_dev_dbg(&dev->interface->dev,
-			    "Polling operation was not running");
+		dev_dbg(&dev->interface->dev,
+			"Polling operation was not running\n");
 		return;
 	}
 
@@ -1707,8 +1691,6 @@ static int pn533_activate_target_nfcdep(struct pn533 *dev)
 	struct sk_buff *skb;
 	struct sk_buff *resp;
 
-	nfc_dev_dbg(&dev->interface->dev, "%s", __func__);
-
 	skb = pn533_alloc_skb(dev, sizeof(u8) * 2); /*TG + Next*/
 	if (!skb)
 		return -ENOMEM;
@@ -1741,12 +1723,12 @@ static int pn533_activate_target(struct nfc_dev *nfc_dev,
 	struct pn533 *dev = nfc_get_drvdata(nfc_dev);
 	int rc;
 
-	nfc_dev_dbg(&dev->interface->dev, "%s - protocol=%u", __func__,
-		    protocol);
+	dev_dbg(&dev->interface->dev, "%s - protocol=%u\n",
+		__func__, protocol);
 
 	if (dev->poll_mod_count) {
-		nfc_dev_err(&dev->interface->dev,
-			    "Cannot activate while polling");
+		dev_err(&dev->interface->dev,
+			"Cannot activate while polling\n");
 		return -EBUSY;
 	}
 
@@ -1794,8 +1776,6 @@ static void pn533_deactivate_target(struct nfc_dev *nfc_dev,
 
 	int rc;
 
-	nfc_dev_dbg(&dev->interface->dev, "%s", __func__);
-
 	if (!dev->tgt_active_prot) {
 		nfc_dev_err(&dev->interface->dev, "There is no active target");
 		return;
@@ -1857,7 +1837,7 @@ static int pn533_in_dep_link_up_complete(struct pn533 *dev, void *arg,
 	if (!dev->tgt_available_prots) {
 		struct nfc_target nfc_target;
 
-		nfc_dev_dbg(&dev->interface->dev, "Creating new target");
+		dev_dbg(&dev->interface->dev, "Creating new target\n");
 
 		nfc_target.supported_protocols = NFC_PROTO_NFC_DEP_MASK;
 		nfc_target.nfcid1_len = 10;
@@ -1910,8 +1890,6 @@ static int pn533_dep_link_up(struct nfc_dev *nfc_dev, struct nfc_target *target,
 
 	u8 passive_data[PASSIVE_DATA_LEN] = {0x00, 0xff, 0xff, 0x00, 0x3};
 
-	nfc_dev_dbg(&dev->interface->dev, "%s", __func__);
-
 	if (dev->poll_mod_count) {
 		nfc_dev_err(&dev->interface->dev,
 			    "Cannot bring the DEP link up while polling");
@@ -1981,8 +1959,6 @@ static int pn533_dep_link_down(struct nfc_dev *nfc_dev)
 {
 	struct pn533 *dev = nfc_get_drvdata(nfc_dev);
 
-	nfc_dev_dbg(&dev->interface->dev, "%s", __func__);
-
 	pn533_poll_reset_mod_list(dev);
 
 	if (dev->tgt_mode || dev->tgt_active_prot) {
@@ -2008,8 +1984,6 @@ static struct sk_buff *pn533_build_response(struct pn533 *dev)
 	struct sk_buff *skb, *tmp, *t;
 	unsigned int skb_len = 0, tmp_len = 0;
 
-	nfc_dev_dbg(&dev->interface->dev, "%s", __func__);
-
 	if (skb_queue_empty(&dev->resp_q))
 		return NULL;
 
@@ -2021,8 +1995,8 @@ static struct sk_buff *pn533_build_response(struct pn533 *dev)
 	skb_queue_walk_safe(&dev->resp_q, tmp, t)
 		skb_len += tmp->len;
 
-	nfc_dev_dbg(&dev->interface->dev, "%s total length %d\n",
-		    __func__, skb_len);
+	dev_dbg(&dev->interface->dev, "%s total length %d\n",
+		__func__, skb_len);
 
 	skb = alloc_skb(skb_len, GFP_KERNEL);
 	if (skb == NULL)
@@ -2049,8 +2023,6 @@ static int pn533_data_exchange_complete(struct pn533 *dev, void *_arg,
 	int rc = 0;
 	u8 status, ret, mi;
 
-	nfc_dev_dbg(&dev->interface->dev, "%s", __func__);
-
 	if (IS_ERR(resp)) {
 		rc = PTR_ERR(resp);
 		goto _error;
@@ -2103,8 +2075,6 @@ static int pn533_transceive(struct nfc_dev *nfc_dev,
 	struct pn533_data_exchange_arg *arg = NULL;
 	int rc;
 
-	nfc_dev_dbg(&dev->interface->dev, "%s", __func__);
-
 	if (skb->len > PN533_CMD_DATAEXCH_DATA_MAXLEN) {
 		/* TODO: Implement support to multi-part data exchange */
 		nfc_dev_err(&dev->interface->dev,
@@ -2166,8 +2136,6 @@ static int pn533_tm_send_complete(struct pn533 *dev, void *arg,
 {
 	u8 status;
 
-	nfc_dev_dbg(&dev->interface->dev, "%s", __func__);
-
 	if (IS_ERR(resp))
 		return PTR_ERR(resp);
 
@@ -2193,8 +2161,6 @@ static int pn533_tm_send(struct nfc_dev *nfc_dev, struct sk_buff *skb)
 	struct pn533 *dev = nfc_get_drvdata(nfc_dev);
 	int rc;
 
-	nfc_dev_dbg(&dev->interface->dev, "%s", __func__);
-
 	if (skb->len > PN533_CMD_DATAEXCH_DATA_MAXLEN) {
 		nfc_dev_err(&dev->interface->dev,
 			    "Data length greater than the max allowed: %d",
@@ -2217,8 +2183,6 @@ static void pn533_wq_mi_recv(struct work_struct *work)
 	struct sk_buff *skb;
 	int rc;
 
-	nfc_dev_dbg(&dev->interface->dev, "%s", __func__);
-
 	skb = pn533_alloc_skb(dev, PN533_CMD_DATAEXCH_HEAD_LEN);
 	if (!skb)
 		goto error;
@@ -2268,8 +2232,6 @@ static int pn533_set_configuration(struct pn533 *dev, u8 cfgitem, u8 *cfgdata,
 
 	int skb_len;
 
-	nfc_dev_dbg(&dev->interface->dev, "%s", __func__);
-
 	skb_len = sizeof(cfgitem) + cfgdata_len; /* cfgitem + cfgdata */
 
 	skb = pn533_alloc_skb(dev, skb_len);
@@ -2315,8 +2277,6 @@ static int pn533_fw_reset(struct pn533 *dev)
 	struct sk_buff *skb;
 	struct sk_buff *resp;
 
-	nfc_dev_dbg(&dev->interface->dev, "%s", __func__);
-
 	skb = pn533_alloc_skb(dev, sizeof(u8));
 	if (!skb)
 		return -ENOMEM;
diff --git a/include/net/nfc/nfc.h b/include/net/nfc/nfc.h
index 87a6417..3448f09 100644
--- a/include/net/nfc/nfc.h
+++ b/include/net/nfc/nfc.h
@@ -30,7 +30,6 @@
 
 #define nfc_dev_info(dev, fmt, arg...) dev_info((dev), "NFC: " fmt "\n", ## arg)
 #define nfc_dev_err(dev, fmt, arg...) dev_err((dev), "NFC: " fmt "\n", ## arg)
-#define nfc_dev_dbg(dev, fmt, arg...) dev_dbg((dev), fmt "\n", ## arg)
 
 struct nfc_dev;
 
-- 
1.8.1.2.459.gbcd45b4.dirty

^ permalink raw reply related

* [PATCH 2/3] nfc: Convert nfc_dev_info and nfc_dev_err to nfc_<level>
From: Joe Perches @ 2013-04-05 19:27 UTC (permalink / raw)
  To: Paul Bolle
  Cc: Lauro Ramos Venancio, Aloisio Almeida Jr, Samuel Ortiz,
	David S. Miller, linux-wireless-u79uwXL29TY76Z2rM5mHXA,
	linux-nfc-hn68Rpc1hR1g9hUCZPvPmw,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	netdev-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <cover.1365189658.git.joe-6d6DIl74uiNBDgjK7y7TUQ@public.gmane.org>

Use a more standard kernel style macro logging name.

Standardize the spacing of the "NFC: " prefix.
Add \n to uses, remove from macro.
Fix the defective uses that already had a \n.

Signed-off-by: Joe Perches <joe-6d6DIl74uiNBDgjK7y7TUQ@public.gmane.org>
---
 drivers/nfc/nfcwilink.c |  44 +++++++-------
 drivers/nfc/pn533.c     | 156 ++++++++++++++++++++++++------------------------
 include/net/nfc/nfc.h   |   4 +-
 3 files changed, 100 insertions(+), 104 deletions(-)

diff --git a/drivers/nfc/nfcwilink.c b/drivers/nfc/nfcwilink.c
index b2f4040..f58b390 100644
--- a/drivers/nfc/nfcwilink.c
+++ b/drivers/nfc/nfcwilink.c
@@ -149,8 +149,8 @@ static int nfcwilink_get_bts_file_name(struct nfcwilink *drv, char *file_name)
 	skb = nfcwilink_skb_alloc(sizeof(struct nci_vs_nfcc_info_cmd),
 					GFP_KERNEL);
 	if (!skb) {
-		nfc_dev_err(&drv->pdev->dev,
-				"no memory for nci_vs_nfcc_info_cmd");
+		nfc_err(&drv->pdev->dev,
+			"no memory for nci_vs_nfcc_info_cmd\n");
 		return -ENOMEM;
 	}
 
@@ -182,8 +182,7 @@ static int nfcwilink_get_bts_file_name(struct nfcwilink *drv, char *file_name)
 		drv->nfcc_info.plen, drv->nfcc_info.status);
 
 	if ((drv->nfcc_info.plen != 5) || (drv->nfcc_info.status != 0)) {
-		nfc_dev_err(&drv->pdev->dev,
-				"invalid nci_vs_nfcc_info_rsp");
+		nfc_err(&drv->pdev->dev, "invalid nci_vs_nfcc_info_rsp\n");
 		return -EINVAL;
 	}
 
@@ -194,7 +193,7 @@ static int nfcwilink_get_bts_file_name(struct nfcwilink *drv, char *file_name)
 			drv->nfcc_info.sw_ver_z,
 			drv->nfcc_info.patch_id);
 
-	nfc_dev_info(&drv->pdev->dev, "nfcwilink FW file name: %s", file_name);
+	nfc_info(&drv->pdev->dev, "nfcwilink FW file name: %s\n", file_name);
 
 	return 0;
 }
@@ -211,8 +210,8 @@ static int nfcwilink_send_bts_cmd(struct nfcwilink *drv, __u8 *data, int len)
 		(len > BTS_FILE_CMD_MAX_LEN) ||
 		(hdr->chnl != NFCWILINK_CHNL) ||
 		(hdr->opcode != NFCWILINK_OPCODE)) {
-		nfc_dev_err(&drv->pdev->dev,
-			"ignoring invalid bts cmd, len %d, chnl %d, opcode %d",
+		nfc_err(&drv->pdev->dev,
+			"ignoring invalid bts cmd, len %d, chnl %d, opcode %d\n",
 			len, hdr->chnl, hdr->opcode);
 		return 0;
 	}
@@ -223,7 +222,7 @@ static int nfcwilink_send_bts_cmd(struct nfcwilink *drv, __u8 *data, int len)
 
 	skb = nfcwilink_skb_alloc(len, GFP_KERNEL);
 	if (!skb) {
-		nfc_dev_err(&drv->pdev->dev, "no memory for bts cmd");
+		nfc_err(&drv->pdev->dev, "no memory for bts cmd\n");
 		return -ENOMEM;
 	}
 
@@ -240,8 +239,8 @@ static int nfcwilink_send_bts_cmd(struct nfcwilink *drv, __u8 *data, int len)
 	dev_dbg(&drv->pdev->dev, "wait_for_completion_timeout returned %ld\n",
 		comp_ret);
 	if (comp_ret == 0) {
-		nfc_dev_err(&drv->pdev->dev,
-				"timeout on wait_for_completion_timeout");
+		nfc_err(&drv->pdev->dev,
+			"timeout on wait_for_completion_timeout\n");
 		return -ETIMEDOUT;
 	}
 
@@ -264,7 +263,7 @@ static int nfcwilink_download_fw(struct nfcwilink *drv)
 
 	rc = request_firmware(&fw, file_name, &drv->pdev->dev);
 	if (rc) {
-		nfc_dev_err(&drv->pdev->dev, "request_firmware failed %d", rc);
+		nfc_err(&drv->pdev->dev, "request_firmware failed %d\n", rc);
 
 		/* if the file is not found, don't exit with failure */
 		if (rc == -ENOENT)
@@ -284,7 +283,7 @@ static int nfcwilink_download_fw(struct nfcwilink *drv)
 
 	if (__le32_to_cpu(((struct bts_file_hdr *)ptr)->magic) !=
 			BTS_FILE_HDR_MAGIC) {
-		nfc_dev_err(&drv->pdev->dev, "wrong bts magic number");
+		nfc_err(&drv->pdev->dev, "wrong bts magic number\n");
 		rc = -EINVAL;
 		goto release_fw;
 	}
@@ -367,7 +366,7 @@ static long nfcwilink_receive(void *priv_data, struct sk_buff *skb)
 	/* Forward skb to NCI core layer */
 	rc = nci_recv_frame(skb);
 	if (rc < 0) {
-		nfc_dev_err(&drv->pdev->dev, "nci_recv_frame failed %d", rc);
+		nfc_err(&drv->pdev->dev, "nci_recv_frame failed %d\n", rc);
 		return rc;
 	}
 
@@ -420,13 +419,12 @@ static int nfcwilink_open(struct nci_dev *ndev)
 				goto clear_exit;
 			} else if (drv->st_register_cb_status != 0) {
 				rc = drv->st_register_cb_status;
-				nfc_dev_err(&drv->pdev->dev,
-				"st_register_cb failed %d", rc);
+				nfc_err(&drv->pdev->dev,
+					"st_register_cb failed %d\n", rc);
 				goto clear_exit;
 			}
 		} else {
-			nfc_dev_err(&drv->pdev->dev,
-				"st_register failed %d", rc);
+			nfc_err(&drv->pdev->dev, "st_register failed %d\n", rc);
 			goto clear_exit;
 		}
 	}
@@ -436,8 +434,8 @@ static int nfcwilink_open(struct nci_dev *ndev)
 	drv->st_write = nfcwilink_proto.write;
 
 	if (nfcwilink_download_fw(drv)) {
-		nfc_dev_err(&drv->pdev->dev, "nfcwilink_download_fw failed %d",
-				rc);
+		nfc_err(&drv->pdev->dev, "nfcwilink_download_fw failed %d\n",
+			rc);
 		/* open should succeed, even if the FW download failed */
 	}
 
@@ -460,7 +458,7 @@ static int nfcwilink_close(struct nci_dev *ndev)
 
 	rc = st_unregister(&nfcwilink_proto);
 	if (rc)
-		nfc_dev_err(&drv->pdev->dev, "st_unregister failed %d", rc);
+		nfc_err(&drv->pdev->dev, "st_unregister failed %d\n", rc);
 
 	drv->st_write = NULL;
 
@@ -492,7 +490,7 @@ static int nfcwilink_send(struct sk_buff *skb)
 	len = drv->st_write(skb);
 	if (len < 0) {
 		kfree_skb(skb);
-		nfc_dev_err(&drv->pdev->dev, "st_write failed %ld", len);
+		nfc_err(&drv->pdev->dev, "st_write failed %ld\n", len);
 		return -EFAULT;
 	}
 
@@ -531,7 +529,7 @@ static int nfcwilink_probe(struct platform_device *pdev)
 					NFCWILINK_HDR_LEN,
 					0);
 	if (!drv->ndev) {
-		nfc_dev_err(&pdev->dev, "nci_allocate_device failed");
+		nfc_err(&pdev->dev, "nci_allocate_device failed\n");
 		rc = -ENOMEM;
 		goto exit;
 	}
@@ -541,7 +539,7 @@ static int nfcwilink_probe(struct platform_device *pdev)
 
 	rc = nci_register_device(drv->ndev);
 	if (rc < 0) {
-		nfc_dev_err(&pdev->dev, "nci_register_device failed %d", rc);
+		nfc_err(&pdev->dev, "nci_register_device failed %d\n", rc);
 		goto free_dev_exit;
 	}
 
diff --git a/drivers/nfc/pn533.c b/drivers/nfc/pn533.c
index e2956a6..714b4ec 100644
--- a/drivers/nfc/pn533.c
+++ b/drivers/nfc/pn533.c
@@ -534,8 +534,8 @@ static void pn533_recv_response(struct urb *urb)
 		goto sched_wq;
 	case -ESHUTDOWN:
 	default:
-		nfc_dev_err(&dev->interface->dev,
-			    "Urb failure (status %d)", urb->status);
+		nfc_err(&dev->interface->dev, "Urb failure (status %d)\n",
+			urb->status);
 		dev->wq_in_error = urb->status;
 		goto sched_wq;
 	}
@@ -547,14 +547,14 @@ static void pn533_recv_response(struct urb *urb)
 		       in_frame, dev->ops->rx_frame_size(in_frame), false);
 
 	if (!dev->ops->rx_is_frame_valid(in_frame)) {
-		nfc_dev_err(&dev->interface->dev, "Received an invalid frame");
+		nfc_err(&dev->interface->dev, "Received an invalid frame\n");
 		dev->wq_in_error = -EIO;
 		goto sched_wq;
 	}
 
 	if (!pn533_rx_frame_is_cmd_response(dev, in_frame)) {
-		nfc_dev_err(&dev->interface->dev,
-			    "It it not the response to the last command");
+		nfc_err(&dev->interface->dev,
+			"It it not the response to the last command\n");
 		dev->wq_in_error = -EIO;
 		goto sched_wq;
 	}
@@ -590,8 +590,8 @@ static void pn533_recv_ack(struct urb *urb)
 		goto sched_wq;
 	case -ESHUTDOWN:
 	default:
-		nfc_dev_err(&dev->interface->dev,
-			    "Urb failure (status %d)", urb->status);
+		nfc_err(&dev->interface->dev, "Urb failure (status %d)\n",
+			urb->status);
 		dev->wq_in_error = urb->status;
 		goto sched_wq;
 	}
@@ -599,15 +599,15 @@ static void pn533_recv_ack(struct urb *urb)
 	in_frame = dev->in_urb->transfer_buffer;
 
 	if (!pn533_rx_frame_is_ack(in_frame)) {
-		nfc_dev_err(&dev->interface->dev, "Received an invalid ack");
+		nfc_err(&dev->interface->dev, "Received an invalid ack\n");
 		dev->wq_in_error = -EIO;
 		goto sched_wq;
 	}
 
 	rc = pn533_submit_urb_for_response(dev, GFP_ATOMIC);
 	if (rc) {
-		nfc_dev_err(&dev->interface->dev,
-			    "usb_submit_urb failed with result %d", rc);
+		nfc_err(&dev->interface->dev,
+			"usb_submit_urb failed with result %d\n", rc);
 		dev->wq_in_error = rc;
 		goto sched_wq;
 	}
@@ -975,8 +975,8 @@ static void pn533_send_complete(struct urb *urb)
 		break;
 	case -ESHUTDOWN:
 	default:
-		nfc_dev_err(&dev->interface->dev,
-			    "Urb failure (status %d)", urb->status);
+		nfc_err(&dev->interface->dev, "Urb failure (status %d)\n",
+			urb->status);
 	}
 }
 
@@ -1256,8 +1256,8 @@ static int pn533_target_found(struct pn533 *dev, u8 tg, u8 *tgdata,
 		rc = pn533_target_found_type_b(&nfc_tgt, tgdata, tgdata_len);
 		break;
 	default:
-		nfc_dev_err(&dev->interface->dev,
-			    "Unknown current poll modulation");
+		nfc_err(&dev->interface->dev,
+			"Unknown current poll modulation\n");
 		return -EPROTO;
 	}
 
@@ -1473,8 +1473,8 @@ static int pn533_init_target_complete(struct pn533 *dev, struct sk_buff *resp)
 	rc = nfc_tm_activated(dev->nfc_dev, NFC_PROTO_NFC_DEP_MASK,
 			      comm_mode, gb, gb_len);
 	if (rc < 0) {
-		nfc_dev_err(&dev->interface->dev,
-			    "Error when signaling target activation");
+		nfc_err(&dev->interface->dev,
+			"Error when signaling target activation\n");
 		return rc;
 	}
 
@@ -1507,8 +1507,8 @@ static int pn533_poll_complete(struct pn533 *dev, void *arg,
 	if (IS_ERR(resp)) {
 		rc = PTR_ERR(resp);
 
-		nfc_dev_err(&dev->interface->dev, "%s  Poll complete error %d",
-			    __func__, rc);
+		nfc_err(&dev->interface->dev, "%s  Poll complete error %d\n",
+			__func__, rc);
 
 		if (rc == -ENOENT) {
 			if (dev->poll_mod_count != 0)
@@ -1516,8 +1516,8 @@ static int pn533_poll_complete(struct pn533 *dev, void *arg,
 			else
 				goto stop_poll;
 		} else if (rc < 0) {
-			nfc_dev_err(&dev->interface->dev,
-				    "Error %d when running poll", rc);
+			nfc_err(&dev->interface->dev,
+				"Error %d when running poll\n", rc);
 			goto stop_poll;
 		}
 	}
@@ -1543,7 +1543,7 @@ done:
 	return rc;
 
 stop_poll:
-	nfc_dev_err(&dev->interface->dev, "Polling operation has been stopped");
+	nfc_err(&dev->interface->dev, "Polling operation has been stopped\n");
 
 	pn533_poll_reset_mod_list(dev);
 	dev->poll_protocols = 0;
@@ -1585,7 +1585,7 @@ static int pn533_send_poll_frame(struct pn533 *dev)
 	}
 
 	if (!skb) {
-		nfc_dev_err(&dev->interface->dev, "Failed to allocate skb.");
+		nfc_err(&dev->interface->dev, "Failed to allocate skb\n");
 		return -ENOMEM;
 	}
 
@@ -1593,7 +1593,7 @@ static int pn533_send_poll_frame(struct pn533 *dev)
 				  NULL);
 	if (rc < 0) {
 		dev_kfree_skb(skb);
-		nfc_dev_err(&dev->interface->dev, "Polling loop error %d", rc);
+		nfc_err(&dev->interface->dev, "Polling loop error %d\n", rc);
 	}
 
 	return rc;
@@ -1636,14 +1636,14 @@ static int pn533_start_poll(struct nfc_dev *nfc_dev,
 		__func__, im_protocols, tm_protocols);
 
 	if (dev->tgt_active_prot) {
-		nfc_dev_err(&dev->interface->dev,
-			    "Cannot poll with a target already activated");
+		nfc_err(&dev->interface->dev,
+			"Cannot poll with a target already activated\n");
 		return -EBUSY;
 	}
 
 	if (dev->tgt_mode) {
-		nfc_dev_err(&dev->interface->dev,
-			    "Cannot poll while already being activated");
+		nfc_err(&dev->interface->dev,
+			"Cannot poll while already being activated\n");
 		return -EBUSY;
 	}
 
@@ -1727,35 +1727,35 @@ static int pn533_activate_target(struct nfc_dev *nfc_dev,
 		__func__, protocol);
 
 	if (dev->poll_mod_count) {
-		dev_err(&dev->interface->dev,
+		nfc_err(&dev->interface->dev,
 			"Cannot activate while polling\n");
 		return -EBUSY;
 	}
 
 	if (dev->tgt_active_prot) {
-		nfc_dev_err(&dev->interface->dev,
-			    "There is already an active target");
+		nfc_err(&dev->interface->dev,
+			"There is already an active target\n");
 		return -EBUSY;
 	}
 
 	if (!dev->tgt_available_prots) {
-		nfc_dev_err(&dev->interface->dev,
-			    "There is no available target to activate");
+		nfc_err(&dev->interface->dev,
+			"There is no available target to activate\n");
 		return -EINVAL;
 	}
 
 	if (!(dev->tgt_available_prots & (1 << protocol))) {
-		nfc_dev_err(&dev->interface->dev,
-			    "Target doesn't support requested proto %u",
-			    protocol);
+		nfc_err(&dev->interface->dev,
+			"Target doesn't support requested proto %u\n",
+			protocol);
 		return -EINVAL;
 	}
 
 	if (protocol == NFC_PROTO_NFC_DEP) {
 		rc = pn533_activate_target_nfcdep(dev);
 		if (rc) {
-			nfc_dev_err(&dev->interface->dev,
-				    "Activating target with DEP failed %d", rc);
+			nfc_err(&dev->interface->dev,
+				"Activating target with DEP failed %d\n", rc);
 			return rc;
 		}
 	}
@@ -1777,7 +1777,7 @@ static void pn533_deactivate_target(struct nfc_dev *nfc_dev,
 	int rc;
 
 	if (!dev->tgt_active_prot) {
-		nfc_dev_err(&dev->interface->dev, "There is no active target");
+		nfc_err(&dev->interface->dev, "There is no active target\n");
 		return;
 	}
 
@@ -1796,8 +1796,8 @@ static void pn533_deactivate_target(struct nfc_dev *nfc_dev,
 
 	rc = resp->data[0] & PN533_CMD_RET_MASK;
 	if (rc != PN533_CMD_RET_SUCCESS)
-		nfc_dev_err(&dev->interface->dev,
-			    "Error 0x%x when releasing the target", rc);
+		nfc_err(&dev->interface->dev,
+			"Error 0x%x when releasing the target\n", rc);
 
 	dev_kfree_skb(resp);
 	return;
@@ -1819,8 +1819,8 @@ static int pn533_in_dep_link_up_complete(struct pn533 *dev, void *arg,
 
 	if (dev->tgt_available_prots &&
 	    !(dev->tgt_available_prots & (1 << NFC_PROTO_NFC_DEP))) {
-		nfc_dev_err(&dev->interface->dev,
-			    "The target does not support DEP");
+		nfc_err(&dev->interface->dev,
+			"The target does not support DEP\n");
 		rc =  -EINVAL;
 		goto error;
 	}
@@ -1829,8 +1829,8 @@ static int pn533_in_dep_link_up_complete(struct pn533 *dev, void *arg,
 
 	rc = rsp->status & PN533_CMD_RET_MASK;
 	if (rc != PN533_CMD_RET_SUCCESS) {
-		nfc_dev_err(&dev->interface->dev,
-			    "Bringing DEP link up failed %d", rc);
+		nfc_err(&dev->interface->dev,
+			"Bringing DEP link up failed %d\n", rc);
 		goto error;
 	}
 
@@ -1891,21 +1891,21 @@ static int pn533_dep_link_up(struct nfc_dev *nfc_dev, struct nfc_target *target,
 	u8 passive_data[PASSIVE_DATA_LEN] = {0x00, 0xff, 0xff, 0x00, 0x3};
 
 	if (dev->poll_mod_count) {
-		nfc_dev_err(&dev->interface->dev,
-			    "Cannot bring the DEP link up while polling");
+		nfc_err(&dev->interface->dev,
+			"Cannot bring the DEP link up while polling\n");
 		return -EBUSY;
 	}
 
 	if (dev->tgt_active_prot) {
-		nfc_dev_err(&dev->interface->dev,
-			    "There is already an active target");
+		nfc_err(&dev->interface->dev,
+			"There is already an active target\n");
 		return -EBUSY;
 	}
 
 	baud = pn533_mod_to_baud(dev);
 	if (baud < 0) {
-		nfc_dev_err(&dev->interface->dev,
-			    "Invalid curr modulation %d", dev->poll_mod_curr);
+		nfc_err(&dev->interface->dev,
+			"Invalid curr modulation %d\n", dev->poll_mod_curr);
 		return baud;
 	}
 
@@ -2035,9 +2035,8 @@ static int pn533_data_exchange_complete(struct pn533 *dev, void *_arg,
 	skb_pull(resp, sizeof(status));
 
 	if (ret != PN533_CMD_RET_SUCCESS) {
-		nfc_dev_err(&dev->interface->dev,
-			    "PN533 reported error %d when exchanging data",
-			    ret);
+		nfc_err(&dev->interface->dev,
+			"PN533 reported error %d when exchanging data\n", ret);
 		rc = -EIO;
 		goto error;
 	}
@@ -2077,16 +2076,16 @@ static int pn533_transceive(struct nfc_dev *nfc_dev,
 
 	if (skb->len > PN533_CMD_DATAEXCH_DATA_MAXLEN) {
 		/* TODO: Implement support to multi-part data exchange */
-		nfc_dev_err(&dev->interface->dev,
-			    "Data length greater than the max allowed: %d",
-			    PN533_CMD_DATAEXCH_DATA_MAXLEN);
+		nfc_err(&dev->interface->dev,
+			"Data length greater than the max allowed: %d\n",
+			PN533_CMD_DATAEXCH_DATA_MAXLEN);
 		rc = -ENOSYS;
 		goto error;
 	}
 
 	if (!dev->tgt_active_prot) {
-		nfc_dev_err(&dev->interface->dev,
-			    "Can't exchange data if there is no active target");
+		nfc_err(&dev->interface->dev,
+			"Can't exchange data if there is no active target\n");
 		rc = -EINVAL;
 		goto error;
 	}
@@ -2162,9 +2161,9 @@ static int pn533_tm_send(struct nfc_dev *nfc_dev, struct sk_buff *skb)
 	int rc;
 
 	if (skb->len > PN533_CMD_DATAEXCH_DATA_MAXLEN) {
-		nfc_dev_err(&dev->interface->dev,
-			    "Data length greater than the max allowed: %d",
-			    PN533_CMD_DATAEXCH_DATA_MAXLEN);
+		nfc_err(&dev->interface->dev,
+			"Data length greater than the max allowed: %d\n",
+			PN533_CMD_DATAEXCH_DATA_MAXLEN);
 		return -ENOSYS;
 	}
 
@@ -2213,8 +2212,8 @@ static void pn533_wq_mi_recv(struct work_struct *work)
 	if (rc == 0) /* success */
 		return;
 
-	nfc_dev_err(&dev->interface->dev,
-		    "Error %d when trying to perform data_exchange", rc);
+	nfc_err(&dev->interface->dev,
+		"Error %d when trying to perform data_exchange\n", rc);
 
 	dev_kfree_skb(skb);
 	kfree(dev->cmd_complete_arg);
@@ -2338,16 +2337,16 @@ static int pn533_setup(struct pn533 *dev)
 		break;
 
 	default:
-		nfc_dev_err(&dev->interface->dev, "Unknown device type %d\n",
-			    dev->device_type);
+		nfc_err(&dev->interface->dev, "Unknown device type %d\n",
+			dev->device_type);
 		return -EINVAL;
 	}
 
 	rc = pn533_set_configuration(dev, PN533_CFGITEM_MAX_RETRIES,
 				     (u8 *)&max_retries, sizeof(max_retries));
 	if (rc) {
-		nfc_dev_err(&dev->interface->dev,
-			    "Error on setting MAX_RETRIES config");
+		nfc_err(&dev->interface->dev,
+			"Error on setting MAX_RETRIES config\n");
 		return rc;
 	}
 
@@ -2355,8 +2354,7 @@ static int pn533_setup(struct pn533 *dev)
 	rc = pn533_set_configuration(dev, PN533_CFGITEM_TIMING,
 				     (u8 *)&timing, sizeof(timing));
 	if (rc) {
-		nfc_dev_err(&dev->interface->dev,
-			    "Error on setting RF timings");
+		nfc_err(&dev->interface->dev, "Error on setting RF timings\n");
 		return rc;
 	}
 
@@ -2370,8 +2368,8 @@ static int pn533_setup(struct pn533 *dev)
 		rc = pn533_set_configuration(dev, PN533_CFGITEM_PASORI,
 					     pasori_cfg, 3);
 		if (rc) {
-			nfc_dev_err(&dev->interface->dev,
-				    "Error while settings PASORI config");
+			nfc_err(&dev->interface->dev,
+				"Error while settings PASORI config\n");
 			return rc;
 		}
 
@@ -2416,8 +2414,8 @@ static int pn533_probe(struct usb_interface *interface,
 	}
 
 	if (!in_endpoint || !out_endpoint) {
-		nfc_dev_err(&interface->dev,
-			    "Could not find bulk-in or bulk-out endpoint");
+		nfc_err(&interface->dev,
+			"Could not find bulk-in or bulk-out endpoint\n");
 		rc = -ENODEV;
 		goto error;
 	}
@@ -2467,8 +2465,8 @@ static int pn533_probe(struct usb_interface *interface,
 		break;
 
 	default:
-		nfc_dev_err(&dev->interface->dev, "Unknown device type %d\n",
-			    dev->device_type);
+		nfc_err(&dev->interface->dev, "Unknown device type %d\n",
+			dev->device_type);
 		rc = -EINVAL;
 		goto destroy_wq;
 	}
@@ -2478,9 +2476,9 @@ static int pn533_probe(struct usb_interface *interface,
 	if (rc < 0)
 		goto destroy_wq;
 
-	nfc_dev_info(&dev->interface->dev,
-		     "NXP PN533 firmware ver %d.%d now attached",
-		     fw_ver.ver, fw_ver.rev);
+	nfc_info(&dev->interface->dev,
+		 "NXP PN533 firmware ver %d.%d now attached\n",
+		 fw_ver.ver, fw_ver.rev);
 
 
 	dev->nfc_dev = nfc_allocate_device(&pn533_nfc_ops, protocols,
@@ -2548,7 +2546,7 @@ static void pn533_disconnect(struct usb_interface *interface)
 	usb_free_urb(dev->out_urb);
 	kfree(dev);
 
-	nfc_dev_info(&interface->dev, "NXP PN533 NFC device disconnected");
+	nfc_info(&interface->dev, "NXP PN533 NFC device disconnected\n");
 }
 
 static struct usb_driver pn533_driver = {
diff --git a/include/net/nfc/nfc.h b/include/net/nfc/nfc.h
index 3448f09..853ff83 100644
--- a/include/net/nfc/nfc.h
+++ b/include/net/nfc/nfc.h
@@ -28,8 +28,8 @@
 #include <linux/device.h>
 #include <linux/skbuff.h>
 
-#define nfc_dev_info(dev, fmt, arg...) dev_info((dev), "NFC: " fmt "\n", ## arg)
-#define nfc_dev_err(dev, fmt, arg...) dev_err((dev), "NFC: " fmt "\n", ## arg)
+#define nfc_info(dev, fmt, ...) dev_info((dev), "NFC: " fmt, ##__VA_ARGS__)
+#define nfc_err(dev, fmt, ...) dev_err((dev), "NFC: " fmt, ##__VA_ARGS__)
 
 struct nfc_dev;
 
-- 
1.8.1.2.459.gbcd45b4.dirty

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

^ permalink raw reply related

* net_dropmon usage documentation/examples?
From: Eric Wong @ 2013-04-05 19:38 UTC (permalink / raw)
  To: Neil Horman; +Cc: linux-kernel, netdev, linux-man

Hi Neil, I'm wondering if you have or know of any public
documentation/examples for using net_dropmon.

If not, I'll figure it out on my own at some point.

Thanks in advance!

(Not a very high priority project for me, my network connectivity
 problems are sadly _very_ obvious at the moment :x)

^ permalink raw reply

* Re: [PATCH 1/5 v2] mv643xx_eth: add Device Tree bindings
From: Sebastian Hesselbarth @ 2013-04-05 20:35 UTC (permalink / raw)
  To: Jason Gunthorpe
  Cc: thomas.petazzoni, moinejf, Simon Baatz, andrew, netdev,
	devicetree-discuss, rob.herring, davem, grant.likely, jogo,
	buytenh, jm, Florian Fainelli, linux-arm-kernel, jason
In-Reply-To: <20130405180443.GE3598@obsidianresearch.com>

On 04/05/2013 08:04 PM, Jason Gunthorpe wrote:
> On Fri, Apr 05, 2013 at 03:58:03PM +0200, Sebastian Hesselbarth wrote:
>> I don't think that the ethernet controller should probe the PHY's on mdio-bus
>> at all. At least not for DT enabled platforms. I had a look at DT and non-DT
>> mdio-bus sources, and realized that there is a bus scan for non-DT only.
>> of_mdiobus_register requires you to set (and know) the PHY address.
>
> DT platforms should have the option to use the standard phy-phandle
> connection:
>
>
> 		mdio@72004 {
> 			#address-cells =<1>;
> 			#size-cells =<0>;
> 			compatible = "marvell,orion-mdio";
> 			reg =<0x72004 0x84>;
> 			status = "disabled";
>
> +                        PHY1: ethernet-phy@1 {
> +                                reg =<1>;
> +                                device_type = "ethernet-phy";
> +                        };
> 		};
>
> 		ethernet-group@72000 {
> 			#address-cells =<1>;
> 			#size-cells =<0>;
> 			compatible = "marvell,mv643xx-eth-block";
> 			reg =<0x72000 0x4000>;
> 			tx-csum-limit =<1600>;
> 			status = "disabled";
>
> 			egiga0: ethernet@0 {
> 				device_type = "network";
> 				compatible = "marvell,mv643xx-eth";
> 				reg =<0>;
> 				interrupts =<29>;
> 				clocks =<&gate_clk 2>;
> +	                        phy-handle =<&PHY1>;
> 			};
> 		};
>
> When phy-handle is present the ethernet driver should not probe/scan for
> phys.
>
> There is standard code to handle all of this - an important gain is
> that the phy driver now has access to a DT node and can apply
> phy-specific properties.

The above is what I use as a modification of Florian's patches.
I compared of_mdiobus_register against mdiobus_register. The difference
is that DT version does not scan the bus for attached PHYs. That is ok,
if you know the phy_address of the PHY that you pass the handle of. But 
in most cases there will be only one PHY on the mdiobus and especially
for new boards probing will help here.

>> We had a similar discussion whether to probe or not for DT nodes,
>> and I guess there also will be some discussion about the above
>> patch. OTOH we could just (again) ask users of every
>> kirkwood/orion5x/dove board to tell their phy addresses and fail to
>> probe the phy for new boards...
>
> Maybe print a warning and call the no-DT phy probe code if phy-handle
> is nor present?

I think it would be best to spam each probing result during mdiobus
scan to encourage people to edit the DT and put in the phy_addr
directly. IMHO it will be best to have bus scan on mdiobus rather than
ethernet driver.

> Not sure this should be in the common code, phy probing is sketchy, it
> shouldn't be encouraged, IMHO..

Actually, it is in common code (non-DT case) and I think passing the
phy_addr by DT node like above is best. But for boards you don't
know the address (yet) probing helps a lot. If all phy nodes have their
reg property set, no probing will be performed.

For testing mdio bus probe, I used

mdio {
	...
	ethphy: ethernet-phy {
		reg = <32>;
	};
};

And 32 should be defined as OF_PHY_ADDR_AUTOSCAN. It is an invalid
address as max phy_addr is 31. I also thought about a bool property
but passing an invalid reg property in SoC file allows to overwrite
it in board file with the actual address easily.
And AFAIK bool properties cannot be removed later on by board specific
nodes.

Sebastian

^ permalink raw reply

* [PATCH] mac802154: Keep track of the channel when changed
From: Alan Ott @ 2013-04-05 20:36 UTC (permalink / raw)
  To: Alexander Smirnov, Dmitry Eremin-Solenikov, David S. Miller
  Cc: linux-zigbee-devel, netdev, linux-kernel, Alan Ott

Prevent set_channel() from getting called every time a packet is sent. This
looks like it was an oversight.

Signed-off-by: Alan Ott <alan@signal11.us>
---
 net/mac802154/tx.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/net/mac802154/tx.c b/net/mac802154/tx.c
index 3fd3e07..6d16473 100644
--- a/net/mac802154/tx.c
+++ b/net/mac802154/tx.c
@@ -58,6 +58,9 @@ static void mac802154_xmit_worker(struct work_struct *work)
 			pr_debug("set_channel failed\n");
 			goto out;
 		}
+
+		xw->priv->phy->current_channel = xw->chan;
+		xw->priv->phy->current_page = xw->page;
 	}
 
 	res = xw->priv->ops->xmit(&xw->priv->hw, xw->skb);
-- 
1.7.11.2

^ permalink raw reply related


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