Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH] iproute2: Add missing tc-ematch.8 for man page installation
From: Stephen Hemminger @ 2012-08-21 14:49 UTC (permalink / raw)
  To: Oliver Hartkopp; +Cc: Linux Netdev List
In-Reply-To: <503319AD.2090701@hartkopp.net>

On Tue, 21 Aug 2012 07:16:29 +0200
Oliver Hartkopp <socketcan@hartkopp.net> wrote:

> 
> Signed-off-by: Oliver Hartkopp <socketcan@hartkopp.net>
Applied

^ permalink raw reply

* Re: IP fragmentation broken in 3.6-rc ?
From: Eric Dumazet @ 2012-08-21 14:42 UTC (permalink / raw)
  To: Sylvain Munaut; +Cc: netdev
In-Reply-To: <1345555336.5158.493.camel@edumazet-glaptop>

On Tue, 2012-08-21 at 15:22 +0200, Eric Dumazet wrote:
> On Tue, 2012-08-21 at 15:05 +0200, Sylvain Munaut wrote:
> > Hi,
> > 
> > I'm starting to wonder if it's not related to
> > 46517008e1168dc926cf2c47d529efc07eca85c0 "ipv4: Kill
> > ip_rt_frag_needed()."
> > 
> > Cheers,
> > 
> >     Sylvain
> 
> Hmm, most probably problem came with commit
> ceb3320610d6f15ff20dd4c042b36473d77de76f
> (ipv4: Kill routes during PMTU/redirect updates.)
> 
> 

Following patch should help :

diff --git a/include/net/dst.h b/include/net/dst.h
index 621e351..a04aa37 100644
--- a/include/net/dst.h
+++ b/include/net/dst.h
@@ -435,7 +435,7 @@ static inline void dst_set_expires(struct dst_entry *dst, int timeout)
 	if (expires == 0)
 		expires = 1;
 
-	if (dst->expires == 0 || time_before(expires, dst->expires))
+	if (dst->expires == 0 || time_after(expires, dst->expires))
 		dst->expires = expires;
 }
 

^ permalink raw reply related

* (ipt_log_packet, sb_add) 3.6.0-rc2 kernel panic - not syncing; Fatal exception in interrupt
From: Sami Farin @ 2012-08-21 13:49 UTC (permalink / raw)
  To: netdev; +Cc: Sami Farin, netdev

this with 3.6.0-rc2-g6dab7ede+ , x86_64 SMP, e1000e
gcc version 4.7.1 20120813 (Red Hat 4.7.1-7) (GCC) 
motherboard Asus P8Z68-V pro gen3

https://docs.google.com/open?id=0B_EMXtsx7o56UnJqNUNmMjNaSkE
(screencapture, 2.2 MB)

RIP [<ffffffff8109078c>] __lock_acquire.isra.25+0x15f/0xb20

Call Trace:
sb_add
lock_acquire
dump_ipv4_packet
_raw_read_lock_bh
dump_ipv4_packet
dump_ipv4_packet
sb_add
ipt_log_packet
log_tg
ipt_do_table
iptable_filter_hook
nf_iterate
ip_rcv_finish
nf_hook_slow
...

from vmlinux:
   0xffffffff8109075a <+301>:   cmpl   $0x0,0x1691a9f(%rip)        # 0xffffffff82722200 <debug_locks_silent>
   0xffffffff81090761 <+308>:   jne    0xffffffff8109073f <__lock_acquire+274>
   0xffffffff81090763 <+310>:   mov    $0xbd5,%esi
   0xffffffff81090768 <+315>:   mov    $0xffffffff81a4e0c9,%rdi
   0xffffffff8109076f <+322>:   callq  0xffffffff8104761f <warn_slowpath_null>
   0xffffffff81090774 <+327>:   xor    %eax,%eax
   0xffffffff81090776 <+329>:   jmpq   0xffffffff8109093a <__lock_acquire+781>
   0xffffffff8109077b <+334>:   mov    %r12d,%eax
   0xffffffff8109077e <+337>:   mov    0x8(%r14,%rax,8),%rbx
   0xffffffff81090783 <+342>:   test   %rbx,%rbx
   0xffffffff81090786 <+345>:   je     0xffffffff81090682 <__lock_acquire+85>
!! 0xffffffff8109078c <+351>:   lock incl 0x198(%rbx)
   0xffffffff81090793 <+358>:   mov    0x660(%r13),%r15d
   0xffffffff8109079a <+365>:   mov    0xedfa0f(%rip),%r10d        # 0xffffffff81f701b0 <oops_in_progress>
   0xffffffff810907a1 <+372>:   test   %r10d,%r10d
   0xffffffff810907a4 <+375>:   jne    0xffffffff810907b0 <__lock_acquire+387>
   0xffffffff810907a6 <+377>:   cmp    $0x2f,%r15d
   0xffffffff810907aa <+381>:   ja     0xffffffff810909fb <__lock_acquire+974>
   0xffffffff810907b0 <+387>:   sub    $0xffffffff81fb66e0,%rbx
   0xffffffff810907b7 <+394>:   sar    $0x4,%rbx
   0xffffffff810907bb <+398>:   movabs $0xef7bdef7bdef7bdf,%rax
   0xffffffff810907c5 <+408>:   imul   %rax,%rbx
   0xffffffff810907c9 <+412>:   mov    %ebx,%r11d
   0xffffffff810907cc <+415>:   lea    0x1(%rbx),%eax
   0xffffffff810907cf <+418>:   test   %r15d,%r15d
   0xffffffff810907d2 <+421>:   je     0xffffffff81090902 <__lock_acquire+725>
   0xffffffff810907d8 <+427>:   mov    %r15d,%edi

kernel 3.5.0-rc6-g4523e145+ worked ok for over a month..

# ethtool eth0
Settings for eth0:
        Supported ports: [ TP ]
        Supported link modes:   10baseT/Half 10baseT/Full 
                                100baseT/Half 100baseT/Full 
                                1000baseT/Full 
        Supported pause frame use: No
        Supports auto-negotiation: Yes
        Advertised link modes:  10baseT/Half 10baseT/Full 
                                100baseT/Half 100baseT/Full 
                                1000baseT/Full 
        Advertised pause frame use: No
        Advertised auto-negotiation: Yes
        Speed: 100Mb/s
        Duplex: Full
        Port: Twisted Pair
        PHYAD: 2
        Transceiver: internal
        Auto-negotiation: on
        MDI-X: on
        Supports Wake-on: pumbg
        Wake-on: g
        Current message level: 0x00000007 (7)
                               drv probe link
        Link detected: yes

# ethtool -k eth0
Offload parameters for eth0:
rx-checksumming: off
tx-checksumming: off
scatter-gather: off
tcp-segmentation-offload: off
udp-fragmentation-offload: off
generic-segmentation-offload: off
generic-receive-offload: off
large-receive-offload: off
rx-vlan-offload: on
tx-vlan-offload: on
ntuple-filters: off
receive-hashing: on

-- 
Do what you love because life is too short for anything else.

^ permalink raw reply

* Re: [PATCH 2/3] ipvs: Fix faulty IPv6 extension header handling in IPVS
From: Julian Anastasov @ 2012-08-21 14:14 UTC (permalink / raw)
  To: Jesper Dangaard Brouer
  Cc: netdev, Patrick McHardy, Hans Schillstrom, lvs-devel,
	Simon Horman, Wensong Zhang, netfilter-devel
In-Reply-To: <20120820130840.1509.75794.stgit@dragon>


	Hello,

On Mon, 20 Aug 2012, Jesper Dangaard Brouer wrote:

> Based on patch from: Hans Schillstrom
> 
> IPv6 headers must be processed in order of appearance,
> neither can it be assumed that Upper layer headers is first.
> If anything else than L4 is the first header IPVS will throw it.
> 
> IPVS will write SNAT & DNAT modifications at a fixed pos which
> will corrupt the message. Proper header position must be found
> before writing modifying packet.
> 
> This patch contains a lot of API changes.  This is done, to avoid
> the costly scan of finding the IPv6 headers, via ipv6_find_hdr().
> Finding the IPv6 headers is done as early as possible, and passed
> on as a pointer "struct ip_vs_iphdr *" to the affected functions.
> 
> Notice, I have choosen, not to change the API of function
> pointer "(*schedule)" (in struct ip_vs_scheduler) as it can be
> used by external schedulers, via {un,}register_ip_vs_scheduler.
> Only 4 out of 10 schedulers use info from ip_vs_iphdr*, and when
> they do, they are only interested in iph->{s,d}addr.
> 
> This patch depends on commit 84018f55a:
>  "netfilter: ip6_tables: add flags parameter to ipv6_find_hdr()"
> 
> This also adds a dependency to ip6_tables.
> 
> Hans left some questions in ip_vs_pe_sip.c, which I'm uncertain about.
> 
> Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>
> Signed-off-by: Hans Schillstrom <hans@schillstrom.com>

	Patch 1 looks ok, following are some small comments
for patch 2 and 3.

> +#if defined(CONFIG_IPV6) || defined(CONFIG_IPV6_MODULE)
> +#include <linux/netfilter_ipv6/ip6_tables.h>
> +#endif

	There is already #if IS_ENABLED(CONFIG_IPV6) that
can replace #if defined(CONFIG_IPV6) || defined(CONFIG_IPV6_MODULE)

	It seems we need IS_ENABLED for many places:
CONFIG_NF_CONNTRACK, CONFIG_NF_DEFRAG_IPV6

> @@ -958,34 +943,26 @@ static int ip_vs_out_icmp_v6(struct sk_buff *skb, int *related,
>  	}
>  
>  	/* Now find the contained IP header */
> -	offset += sizeof(_icmph);
> -	cih = skb_header_pointer(skb, offset, sizeof(_ciph), &_ciph);
> -	if (cih == NULL)
> -		return NF_ACCEPT; /* The packet looks wrong, ignore */
> +	ipvsh->len += sizeof(_icmph);
> +	ip6 = skb_header_pointer(skb, ipvsh->len, sizeof(_ip6), &_ip6);

	ip6 is not checked here for NULL or we rely on
ipv6_find_hdr checks?

> @@ -1506,39 +1464,43 @@ ip_vs_in_icmp_v6(struct sk_buff *skb, int *related, unsigned int hooknum)
>  	}
>  
>  	/* Now find the contained IP header */
> -	offset += sizeof(_icmph);
> -	cih = skb_header_pointer(skb, offset, sizeof(_ciph), &_ciph);
> -	if (cih == NULL)
> -		return NF_ACCEPT; /* The packet looks wrong, ignore */
> +	ciph.len = iph->len + sizeof(_icmph);
> +	ciph.flags = 0;
> +	ciph.fragoffs = 0;
> +	ciph.protocol = ipv6_find_hdr(skb, &ciph.len, -1, &ciph.fragoffs,
> +				      &ciph.flags);
> +	ciph.saddr = iph->saddr;	/* con_in_get() handles reverse order */
> +	ciph.daddr = iph->daddr;

	The ciph initialization looks dangerous if one day
we add new field into the header.

	Can we use ciph = (struct ip_vs_iphdr) { .XXX = val, ... },
in such case we have to call ipv6_find_hdr out of (after)
this initialization? Of course, we will write twice to
small fields such as protocol, len, fragoffs, flags

	Also ipv6_find_hdr looks a bit noisy for missing header,
can it be a problem for the inner IPv6 header in ICMP messages?

	In patch 3 ip_vs_in_icmp_v6 initializes ciph in the
same way. It will be difficult to audit the code later
considering the large number of places where iph is used.

>  	net = skb_net(skb);
> -	pd = ip_vs_proto_data_get(net, cih->nexthdr);
> +	pd = ip_vs_proto_data_get(net, ciph.protocol);
>  	if (!pd)
>  		return NF_ACCEPT;
>  	pp = pd->pp;
>  
> -	/* Is the embedded protocol header present? */
> -	/* TODO: we don't support fragmentation at the moment anyways */
> -	if (unlikely(cih->nexthdr == IPPROTO_FRAGMENT && pp->dont_defrag))
> +	/* Is the embedded protocol header present?
> +	 * If it's the second or later fragment we don't know what it is
> +	 * i.e. just let it through.
> +	 */
> +	if (ciph.fragoffs)
>  		return NF_ACCEPT;
>  
> +	offset = ciph.len;
>  	IP_VS_DBG_PKT(11, AF_INET6, pp, skb, offset,
>  		      "Checking incoming ICMPv6 for");
>  
> -	offset += sizeof(struct ipv6hdr);
> -
> -	ip_vs_fill_iphdr(AF_INET6, cih, &ciph);
>  	/* The embedded headers contain source and dest in reverse order */
> -	cp = pp->conn_in_get(AF_INET6, skb, &ciph, offset, 1);
> +	cp = pp->conn_in_get(AF_INET6, skb, &ciph, 1);
>  	if (!cp)
>  		return NF_ACCEPT;
>  
>  	/* do the statistics and put it back */
>  	ip_vs_in_stats(cp, skb);
> -	if (IPPROTO_TCP == cih->nexthdr || IPPROTO_UDP == cih->nexthdr ||
> -	    IPPROTO_SCTP == cih->nexthdr)
> -		offset += 2 * sizeof(__u16);
> -	verdict = ip_vs_icmp_xmit_v6(skb, cp, pp, offset, hooknum);
> +	if (IPPROTO_TCP == ciph.protocol || IPPROTO_UDP == ciph.protocol ||
> +	    IPPROTO_SCTP == ciph.protocol)
> +		offset = ciph.len + (2 * sizeof(__u16));

	Still in the same func, above code is correct but
patch 3 changes it back to wrong state (offs_ciph = ciph.len).

> +
> +	verdict = ip_vs_icmp_xmit_v6(skb, cp, pp, offset, hooknum, &ciph);
>  
>  	__ip_vs_conn_put(cp);

> diff --git a/net/netfilter/ipvs/ip_vs_pe_sip.c b/net/netfilter/ipvs/ip_vs_pe_sip.c
> index 1aa5cac..bb28b4f 100644
> --- a/net/netfilter/ipvs/ip_vs_pe_sip.c
> +++ b/net/netfilter/ipvs/ip_vs_pe_sip.c
> @@ -68,26 +68,37 @@ static int get_callid(const char *dptr, unsigned int dataoff,
>  static int
>  ip_vs_sip_fill_param(struct ip_vs_conn_param *p, struct sk_buff *skb)
>  {
> +	struct sk_buff *reasm = skb_nfct_reasm(skb);
>  	struct ip_vs_iphdr iph;
>  	unsigned int dataoff, datalen, matchoff, matchlen;
>  	const char *dptr;
>  	int retc;
>  
> -	ip_vs_fill_iphdr(p->af, skb_network_header(skb), &iph);
> +	ip_vs_fill_iph_skb(p->af, skb, &iph);

	May be skb_linearize is bad for IPv6? IIRC,
ip_vs_pe_sip.c needs access just to read the Call-ID.
For IPv4 it was simple to use skb_linearize, may be
the logic should be improved to read the values even
from non-linear data. May be there is already some
example code for this. For IPv6 I'm not sure what
kind are the problems here, may be it depends if
we try to call skb_linearize for reasm packet?

> -
> -	if ((retc=skb_linearize(skb)) < 0)
> +	/*
> +	 * todo: Check if this will mess-up the reasm skb !!! /HS
> +	 */
> +	retc = skb_linearize(reasm);
> +	if (retc < 0)
>  		return retc;
> -	dptr = skb->data + dataoff;
> -	datalen = skb->len - dataoff;
> +	dptr = reasm->data + dataoff;
> +	datalen = reasm->len - dataoff;
>  
>  	if (get_callid(dptr, dataoff, datalen, &matchoff, &matchlen))
>  		return -EINVAL;

	There are recents changes for IPv6 from Patrick McHardy:

http://marc.info/?l=netfilter-devel&m=134543406303402&w=2
http://marc.info/?l=netfilter-devel&m=134543407803412&w=2

	In this context, may be soon we will modify ip_vs_ftp to
support IPv6. It is possible to work with present FTP helper
in netfilter. Does it mean that we will see only reassembled
packets when conntrack is running? No original fragments.
Are we prepared to work in both ways (originals+reasm and
just reasm) ?

- For patch 3 in Kconfig do we need 'select NF_DEFRAG_IPV6' ?

	Basicly, I'm concerned what will happen when we
start to mangle the protocol payloads (FTP). For now
we are safe by touching only addresses and ports. May be
we have to synchronize these changes with the work from
Patrick.

Regards

--
Julian Anastasov <ja@ssi.bg>

^ permalink raw reply

* Re: [RFC Patch net-next] ipv6: unify conntrack reassembly expire code with standard one
From: Cong Wang @ 2012-08-21 13:38 UTC (permalink / raw)
  To: Michal Kubecek
  Cc: netdev, Herbert Xu, David S. Miller, Hideaki YOSHIFUJI,
	Patrick McHardy, Shan Wei, Pablo Neira Ayuso, netfilter-devel
In-Reply-To: <20120820202109.GB28790@unicorn.suse.cz>

On Mon, 2012-08-20 at 22:21 +0200, Michal Kubecek wrote:
> On Mon, Aug 20, 2012 at 05:06:39PM +0800, Cong Wang wrote:
> > doesn't work. I think we probably can save the struct net pointer in
> > struct netns_frags during inet_frags_init_net(), so that container_of()
> > can be eliminated. 
> 
> This would work but one would have to check carefully that the pointer
> is always set to an usable value. Or we could just add a flag indicating
> whether the structure is embedded (and if it is not, use init_net).
> Another approach was suggested in the patchworks discussion: add a
> special namespace for IPv6 conntrack fragment handling.

I think your former solution is easier, I will try it.

> 
> > Thanks for testing! I tried to test it too, but seems I can't trigger a
> > defragment. Any hints?
> 
> I used netfilter on a computer between source and destination of the 
> packets (generated with "ping6 -s 3000"):
> 
> ip6tables -A FORWARD -o ... -m frag --fragid 0:0xFFFFFFFF --fraglast -j DROP
> 
> The --fragid condition is needed to work around a bug in iptables (if 
> frag module is loaded and there is no --fragid, only packets with zero
> fragment id match - patch for this is already in git). On the target,
> packet is defragmented automatically, by default by the "normal" code,
> with nf_conntrack_ipv6 module by the conntrack code.
> 
> Setting
> 
>   echo 5 >/proc/sys/net/ipv6/ip6frag_time
>   echo 5 >/proc/sys/net/netfilter/nf_conntrack_frag6_timeout
> 
> on target also helps.
> 

Great! This helps!

Thanks!


^ permalink raw reply

* Re: IP fragmentation broken in 3.6-rc ?
From: Eric Dumazet @ 2012-08-21 13:22 UTC (permalink / raw)
  To: Sylvain Munaut; +Cc: netdev
In-Reply-To: <CAF6-1L6DTXPGi4kbcPqbOXrrafMmZ6K1QdcAa=PxJBzZWkvojg@mail.gmail.com>

On Tue, 2012-08-21 at 15:05 +0200, Sylvain Munaut wrote:
> Hi,
> 
> I'm starting to wonder if it's not related to
> 46517008e1168dc926cf2c47d529efc07eca85c0 "ipv4: Kill
> ip_rt_frag_needed()."
> 
> Cheers,
> 
>     Sylvain

Hmm, most probably problem came with commit
ceb3320610d6f15ff20dd4c042b36473d77de76f
(ipv4: Kill routes during PMTU/redirect updates.)

^ permalink raw reply

* Re: IP fragmentation broken in 3.6-rc ?
From: Sylvain Munaut @ 2012-08-21 13:05 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: netdev

Hi,

I'm starting to wonder if it's not related to
46517008e1168dc926cf2c47d529efc07eca85c0 "ipv4: Kill
ip_rt_frag_needed()."

Cheers,

    Sylvain

^ permalink raw reply

* Re: [net-next PATCH] mdio: translation of MMD EEE registers to/from ethtool settings
From: Giuseppe CAVALLARO @ 2012-08-21 12:47 UTC (permalink / raw)
  To: Bruce Allan; +Cc: netdev
In-Reply-To: <20120820145529.19390.66175.stgit@bwallan-smackover2.jf.intel.com>

On 8/20/2012 4:55 PM, Bruce Allan wrote:
> The helper functions which translate IEEE MDIO Manageable Device (MMD)
> Energy-Efficient Ethernet (EEE) registers 3.20, 7.60 and 7.61 to and from
> the comparable ethtool supported/advertised settings will be needed by
> drivers other than those in PHYLIB (e.g. e1000e in a follow-on patch).
> 
> In the same fashion as similar translation functions in linux/mii.h, move
> these functions from the PHYLIB core to the linux/mdio.h header file so the
> code will not have to be duplicated in each driver needing MMD-to-ethtool
> (and vice-versa) translations.  The function and some variable names have
> been renamed to be more descriptive.
> 
> Not tested on the only hardware that currently calls the related functions,
> stmmac, because I don't have access to any.  Has been compile tested and
> the translations have been tested on a locally modified version of e1000e.
> 
> Signed-off-by: Bruce Allan <bruce.w.allan@intel.com>
> Cc: Giuseppe Cavallaro <peppe.cavallaro@st.com>

I did some tests on my side with the stmmac and I've not seen any issues
on EEE. Also the patch looks fine to me.

Peppe

> ---
> 
>  drivers/net/phy/phy.c |   74 ++++----------------------------------------
>  include/linux/mdio.h  |   83 +++++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 90 insertions(+), 67 deletions(-)
> 
> diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
> index 7ca2ff9..ef9ea92 100644
> --- a/drivers/net/phy/phy.c
> +++ b/drivers/net/phy/phy.c
> @@ -1035,66 +1035,6 @@ static void phy_write_mmd_indirect(struct mii_bus *bus, int prtad, int devad,
>  	bus->write(bus, addr, MII_MMD_DATA, data);
>  }
>  
> -static u32 phy_eee_to_adv(u16 eee_adv)
> -{
> -	u32 adv = 0;
> -
> -	if (eee_adv & MDIO_EEE_100TX)
> -		adv |= ADVERTISED_100baseT_Full;
> -	if (eee_adv & MDIO_EEE_1000T)
> -		adv |= ADVERTISED_1000baseT_Full;
> -	if (eee_adv & MDIO_EEE_10GT)
> -		adv |= ADVERTISED_10000baseT_Full;
> -	if (eee_adv & MDIO_EEE_1000KX)
> -		adv |= ADVERTISED_1000baseKX_Full;
> -	if (eee_adv & MDIO_EEE_10GKX4)
> -		adv |= ADVERTISED_10000baseKX4_Full;
> -	if (eee_adv & MDIO_EEE_10GKR)
> -		adv |= ADVERTISED_10000baseKR_Full;
> -
> -	return adv;
> -}
> -
> -static u32 phy_eee_to_supported(u16 eee_caported)
> -{
> -	u32 supported = 0;
> -
> -	if (eee_caported & MDIO_EEE_100TX)
> -		supported |= SUPPORTED_100baseT_Full;
> -	if (eee_caported & MDIO_EEE_1000T)
> -		supported |= SUPPORTED_1000baseT_Full;
> -	if (eee_caported & MDIO_EEE_10GT)
> -		supported |= SUPPORTED_10000baseT_Full;
> -	if (eee_caported & MDIO_EEE_1000KX)
> -		supported |= SUPPORTED_1000baseKX_Full;
> -	if (eee_caported & MDIO_EEE_10GKX4)
> -		supported |= SUPPORTED_10000baseKX4_Full;
> -	if (eee_caported & MDIO_EEE_10GKR)
> -		supported |= SUPPORTED_10000baseKR_Full;
> -
> -	return supported;
> -}
> -
> -static u16 phy_adv_to_eee(u32 adv)
> -{
> -	u16 reg = 0;
> -
> -	if (adv & ADVERTISED_100baseT_Full)
> -		reg |= MDIO_EEE_100TX;
> -	if (adv & ADVERTISED_1000baseT_Full)
> -		reg |= MDIO_EEE_1000T;
> -	if (adv & ADVERTISED_10000baseT_Full)
> -		reg |= MDIO_EEE_10GT;
> -	if (adv & ADVERTISED_1000baseKX_Full)
> -		reg |= MDIO_EEE_1000KX;
> -	if (adv & ADVERTISED_10000baseKX4_Full)
> -		reg |= MDIO_EEE_10GKX4;
> -	if (adv & ADVERTISED_10000baseKR_Full)
> -		reg |= MDIO_EEE_10GKR;
> -
> -	return reg;
> -}
> -
>  /**
>   * phy_init_eee - init and check the EEE feature
>   * @phydev: target phy_device struct
> @@ -1132,7 +1072,7 @@ int phy_init_eee(struct phy_device *phydev, bool clk_stop_enable)
>  		if (eee_cap < 0)
>  			return eee_cap;
>  
> -		cap = phy_eee_to_supported(eee_cap);
> +		cap = mmd_eee_cap_to_ethtool_sup_t(eee_cap);
>  		if (!cap)
>  			goto eee_exit;
>  
> @@ -1149,8 +1089,8 @@ int phy_init_eee(struct phy_device *phydev, bool clk_stop_enable)
>  		if (eee_adv < 0)
>  			return eee_adv;
>  
> -		adv = phy_eee_to_adv(eee_adv);
> -		lp = phy_eee_to_adv(eee_lp);
> +		adv = mmd_eee_adv_to_ethtool_adv_t(eee_adv);
> +		lp = mmd_eee_adv_to_ethtool_adv_t(eee_lp);
>  		idx = phy_find_setting(phydev->speed, phydev->duplex);
>  		if ((lp & adv & settings[idx].setting))
>  			goto eee_exit;
> @@ -1210,21 +1150,21 @@ int phy_ethtool_get_eee(struct phy_device *phydev, struct ethtool_eee *data)
>  				    MDIO_MMD_PCS, phydev->addr);
>  	if (val < 0)
>  		return val;
> -	data->supported = phy_eee_to_supported(val);
> +	data->supported = mmd_eee_cap_to_ethtool_sup_t(val);
>  
>  	/* Get advertisement EEE */
>  	val = phy_read_mmd_indirect(phydev->bus, MDIO_AN_EEE_ADV,
>  				    MDIO_MMD_AN, phydev->addr);
>  	if (val < 0)
>  		return val;
> -	data->advertised = phy_eee_to_adv(val);
> +	data->advertised = mmd_eee_adv_to_ethtool_adv_t(val);
>  
>  	/* Get LP advertisement EEE */
>  	val = phy_read_mmd_indirect(phydev->bus, MDIO_AN_EEE_LPABLE,
>  				    MDIO_MMD_AN, phydev->addr);
>  	if (val < 0)
>  		return val;
> -	data->lp_advertised = phy_eee_to_adv(val);
> +	data->lp_advertised = mmd_eee_adv_to_ethtool_adv_t(val);
>  
>  	return 0;
>  }
> @@ -1241,7 +1181,7 @@ int phy_ethtool_set_eee(struct phy_device *phydev, struct ethtool_eee *data)
>  {
>  	int val;
>  
> -	val = phy_adv_to_eee(data->advertised);
> +	val = ethtool_adv_to_mmd_eee_adv_t(data->advertised);
>  	phy_write_mmd_indirect(phydev->bus, MDIO_AN_EEE_ADV, MDIO_MMD_AN,
>  			       phydev->addr, val);
>  
> diff --git a/include/linux/mdio.h b/include/linux/mdio.h
> index 7cccafe..6c40684 100644
> --- a/include/linux/mdio.h
> +++ b/include/linux/mdio.h
> @@ -377,5 +377,88 @@ static inline void mdio45_ethtool_gset(const struct mdio_if_info *mdio,
>  extern int mdio_mii_ioctl(const struct mdio_if_info *mdio,
>  			  struct mii_ioctl_data *mii_data, int cmd);
>  
> +/**
> + * mmd_eee_cap_to_ethtool_sup_t
> + * @eee_cap: value of the MMD EEE Capability register
> + *
> + * A small helper function that translates MMD EEE Capability (3.20) bits
> + * to ethtool supported settings.
> + */
> +static inline u32 mmd_eee_cap_to_ethtool_sup_t(u16 eee_cap)
> +{
> +	u32 supported = 0;
> +
> +	if (eee_cap & MDIO_EEE_100TX)
> +		supported |= SUPPORTED_100baseT_Full;
> +	if (eee_cap & MDIO_EEE_1000T)
> +		supported |= SUPPORTED_1000baseT_Full;
> +	if (eee_cap & MDIO_EEE_10GT)
> +		supported |= SUPPORTED_10000baseT_Full;
> +	if (eee_cap & MDIO_EEE_1000KX)
> +		supported |= SUPPORTED_1000baseKX_Full;
> +	if (eee_cap & MDIO_EEE_10GKX4)
> +		supported |= SUPPORTED_10000baseKX4_Full;
> +	if (eee_cap & MDIO_EEE_10GKR)
> +		supported |= SUPPORTED_10000baseKR_Full;
> +
> +	return supported;
> +}
> +
> +/**
> + * mmd_eee_adv_to_ethtool_adv_t
> + * @eee_adv: value of the MMD EEE Advertisement/Link Partner Ability registers
> + *
> + * A small helper function that translates the MMD EEE Advertisment (7.60)
> + * and MMD EEE Link Partner Ability (7.61) bits to ethtool advertisement
> + * settings.
> + */
> +static inline u32 mmd_eee_adv_to_ethtool_adv_t(u16 eee_adv)
> +{
> +	u32 adv = 0;
> +
> +	if (eee_adv & MDIO_EEE_100TX)
> +		adv |= ADVERTISED_100baseT_Full;
> +	if (eee_adv & MDIO_EEE_1000T)
> +		adv |= ADVERTISED_1000baseT_Full;
> +	if (eee_adv & MDIO_EEE_10GT)
> +		adv |= ADVERTISED_10000baseT_Full;
> +	if (eee_adv & MDIO_EEE_1000KX)
> +		adv |= ADVERTISED_1000baseKX_Full;
> +	if (eee_adv & MDIO_EEE_10GKX4)
> +		adv |= ADVERTISED_10000baseKX4_Full;
> +	if (eee_adv & MDIO_EEE_10GKR)
> +		adv |= ADVERTISED_10000baseKR_Full;
> +
> +	return adv;
> +}
> +
> +/**
> + * ethtool_adv_to_mmd_eee_adv_t
> + * @adv: the ethtool advertisement settings
> + *
> + * A small helper function that translates ethtool advertisement settings
> + * to EEE advertisements for the MMD EEE Advertisement (7.60) and
> + * MMD EEE Link Partner Ability (7.61) registers.
> + */
> +static inline u16 ethtool_adv_to_mmd_eee_adv_t(u32 adv)
> +{
> +	u16 reg = 0;
> +
> +	if (adv & ADVERTISED_100baseT_Full)
> +		reg |= MDIO_EEE_100TX;
> +	if (adv & ADVERTISED_1000baseT_Full)
> +		reg |= MDIO_EEE_1000T;
> +	if (adv & ADVERTISED_10000baseT_Full)
> +		reg |= MDIO_EEE_10GT;
> +	if (adv & ADVERTISED_1000baseKX_Full)
> +		reg |= MDIO_EEE_1000KX;
> +	if (adv & ADVERTISED_10000baseKX4_Full)
> +		reg |= MDIO_EEE_10GKX4;
> +	if (adv & ADVERTISED_10000baseKR_Full)
> +		reg |= MDIO_EEE_10GKR;
> +
> +	return reg;
> +}
> +
>  #endif /* __KERNEL__ */
>  #endif /* __LINUX_MDIO_H__ */
> 
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> 

^ permalink raw reply

* Re: Fwd: TCP fragmentation broken in 3.6-rc ?
From: Sylvain Munaut @ 2012-08-21 11:56 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: netdev
In-Reply-To: <1345478571.5158.323.camel@edumazet-glaptop>

Hi,

> Any chance you could post a tcpdump demonstrating the problem ?
> (including the tcp session plus icmp messages)

Here it is :

http://ge.tt/api/1/files/76YVbLM/0/blob?download


Cheers,

    Sylvain

^ permalink raw reply

* [PATCH net-next] packet: Protect packet sk list with mutex (v2)
From: Pavel Emelyanov @ 2012-08-21 11:06 UTC (permalink / raw)
  To: Eric Dumazet, David Miller, Linux Netdev List

Change since v1:

* Fixed inuse counters access spotted by Eric

In patch eea68e2f (packet: Report socket mclist info via diag module) I've
introduced a "scheduling in atomic" problem in packet diag module -- the
socket list is traversed under rcu_read_lock() while performed under it sk
mclist access requires rtnl lock (i.e. -- mutex) to be taken.

[152363.820563] BUG: scheduling while atomic: crtools/12517/0x10000002
[152363.820573] 4 locks held by crtools/12517:
[152363.820581]  #0:  (sock_diag_mutex){+.+.+.}, at: [<ffffffff81a2dcb5>] sock_diag_rcv+0x1f/0x3e
[152363.820613]  #1:  (sock_diag_table_mutex){+.+.+.}, at: [<ffffffff81a2de70>] sock_diag_rcv_msg+0xdb/0x11a
[152363.820644]  #2:  (nlk->cb_mutex){+.+.+.}, at: [<ffffffff81a67d01>] netlink_dump+0x23/0x1ab
[152363.820693]  #3:  (rcu_read_lock){.+.+..}, at: [<ffffffff81b6a049>] packet_diag_dump+0x0/0x1af

Similar thing was then re-introduced by further packet diag patches (fanount 
mutex and pgvec mutex for rings) :(

Apart from being terribly sorry for the above, I propose to change the packet
sk list protection from spinlock to mutex. This lock currently protects two
modifications:

* sklist
* prot inuse counters

The sklist modifications can be just reprotected with mutex since they already
occur in a sleeping context. The inuse counters modifications are trickier -- the
__this_cpu_-s are used inside, thus requiring the caller to handle the potential
issues with contexts himself. Since packet sockets' counters are modified in two 
places only (packet_create and packet_release) we only need to protect the context 
from being preempted. BH disabling is not required in this case.

Signed-off-by: Pavel Emelyanov <xemul@parallels.com>

---

diff --git a/include/net/netns/packet.h b/include/net/netns/packet.h
index cb4e894..4780b08 100644
--- a/include/net/netns/packet.h
+++ b/include/net/netns/packet.h
@@ -8,7 +8,7 @@
 #include <linux/spinlock.h>
 
 struct netns_packet {
-	spinlock_t		sklist_lock;
+	struct mutex		sklist_lock;
 	struct hlist_head	sklist;
 };
 
diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c
index 226b2cd..79bc69c 100644
--- a/net/packet/af_packet.c
+++ b/net/packet/af_packet.c
@@ -2308,10 +2308,13 @@ static int packet_release(struct socket *sock)
 	net = sock_net(sk);
 	po = pkt_sk(sk);
 
-	spin_lock_bh(&net->packet.sklist_lock);
+	mutex_lock(&net->packet.sklist_lock);
 	sk_del_node_init_rcu(sk);
+	mutex_unlock(&net->packet.sklist_lock);
+
+	preempt_disable();
 	sock_prot_inuse_add(net, sk->sk_prot, -1);
-	spin_unlock_bh(&net->packet.sklist_lock);
+	preempt_enable();
 
 	spin_lock(&po->bind_lock);
 	unregister_prot_hook(sk, false);
@@ -2510,10 +2513,13 @@ static int packet_create(struct net *net, struct socket *sock, int protocol,
 		register_prot_hook(sk);
 	}
 
-	spin_lock_bh(&net->packet.sklist_lock);
+	mutex_lock(&net->packet.sklist_lock);
 	sk_add_node_rcu(sk, &net->packet.sklist);
+	mutex_unlock(&net->packet.sklist_lock);
+
+	preempt_disable();
 	sock_prot_inuse_add(net, &packet_proto, 1);
-	spin_unlock_bh(&net->packet.sklist_lock);
+	preempt_enable();
 
 	return 0;
 out:
@@ -3766,7 +3772,7 @@ static const struct file_operations packet_seq_fops = {
 
 static int __net_init packet_net_init(struct net *net)
 {
-	spin_lock_init(&net->packet.sklist_lock);
+	mutex_init(&net->packet.sklist_lock);
 	INIT_HLIST_HEAD(&net->packet.sklist);
 
 	if (!proc_net_fops_create(net, "packet", 0, &packet_seq_fops))
diff --git a/net/packet/diag.c b/net/packet/diag.c
index bc33fbe..39bce0d 100644
--- a/net/packet/diag.c
+++ b/net/packet/diag.c
@@ -177,8 +177,8 @@ static int packet_diag_dump(struct sk_buff *skb, struct netlink_callback *cb)
 	net = sock_net(skb->sk);
 	req = nlmsg_data(cb->nlh);
 
-	rcu_read_lock();
-	sk_for_each_rcu(sk, node, &net->packet.sklist) {
+	mutex_lock(&net->packet.sklist_lock);
+	sk_for_each(sk, node, &net->packet.sklist) {
 		if (!net_eq(sock_net(sk), net))
 			continue;
 		if (num < s_num)
@@ -192,7 +192,7 @@ next:
 		num++;
 	}
 done:
-	rcu_read_unlock();
+	mutex_unlock(&net->packet.sklist_lock);
 	cb->args[0] = num;
 
 	return skb->len;

^ permalink raw reply related

* Re: coping with memory limitations and packet flooding in codel and fq_codel
From: Eric Dumazet @ 2012-08-21 10:50 UTC (permalink / raw)
  To: Andrew McGregor; +Cc: netdev, codel
In-Reply-To: <46C4C66E-A4EE-4051-9686-7AD02EE925B4@gmail.com>

On Tue, 2012-08-21 at 22:27 +1200, Andrew McGregor wrote:
> On 21/08/2012, at 8:38 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> 
> > ath9k driver is known to deliver very fat skbs (skb->truesize is too
> > big)
> 
> What would it take to fix this?
> 
> Andrew

Someone could use the following : 

https://gerrit.chromium.org/gerrit/#/c/18412/

^ permalink raw reply

* Re: iputils git tree
From: Xose Vazquez Perez @ 2012-08-21 10:42 UTC (permalink / raw)
  To: netdev

Jan Engelhardt wrote:

> And half of iputils has been superseded or thelike.
>
> * arping -> th's arping http://freecode.com/projects/arping
> * ifenslave -> bonding's sysfs files
> * tftpd -> tftpd-hpa

* traceroute6 -> http://traceroute.sf.net/

^ permalink raw reply

* Re: coping with memory limitations and packet flooding in codel and fq_codel
From: Andrew McGregor @ 2012-08-21 10:27 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: netdev, codel
In-Reply-To: <1345538298.5158.401.camel@edumazet-glaptop>


[-- Attachment #1.1: Type: text/plain, Size: 192 bytes --]


On 21/08/2012, at 8:38 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:

> ath9k driver is known to deliver very fat skbs (skb->truesize is too
> big)

What would it take to fix this?

Andrew

[-- Attachment #1.2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 2330 bytes --]

[-- Attachment #2: Type: text/plain, Size: 140 bytes --]

_______________________________________________
Codel mailing list
Codel@lists.bufferbloat.net
https://lists.bufferbloat.net/listinfo/codel

^ permalink raw reply

* Re: smsc75xx & smsc95xx, setting skb->truesize correctly?
From: Eric Dumazet @ 2012-08-21  9:46 UTC (permalink / raw)
  To: Jussi Kivilinna; +Cc: netdev, Steve Glendinning
In-Reply-To: <20120820175706.12394vktsvm3yvqc@www.81.fi>

On Mon, 2012-08-20 at 17:57 +0300, Jussi Kivilinna wrote:
> Hello,
> 
> Is setting skb->truesize in smsc75xx and smsc95xx correct?
> In smsc75xx/smsc95xx_rx_fixup(), input skb containing multiple packets  
> is cloned and truesize for each clone is set to packet-size +  
> sizeof(struct sk_buff), but input skb has minimum allocation size of  
> 9000 bytes (MAX_SINGLE_PACKET_SIZE) and maximum of 18944 bytes  
> (DEFAULT_HS_BURST_CAP_SIZE) (+ NET_IP_ALIGN). Doesn't this cause  
> truesize to be underestimated?

This has been discussed in a "TCP transmit performance regression"
thread some weeks ago.

More generally, skb_clone() is not a good idea in rx path.

I dont have the hardware so cannot send a formal patch.

diff --git a/drivers/net/usb/smsc95xx.c b/drivers/net/usb/smsc95xx.c
index b1112e7..3d9566f 100644
--- a/drivers/net/usb/smsc95xx.c
+++ b/drivers/net/usb/smsc95xx.c
@@ -1080,30 +1080,17 @@ static int smsc95xx_rx_fixup(struct usbnet *dev, struct sk_buff *skb)
 				return 0;
 			}
 
-			/* last frame in this batch */
-			if (skb->len == size) {
-				if (dev->net->features & NETIF_F_RXCSUM)
-					smsc95xx_rx_csum_offload(skb);
-				skb_trim(skb, skb->len - 4); /* remove fcs */
-				skb->truesize = size + sizeof(struct sk_buff);
-
-				return 1;
-			}
-
-			ax_skb = skb_clone(skb, GFP_ATOMIC);
+			ax_skb = netdev_alloc_skb_ip_align(dev->net, size);
 			if (unlikely(!ax_skb)) {
 				netdev_warn(dev->net, "Error allocating skb\n");
 				return 0;
 			}
 
-			ax_skb->len = size;
-			ax_skb->data = packet;
-			skb_set_tail_pointer(ax_skb, size);
+			memcpy(skb_put(ax_skb, size), packet, size);
 
 			if (dev->net->features & NETIF_F_RXCSUM)
 				smsc95xx_rx_csum_offload(ax_skb);
-			skb_trim(ax_skb, ax_skb->len - 4); /* remove fcs */
-			ax_skb->truesize = size + sizeof(struct sk_buff);
+			__skb_trim(ax_skb, ax_skb->len - 4); /* remove fcs */
 
 			usbnet_skb_return(dev, ax_skb);
 		}

^ permalink raw reply related

* Re: [PATCH v1 3/5] cgroup: Protect access to task_cls_classid() when built as module
From: Glauber Costa @ 2012-08-21  9:12 UTC (permalink / raw)
  To: Neil Horman
  Cc: Daniel Wagner, netdev-u79uwXL29TY76Z2rM5mHXA,
	cgroups-u79uwXL29TY76Z2rM5mHXA, Daniel Wagner, David S. Miller,
	Gao feng, Jamal Hadi Salim, John Fastabend, Li Zefan, Tejun Heo
In-Reply-To: <20120820170342.GC1734-B26myB8xz7F8NnZeBjwnZQMhkBWG/bsMQH7oEaQurus@public.gmane.org>

On 08/20/2012 09:03 PM, Neil Horman wrote:
>>> 2) Keep a separate internal counter to track the number of cgroup instances
>>> > > so that you only inc the static key on the first create and dec it on the last
>>> > > delete.
>> > 
>> > If I got you right, than this would not be different then direclty using
>> > static_key_slow_[inc|dec].
>> > 
> As long as a cgroup subsystems ->destroy method is only called when the
> subsystem is being removed, then I think thats correct.  I'm not 100% sure thats
> the case though.
> 
THAT is correct, but not the call itself. ->destroy() is called with the
cgroup_lock held, and there is a lockdep dependency created by cpuset
that prevents the cpu_hotplug lock, taken by static branch updates, to
be taken inside cgroup_lock.

So unless cpuset is fixed - which is a major work, we can't do
static_branch updates while holding the cgroup_lock.

^ permalink raw reply

* Re: binding UDP port 0 with SO_REUSEADDR
From: Jan Engelhardt @ 2012-08-21  9:09 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: Tobias S. Josefowitz, netdev
In-Reply-To: <1343926645.9299.303.camel@edumazet-glaptop>


On Thursday 2012-08-02 18:57, Eric Dumazet wrote:
>>SO_REUSEADDR
>>       Indicates  that  the rules used in validating addresses supplied
>>       in a bind(2) call should allow reuse of  local  addresses.   For
>>       AF_INET  sockets  this means that a socket may bind, except when
>>       there is an active listening socket bound to the address.   When
>>       the listening socket is bound to INADDR_ANY with a specific port
>>       then it is not possible to bind  to  this  port  for  any  local
>>       address.  Argument is an integer boolean flag.
>> 
>> I think the surprise-factor of the bind-0-behaviour even increased for
>> me. Is there a specific reason for handing out used ports when binding
>> port 0 with REUSEADDR?
>
>There is no concept of listening sockets for UDP.
>This documentation applies for TCP, and makes no sense for UDP.
>There is no value using 'port 0' and REUSEADDR on UDP, really.

Setting REUSEADDR on one UDP socket currently has the added effect
that bind(2) on another UDP socket for the same tuple will never
complain with -EADDRINUSE.

This behavior - willingly or not - is used in practice by socat(1)'s
udp-listen mode.

^ permalink raw reply

* Re: iputils git tree
From: Jan Engelhardt @ 2012-08-21  8:47 UTC (permalink / raw)
  To: Mike Frysinger; +Cc: YOSHIFUJI Hideaki, Noah Meyerhans, David Miller, netdev
In-Reply-To: <201208011121.35902.vapier@gentoo.org>

On Wednesday 2012-08-01 17:21, Mike Frysinger wrote:

>On Wednesday 01 August 2012 00:18:33 YOSHIFUJI Hideaki wrote:
>> In fact my first motivation was to integrate Debian, Fedora and
>> other distro updates in to single source.  I sometimes look
>> into their sources and integrate some of their changes.
>
>i think we should get bugging the maintainers to push to you rather than 
>forcing you to go out and locate patches.  especially when the patches are 
>often times underdocumented.

And half of iputils has been superseded or thelike.

* arping -> th's arping http://freecode.com/projects/arping
* ifenslave -> bonding's sysfs files
* tftpd -> tftpd-hpa

^ permalink raw reply

* RE: [PATCH] ethtool: don't overwrite useful bits in advertising bitfield
From: Johan Gunnarsson @ 2012-08-21  8:41 UTC (permalink / raw)
  To: Ben Hutchings; +Cc: netdev@vger.kernel.org, Mikael Starvik
In-Reply-To: <1345476178.2659.15.camel@bwh-desktop.uk.solarflarecom.com>



> -----Original Message-----
> From: netdev-owner@vger.kernel.org [mailto:netdev-
> owner@vger.kernel.org] On Behalf Of Ben Hutchings
> Sent: den 20 augusti 2012 17:23
> To: Johan Gunnarsson
> Cc: netdev@vger.kernel.org; Mikael Starvik
> Subject: Re: [PATCH] ethtool: don't overwrite useful bits in
> advertising bitfield
> 
> On Tue, 2012-08-14 at 16:15 +0200, Johan Gunnarsson wrote:
> > There are bits in this bitfield that we want to leave untouched
> (PAUSE
> > and ASYM_PAUSE bits) when changing other bits (speed and duplex
> bits.)
> > Previously, these were always overwritten to zero when running
> commands
> > like "ethtool -s eth0 speed 10 duplex full autoneg off".
> 
> This is right in principle, but the implementation isn't quite right.
> 
> > Signed-off-by: Johan Gunnarsson <johangu@axis.com>
> > ---
> >  ethtool.c |   45 +++++++++++++++++++++++++++++++--------------
> >  1 file changed, 31 insertions(+), 14 deletions(-)
> >
> > diff --git a/ethtool.c b/ethtool.c
> > index e573357..efa12c7 100644
> > --- a/ethtool.c
> > +++ b/ethtool.c
> > @@ -46,6 +46,18 @@
> >  #define MAX_ADDR_LEN	32
> >  #endif
> >
> > +#define ALL_ADVERTISED_MODES \
> > +	(ADVERTISED_10baseT_Half | \
> > +	 ADVERTISED_10baseT_Full | \
> > +	 ADVERTISED_100baseT_Half | \
> > +	 ADVERTISED_100baseT_Full | \
> > +	 ADVERTISED_1000baseT_Half | \
> > +	 ADVERTISED_1000baseT_Full | \
> > +	 ADVERTISED_2500baseX_Full | \
> > +	 ADVERTISED_10000baseT_Full | \
> > +	 ADVERTISED_20000baseMLD2_Full | \
> > +	 ADVERTISED_20000baseKR2_Full)
> 
> This is missing the new 40G modes (not a regression, I realise).

I'll add the 40G modes. There is also a bunch of 10G modes missing. Shall I add these too?

> 
> [...]
> > @@ -2405,19 +2421,20 @@ static int do_sset(struct cmd_context *ctx)
> >  			}
> >  			if (autoneg_wanted == AUTONEG_ENABLE &&
> >  			    advertising_wanted == 0) {
> > -				ecmd.advertising = ecmd.supported &
> > -					(ADVERTISED_10baseT_Half |
> > -					 ADVERTISED_10baseT_Full |
> > -					 ADVERTISED_100baseT_Half |
> > -					 ADVERTISED_100baseT_Full |
> > -					 ADVERTISED_1000baseT_Half |
> > -					 ADVERTISED_1000baseT_Full |
> > -					 ADVERTISED_2500baseX_Full |
> > -					 ADVERTISED_10000baseT_Full |
> > -					 ADVERTISED_20000baseMLD2_Full |
> > -					 ADVERTISED_20000baseKR2_Full);
> > +				/* Auto negotation enabled, but with
> > +				 * unspecified speed and duplex: enable all
> > +				 * supported speeds and duplexes.
> > +				 */
> > +				ecmd.advertising = (ecmd.advertising &
> > +					~ALL_ADVERTISED_MODES) |
> > +					(ALL_ADVERTISED_MODES & ecmd.supported);
> 
> Perhaps we should also warn if there's a 'supported' flag we don't
> recognise, because we don't know whether it's a link mode and we might
> be failing to enable/disable it as requested.

You mean if ecmd.supported has bits enabled that ALL_ADVERTISED_MODES hasn't? I don't think that's a good idea, because that happens very often (for example PAUSE bits in my case.)

> 
> >  			} else if (advertising_wanted > 0) {
> > -				ecmd.advertising = advertising_wanted;
> > +				/* Enable all requested modes */
> > +				ecmd.advertising = (ecmd.advertising &
> > +					~ALL_ADVERTISED_MODES) |
> > +					(advertising_wanted & ecmd.supported);
> 
> I don't think the '& ecmd.supported' here is right.  If an autoneg
> device supports some new link mode L that is not in
> ALL_ADVERTISED_MODES, but not link mode M which the user requested,
> then
> this can silently fail because the resulting advertising mask will
> include L but not M.
> 
> We should either use advertising_wanted unmasked and let the driver
> validate it, or report an error if it's not present in the supported
> mask.  I think we should be consistent with the following case, i.e.
> let
> the driver validate it.

How about remove "& ecmd.supported", but also warn if trying to add an unsupported mode? Similar to the previous case.

> 
> Ben.
> 
> > +			} else if (full_advertising_wanted > 0) {
> > +				ecmd.advertising = full_advertising_wanted;
> >  			}
> >
> >  			/* Try to perform the update. */
> 
> --
> Ben Hutchings, Staff Engineer, Solarflare
> Not speaking for my employer; that's the marketing department's job.
> They asked us to note that Solarflare product names are trademarked.
> 
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: coping with memory limitations and packet flooding in codel and fq_codel
From: Eric Dumazet @ 2012-08-21  8:38 UTC (permalink / raw)
  To: Dave Taht; +Cc: netdev, codel
In-Reply-To: <CAA93jw7=K8Vvp6N5qYZ8gaODaFyZtAEjGyQe2BDh9-RBsvVHcQ@mail.gmail.com>

On Mon, 2012-08-20 at 14:05 -0700, Dave Taht wrote:
> We've had a raft of deployment questions regarding fq_codel, memory
> use, and the impact of unresponsive flows
> over on the cerowrt development list.
> 
> sort of summary of the thread here, with some codel related ideas
> 
> https://lists.bufferbloat.net/pipermail/cerowrt-devel/2012-August/000423.html
> 
> some early data on wifi interactions here
> 
> https://lists.bufferbloat.net/pipermail/cerowrt-devel/2012-August/000407.html
> 

CC netdev

codel / fq_codel have a packet limit per qdisc, like any other linux
qdisc.

DEFAULT_CODEL_LIMIT = 1000 packets

fq_codel : 10240 packets

For memory constrained boxes, it would be wise to add a 'truesize' limit

Not a bytes limit based on skb->len sum, because a skb->len = 60 bytes
packet might consume far more memory (more than 2 Kbytes)

ath9k driver is known to deliver very fat skbs (skb->truesize is too
big)

One 'other way' to limit bloat would be to reallocate skbs when one
qdisc begins to use more than 25 % of its truesize limit

enqueue(q, skb)
{
	if (q->truesize_sum > q->truesize_limit / 4)
		skb = skb_reduce_truesize(skb, GFP_ATOMIC);
	...
}

This is what some drivers already do in their rx handler (this is called
copybreak), but this would do the copy (and extra cpu cost) only in
stress situations.

Note : we also have a very big struct skb_shared_info (320 bytes), we
probably could shrink it for packets without fragments.

^ permalink raw reply

* [net-next 6/6] igb: update to allow reading/setting MDI state
From: Jeff Kirsher @ 2012-08-21  8:37 UTC (permalink / raw)
  To: davem
  Cc: Jesse Brandeburg, netdev, gospo, sassmann, Carolyn Wyborny,
	Jeff Kirsher
In-Reply-To: <1345538275-1690-1-git-send-email-jeffrey.t.kirsher@intel.com>

From: Jesse Brandeburg <jesse.brandeburg@intel.com>

This is the implementation for igb to allow forcing MDI state
via ethtool, allowing users to work around some improperly
behaving switches.

Forcing in this driver is for now only allowed when auto-neg is
enabled.

Signed-off-by: Jesse Brandeburg <jesse.brandeburg@intel.com>
CC: Carolyn Wyborny <carolyn.wyborny@intel.com>
Tested-by: Aaron Brown aaron.f.brown@intel.com
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/igb/igb_ethtool.c | 42 ++++++++++++++++++++++++++++
 drivers/net/ethernet/intel/igb/igb_main.c    |  4 +++
 2 files changed, 46 insertions(+)

diff --git a/drivers/net/ethernet/intel/igb/igb_ethtool.c b/drivers/net/ethernet/intel/igb/igb_ethtool.c
index a19c84c..cf0e9fb 100644
--- a/drivers/net/ethernet/intel/igb/igb_ethtool.c
+++ b/drivers/net/ethernet/intel/igb/igb_ethtool.c
@@ -198,6 +198,19 @@ static int igb_get_settings(struct net_device *netdev, struct ethtool_cmd *ecmd)
 	}
 
 	ecmd->autoneg = hw->mac.autoneg ? AUTONEG_ENABLE : AUTONEG_DISABLE;
+
+	/* MDI-X => 2; MDI =>1; Invalid =>0 */
+	if (hw->phy.media_type == e1000_media_type_copper)
+		ecmd->eth_tp_mdix = hw->phy.is_mdix ? ETH_TP_MDI_X :
+						      ETH_TP_MDI;
+	else
+		ecmd->eth_tp_mdix = ETH_TP_MDI_INVALID;
+
+	if (hw->phy.mdix == AUTO_ALL_MODES)
+		ecmd->eth_tp_mdix_ctrl = ETH_TP_MDI_AUTO;
+	else
+		ecmd->eth_tp_mdix_ctrl = hw->phy.mdix;
+
 	return 0;
 }
 
@@ -214,6 +227,22 @@ static int igb_set_settings(struct net_device *netdev, struct ethtool_cmd *ecmd)
 		return -EINVAL;
 	}
 
+	/*
+	 * MDI setting is only allowed when autoneg enabled because
+	 * some hardware doesn't allow MDI setting when speed or
+	 * duplex is forced.
+	 */
+	if (ecmd->eth_tp_mdix_ctrl) {
+		if (hw->phy.media_type != e1000_media_type_copper)
+			return -EOPNOTSUPP;
+
+		if ((ecmd->eth_tp_mdix_ctrl != ETH_TP_MDI_AUTO) &&
+		    (ecmd->autoneg != AUTONEG_ENABLE)) {
+			dev_err(&adapter->pdev->dev, "forcing MDI/MDI-X state is not supported when link speed and/or duplex are forced\n");
+			return -EINVAL;
+		}
+	}
+
 	while (test_and_set_bit(__IGB_RESETTING, &adapter->state))
 		msleep(1);
 
@@ -227,12 +256,25 @@ static int igb_set_settings(struct net_device *netdev, struct ethtool_cmd *ecmd)
 			hw->fc.requested_mode = e1000_fc_default;
 	} else {
 		u32 speed = ethtool_cmd_speed(ecmd);
+		/* calling this overrides forced MDI setting */
 		if (igb_set_spd_dplx(adapter, speed, ecmd->duplex)) {
 			clear_bit(__IGB_RESETTING, &adapter->state);
 			return -EINVAL;
 		}
 	}
 
+	/* MDI-X => 2; MDI => 1; Auto => 3 */
+	if (ecmd->eth_tp_mdix_ctrl) {
+		/*
+		 * fix up the value for auto (3 => 0) as zero is mapped
+		 * internally to auto
+		 */
+		if (ecmd->eth_tp_mdix_ctrl == ETH_TP_MDI_AUTO)
+			hw->phy.mdix = AUTO_ALL_MODES;
+		else
+			hw->phy.mdix = ecmd->eth_tp_mdix_ctrl;
+	}
+
 	/* reset the link */
 	if (netif_running(adapter->netdev)) {
 		igb_down(adapter);
diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c
index b7c2d50..d4f47a0 100644
--- a/drivers/net/ethernet/intel/igb/igb_main.c
+++ b/drivers/net/ethernet/intel/igb/igb_main.c
@@ -6676,6 +6676,10 @@ int igb_set_spd_dplx(struct igb_adapter *adapter, u32 spd, u8 dplx)
 	default:
 		goto err_inval;
 	}
+
+	/* clear MDI, MDI(-X) override is only allowed when autoneg enabled */
+	adapter->hw.phy.mdix = AUTO_ALL_MODES;
+
 	return 0;
 
 err_inval:
-- 
1.7.11.4

^ permalink raw reply related

* [net-next 5/6] e1000e: implement MDI/MDI-X control
From: Jeff Kirsher @ 2012-08-21  8:37 UTC (permalink / raw)
  To: davem
  Cc: Jesse Brandeburg, netdev, gospo, sassmann, bruce.w.allan,
	n.poppelier, bastien, jsveiga, Jeff Kirsher
In-Reply-To: <1345538275-1690-1-git-send-email-jeffrey.t.kirsher@intel.com>

From: Jesse Brandeburg <jesse.brandeburg@intel.com>

Some users report issues with link failing when connected to certain
switches.  This gives the user the ability to control the MDI state
from the driver, allowing users to work around some improperly
behaving switches.

Forcing in this driver is for now only allowed when auto-neg is
enabled.

This is in regards to the related ethtool app patch and
bugzilla.kernel.org bug 11998

Signed-off-by: Jesse Brandeburg <jesse.brandeburg@intel.com>
CC: bruce.w.allan@intel.com
CC: n.poppelier@xs4all.nl
CC: bastien@durel.org
CC: jsveiga@it.eng.br
Tested-by: Aaron Brown aaron.f.brown@intel.com
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/e1000e/ethtool.c | 41 +++++++++++++++++++++++++++--
 1 file changed, 39 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/intel/e1000e/ethtool.c b/drivers/net/ethernet/intel/e1000e/ethtool.c
index 0349e24..2e76f06 100644
--- a/drivers/net/ethernet/intel/e1000e/ethtool.c
+++ b/drivers/net/ethernet/intel/e1000e/ethtool.c
@@ -199,6 +199,11 @@ static int e1000_get_settings(struct net_device *netdev,
 	else
 		ecmd->eth_tp_mdix = ETH_TP_MDI_INVALID;
 
+	if (hw->phy.mdix == AUTO_ALL_MODES)
+		ecmd->eth_tp_mdix_ctrl = ETH_TP_MDI_AUTO;
+	else
+		ecmd->eth_tp_mdix_ctrl = hw->phy.mdix;
+
 	return 0;
 }
 
@@ -241,6 +246,10 @@ static int e1000_set_spd_dplx(struct e1000_adapter *adapter, u32 spd, u8 dplx)
 	default:
 		goto err_inval;
 	}
+
+	/* clear MDI, MDI(-X) override is only allowed when autoneg enabled */
+	adapter->hw.phy.mdix = AUTO_ALL_MODES;
+
 	return 0;
 
 err_inval:
@@ -264,6 +273,22 @@ static int e1000_set_settings(struct net_device *netdev,
 		return -EINVAL;
 	}
 
+	/*
+	 * MDI setting is only allowed when autoneg enabled because
+	 * some hardware doesn't allow MDI setting when speed or
+	 * duplex is forced.
+	 */
+	if (ecmd->eth_tp_mdix_ctrl) {
+		if (hw->phy.media_type != e1000_media_type_copper)
+			return -EOPNOTSUPP;
+
+		if ((ecmd->eth_tp_mdix_ctrl != ETH_TP_MDI_AUTO) &&
+		    (ecmd->autoneg != AUTONEG_ENABLE)) {
+			e_err("forcing MDI/MDI-X state is not supported when link speed and/or duplex are forced\n");
+			return -EINVAL;
+		}
+	}
+
 	while (test_and_set_bit(__E1000_RESETTING, &adapter->state))
 		usleep_range(1000, 2000);
 
@@ -282,20 +307,32 @@ static int e1000_set_settings(struct net_device *netdev,
 			hw->fc.requested_mode = e1000_fc_default;
 	} else {
 		u32 speed = ethtool_cmd_speed(ecmd);
+		/* calling this overrides forced MDI setting */
 		if (e1000_set_spd_dplx(adapter, speed, ecmd->duplex)) {
 			clear_bit(__E1000_RESETTING, &adapter->state);
 			return -EINVAL;
 		}
 	}
 
+	/* MDI-X => 2; MDI => 1; Auto => 3 */
+	if (ecmd->eth_tp_mdix_ctrl) {
+		/*
+		 * fix up the value for auto (3 => 0) as zero is mapped
+		 * internally to auto
+		 */
+		if (ecmd->eth_tp_mdix_ctrl == ETH_TP_MDI_AUTO)
+			hw->phy.mdix = AUTO_ALL_MODES;
+		else
+			hw->phy.mdix = ecmd->eth_tp_mdix_ctrl;
+	}
+
 	/* reset the link */
 
 	if (netif_running(adapter->netdev)) {
 		e1000e_down(adapter);
 		e1000e_up(adapter);
-	} else {
+	} else
 		e1000e_reset(adapter);
-	}
 
 	clear_bit(__E1000_RESETTING, &adapter->state);
 	return 0;
-- 
1.7.11.4

^ permalink raw reply related

* [net-next 4/6] e1000: configure and read MDI settings
From: Jeff Kirsher @ 2012-08-21  8:37 UTC (permalink / raw)
  To: davem; +Cc: Jesse Brandeburg, netdev, gospo, sassmann, Tushar Dave,
	Jeff Kirsher
In-Reply-To: <1345538275-1690-1-git-send-email-jeffrey.t.kirsher@intel.com>

From: Jesse Brandeburg <jesse.brandeburg@intel.com>

This is the implementation in e1000 to allow ethtool to force
MDI state, allowing users to work around some improperly
behaving switches.

Forcing in this driver is for now only allowed when auto-neg is enabled.

To use must have the matching version of ethtool app that supports
this functionality.

Signed-off-by: Jesse Brandeburg <jesse.brandeburg@intel.com>
CC: Tushar Dave <tushar.n.dave@intel.com>
Tested-by: Aaron Brown aaron.f.brown@intel.com
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/e1000/e1000_ethtool.c | 39 ++++++++++++++++++++++++
 drivers/net/ethernet/intel/e1000/e1000_main.c    |  4 +++
 2 files changed, 43 insertions(+)

diff --git a/drivers/net/ethernet/intel/e1000/e1000_ethtool.c b/drivers/net/ethernet/intel/e1000/e1000_ethtool.c
index 736a7d9..9089d00 100644
--- a/drivers/net/ethernet/intel/e1000/e1000_ethtool.c
+++ b/drivers/net/ethernet/intel/e1000/e1000_ethtool.c
@@ -174,6 +174,20 @@ static int e1000_get_settings(struct net_device *netdev,
 
 	ecmd->autoneg = ((hw->media_type == e1000_media_type_fiber) ||
 			 hw->autoneg) ? AUTONEG_ENABLE : AUTONEG_DISABLE;
+
+	/* MDI-X => 1; MDI => 0 */
+	if ((hw->media_type == e1000_media_type_copper) &&
+	    netif_carrier_ok(netdev))
+		ecmd->eth_tp_mdix = (!!adapter->phy_info.mdix_mode ?
+							ETH_TP_MDI_X :
+							ETH_TP_MDI);
+	else
+		ecmd->eth_tp_mdix = ETH_TP_MDI_INVALID;
+
+	if (hw->mdix == AUTO_ALL_MODES)
+		ecmd->eth_tp_mdix_ctrl = ETH_TP_MDI_AUTO;
+	else
+		ecmd->eth_tp_mdix_ctrl = hw->mdix;
 	return 0;
 }
 
@@ -183,6 +197,22 @@ static int e1000_set_settings(struct net_device *netdev,
 	struct e1000_adapter *adapter = netdev_priv(netdev);
 	struct e1000_hw *hw = &adapter->hw;
 
+	/*
+	 * MDI setting is only allowed when autoneg enabled because
+	 * some hardware doesn't allow MDI setting when speed or
+	 * duplex is forced.
+	 */
+	if (ecmd->eth_tp_mdix_ctrl) {
+		if (hw->media_type != e1000_media_type_copper)
+			return -EOPNOTSUPP;
+
+		if ((ecmd->eth_tp_mdix_ctrl != ETH_TP_MDI_AUTO) &&
+		    (ecmd->autoneg != AUTONEG_ENABLE)) {
+			e_err(drv, "forcing MDI/MDI-X state is not supported when link speed and/or duplex are forced\n");
+			return -EINVAL;
+		}
+	}
+
 	while (test_and_set_bit(__E1000_RESETTING, &adapter->flags))
 		msleep(1);
 
@@ -199,12 +229,21 @@ static int e1000_set_settings(struct net_device *netdev,
 		ecmd->advertising = hw->autoneg_advertised;
 	} else {
 		u32 speed = ethtool_cmd_speed(ecmd);
+		/* calling this overrides forced MDI setting */
 		if (e1000_set_spd_dplx(adapter, speed, ecmd->duplex)) {
 			clear_bit(__E1000_RESETTING, &adapter->flags);
 			return -EINVAL;
 		}
 	}
 
+	/* MDI-X => 2; MDI => 1; Auto => 3 */
+	if (ecmd->eth_tp_mdix_ctrl) {
+		if (ecmd->eth_tp_mdix_ctrl == ETH_TP_MDI_AUTO)
+			hw->mdix = AUTO_ALL_MODES;
+		else
+			hw->mdix = ecmd->eth_tp_mdix_ctrl;
+	}
+
 	/* reset the link */
 
 	if (netif_running(adapter->netdev)) {
diff --git a/drivers/net/ethernet/intel/e1000/e1000_main.c b/drivers/net/ethernet/intel/e1000/e1000_main.c
index 3bfbb8d..0ae2fcf 100644
--- a/drivers/net/ethernet/intel/e1000/e1000_main.c
+++ b/drivers/net/ethernet/intel/e1000/e1000_main.c
@@ -4939,6 +4939,10 @@ int e1000_set_spd_dplx(struct e1000_adapter *adapter, u32 spd, u8 dplx)
 	default:
 		goto err_inval;
 	}
+
+	/* clear MDI, MDI(-X) override is only allowed when autoneg enabled */
+	hw->mdix = AUTO_ALL_MODES;
+
 	return 0;
 
 err_inval:
-- 
1.7.11.4

^ permalink raw reply related

* [net-next 3/6] igb: implement 580 MDI setting support
From: Jeff Kirsher @ 2012-08-21  8:37 UTC (permalink / raw)
  To: davem; +Cc: Jesse Brandeburg, netdev, gospo, sassmann, Jeff Kirsher
In-Reply-To: <1345538275-1690-1-git-send-email-jeffrey.t.kirsher@intel.com>

From: Jesse Brandeburg <jesse.brandeburg@intel.com>

In order for igb to support MDI setting support via
ethtool this code is needed to allow setting the MDI state
via software.

This is in regards to the related ethtool patch

Signed-off-by: Jesse Brandeburg <jesse.brandeburg@intel.com>
Tested-by: Aaron Brown aaron.f.brown@intel.com
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/igb/e1000_phy.c | 29 +++++++++++++++++++++++++++--
 drivers/net/ethernet/intel/igb/e1000_phy.h |  5 +++--
 2 files changed, 30 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/intel/igb/e1000_phy.c b/drivers/net/ethernet/intel/igb/e1000_phy.c
index 7be98b6..3404bc7 100644
--- a/drivers/net/ethernet/intel/igb/e1000_phy.c
+++ b/drivers/net/ethernet/intel/igb/e1000_phy.c
@@ -464,6 +464,32 @@ s32 igb_copper_link_setup_82580(struct e1000_hw *hw)
 	phy_data |= I82580_CFG_ENABLE_DOWNSHIFT;
 
 	ret_val = phy->ops.write_reg(hw, I82580_CFG_REG, phy_data);
+	if (ret_val)
+		goto out;
+
+	/* Set MDI/MDIX mode */
+	ret_val = phy->ops.read_reg(hw, I82580_PHY_CTRL_2, &phy_data);
+	if (ret_val)
+		goto out;
+	phy_data &= ~I82580_PHY_CTRL2_MDIX_CFG_MASK;
+	/*
+	 * Options:
+	 *   0 - Auto (default)
+	 *   1 - MDI mode
+	 *   2 - MDI-X mode
+	 */
+	switch (hw->phy.mdix) {
+	case 1:
+		break;
+	case 2:
+		phy_data |= I82580_PHY_CTRL2_MANUAL_MDIX;
+		break;
+	case 0:
+	default:
+		phy_data |= I82580_PHY_CTRL2_AUTO_MDI_MDIX;
+		break;
+	}
+	ret_val = hw->phy.ops.write_reg(hw, I82580_PHY_CTRL_2, phy_data);
 
 out:
 	return ret_val;
@@ -2246,8 +2272,7 @@ s32 igb_phy_force_speed_duplex_82580(struct e1000_hw *hw)
 	if (ret_val)
 		goto out;
 
-	phy_data &= ~I82580_PHY_CTRL2_AUTO_MDIX;
-	phy_data &= ~I82580_PHY_CTRL2_FORCE_MDI_MDIX;
+	phy_data &= ~I82580_PHY_CTRL2_MDIX_CFG_MASK;
 
 	ret_val = phy->ops.write_reg(hw, I82580_PHY_CTRL_2, phy_data);
 	if (ret_val)
diff --git a/drivers/net/ethernet/intel/igb/e1000_phy.h b/drivers/net/ethernet/intel/igb/e1000_phy.h
index 34e4061..6ac3299 100644
--- a/drivers/net/ethernet/intel/igb/e1000_phy.h
+++ b/drivers/net/ethernet/intel/igb/e1000_phy.h
@@ -111,8 +111,9 @@ s32  igb_check_polarity_m88(struct e1000_hw *hw);
 #define I82580_PHY_STATUS2_SPEED_100MBPS  0x0100
 
 /* I82580 PHY Control 2 */
-#define I82580_PHY_CTRL2_AUTO_MDIX        0x0400
-#define I82580_PHY_CTRL2_FORCE_MDI_MDIX   0x0200
+#define I82580_PHY_CTRL2_MANUAL_MDIX      0x0200
+#define I82580_PHY_CTRL2_AUTO_MDI_MDIX    0x0400
+#define I82580_PHY_CTRL2_MDIX_CFG_MASK    0x0600
 
 /* I82580 PHY Diagnostics Status */
 #define I82580_DSTATUS_CABLE_LENGTH       0x03FC
-- 
1.7.11.4

^ permalink raw reply related

* [net-next 2/6] e1000e: implement 82577/579 MDI setting support
From: Jeff Kirsher @ 2012-08-21  8:37 UTC (permalink / raw)
  To: davem
  Cc: Bruce W Allan, netdev, gospo, sassmann, Jesse Brandeburg,
	Jeff Kirsher
In-Reply-To: <1345538275-1690-1-git-send-email-jeffrey.t.kirsher@intel.com>

From: Bruce W Allan <bruce.w.allan@intel.com>

In order for e1000e to support MDI setting support via
ethtool this code is needed to allow setting the MDI state
via software.

This is in regards to the related ethtool patch and
fixes bugzilla.kernel.org bug 11998

Signed-off-by: Bruce W Allan <bruce.w.allan@intel.com>
Signed-off-by: Jesse Brandeburg <jesse.brandeburg@intel.com>
Tested-by: Aaron Brown aaron.f.brown@intel.com
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/e1000e/phy.c | 31 +++++++++++++++++++++++++++++--
 1 file changed, 29 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/intel/e1000e/phy.c b/drivers/net/ethernet/intel/e1000e/phy.c
index b860d4f..fc62a3f 100644
--- a/drivers/net/ethernet/intel/e1000e/phy.c
+++ b/drivers/net/ethernet/intel/e1000e/phy.c
@@ -84,8 +84,9 @@ static const u16 e1000_igp_2_cable_length_table[] = {
 #define I82577_PHY_STATUS2_SPEED_1000MBPS 0x0200
 
 /* I82577 PHY Control 2 */
-#define I82577_PHY_CTRL2_AUTO_MDIX        0x0400
-#define I82577_PHY_CTRL2_FORCE_MDI_MDIX   0x0200
+#define I82577_PHY_CTRL2_MANUAL_MDIX      0x0200
+#define I82577_PHY_CTRL2_AUTO_MDI_MDIX    0x0400
+#define I82577_PHY_CTRL2_MDIX_CFG_MASK    0x0600
 
 /* I82577 PHY Diagnostics Status */
 #define I82577_DSTATUS_CABLE_LENGTH       0x03FC
@@ -702,6 +703,32 @@ s32 e1000_copper_link_setup_82577(struct e1000_hw *hw)
 	if (ret_val)
 		return ret_val;
 
+	/* Set MDI/MDIX mode */
+	ret_val = e1e_rphy(hw, I82577_PHY_CTRL_2, &phy_data);
+	if (ret_val)
+		return ret_val;
+	phy_data &= ~I82577_PHY_CTRL2_MDIX_CFG_MASK;
+	/*
+	 * Options:
+	 *   0 - Auto (default)
+	 *   1 - MDI mode
+	 *   2 - MDI-X mode
+	 */
+	switch (hw->phy.mdix) {
+	case 1:
+		break;
+	case 2:
+		phy_data |= I82577_PHY_CTRL2_MANUAL_MDIX;
+		break;
+	case 0:
+	default:
+		phy_data |= I82577_PHY_CTRL2_AUTO_MDI_MDIX;
+		break;
+	}
+	ret_val = e1e_wphy(hw, I82577_PHY_CTRL_2, phy_data);
+	if (ret_val)
+		return ret_val;
+
 	return e1000_set_master_slave_mode(hw);
 }
 
-- 
1.7.11.4

^ permalink raw reply related

* [net-next 1/6] ethtool.h: MDI setting support
From: Jeff Kirsher @ 2012-08-21  8:37 UTC (permalink / raw)
  To: davem
  Cc: Jesse Brandeburg, netdev, gospo, sassmann, Ben Hutchings,
	Jeff Kirsher
In-Reply-To: <1345538275-1690-1-git-send-email-jeffrey.t.kirsher@intel.com>

From: Jesse Brandeburg <jesse.brandeburg@intel.com>

This change modifies the core ethtool struct to allow a driver to
support setting of MDI/MDI-X state for twisted pair wiring.  This
change uses a previously reserved u8 and should not change any
binary compatibility of ethtool.

Also as per Ben Hutchings' suggestion, the capabilities are
stored in a separate byte so the driver can report if it supports
changing settings.

see thread: http://kerneltrap.org/mailarchive/linux-netdev/2010/11/17/6289820/thread

see ethtool patches titled:
ethtool: allow setting MDI-X state

Signed-off-by: Jesse Brandeburg <jesse.brandeburg@intel.com>
CC: Ben Hutchings <bhutchings@solarflare.com>
Tested-by: Aaron Brown aaron.f.brown@intel.com
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 include/linux/ethtool.h | 17 +++++++++++------
 1 file changed, 11 insertions(+), 6 deletions(-)

diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h
index 21eff41..fcb4f8e 100644
--- a/include/linux/ethtool.h
+++ b/include/linux/ethtool.h
@@ -45,8 +45,10 @@ struct ethtool_cmd {
 				 * bits) in Mbps. Please use
 				 * ethtool_cmd_speed()/_set() to
 				 * access it */
-	__u8	eth_tp_mdix;
-	__u8	reserved2;
+	__u8	eth_tp_mdix;	/* twisted pair MDI-X status */
+	__u8    eth_tp_mdix_ctrl; /* twisted pair MDI-X control, when set,
+				   * link should be renegotiated if necessary
+				   */
 	__u32	lp_advertising;	/* Features the link partner advertises */
 	__u32	reserved[2];
 };
@@ -1229,10 +1231,13 @@ struct ethtool_ops {
 #define AUTONEG_DISABLE		0x00
 #define AUTONEG_ENABLE		0x01
 
-/* Mode MDI or MDI-X */
-#define ETH_TP_MDI_INVALID	0x00
-#define ETH_TP_MDI		0x01
-#define ETH_TP_MDI_X		0x02
+/* MDI or MDI-X status/control - if MDI/MDI_X/AUTO is set then
+ * the driver is required to renegotiate link
+ */
+#define ETH_TP_MDI_INVALID	0x00 /* status: unknown; control: unsupported */
+#define ETH_TP_MDI		0x01 /* status: MDI;     control: force MDI */
+#define ETH_TP_MDI_X		0x02 /* status: MDI-X;   control: force MDI-X */
+#define ETH_TP_MDI_AUTO		0x03 /*                  control: auto-select */
 
 /* Wake-On-Lan options. */
 #define WAKE_PHY		(1 << 0)
-- 
1.7.11.4

^ 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