Netdev List
 help / color / mirror / Atom feed
* Re: [E1000-devel] [PATCH net-next] igbvf: fix setting addr_assign_type if PF is up
From: Andy Gospodarek @ 2013-01-17  1:10 UTC (permalink / raw)
  To: Williams, Mitch A
  Cc: Rose, Gregory V, Stefan Assmann, netdev@vger.kernel.org,
	e1000-devel@lists.sourceforge.net
In-Reply-To: <AAEA33E297BCAC4B9BB20A7C2DF0AB8D2A3468AE@FMSMSX107.amr.corp.intel.com>

On Wed, Jan 16, 2013 at 7:42 PM, Williams, Mitch A
<mitch.a.williams@intel.com> wrote:
>
> > -----Original Message-----
> > From: Rose, Gregory V
> > Sent: Tuesday, January 15, 2013 10:32 AM
> > To: Andy Gospodarek
> > Cc: Williams, Mitch A; Stefan Assmann; netdev@vger.kernel.org; e1000-
> > devel@lists.sourceforge.net
> > Subject: Re: [E1000-devel] [PATCH net-next] igbvf: fix setting
> > addr_assign_type if PF is up
> >
> > On Mon, 14 Jan 2013 17:25:42 -0500
> > Andy Gospodarek <andy@greyhouse.net> wrote:
> >
> > > On Wed, Jan 09, 2013 at 01:37:45PM -0800, Greg Rose wrote:
> > > > On Wed, 9 Jan 2013 18:56:36 +0000
> > > > "Williams, Mitch A" <mitch.a.williams@intel.com> wrote:
> > > >
> > > > > > >> When the PF is up and igbvf is loaded the MAC address is not
> > > > > > >> generated using eth_hw_addr_random(). This results in
> > > > > > >> addr_assign_type not to be set.
> > > > > > >> Make sure it gets set.
> > > > > > >>
> > > > > > >
> > > > > > > NAK - In this case, the address may or may not be random. The
> > > > > > > user may have (and should have!) explicitly set this address
> > > > > > > from the host to ensure that the VF device receives the same
> > > > > > > address each time it
> > > > > > boots.
> > > > > >
> > > > > > Maybe you can give me some advice on this then. Why is there
> > > > > > different behaviour depending on the PF being up or down? The
> > > > > > problem I'm facing is that if the user did not set a MAC address
> > > > > > for the VF manually and the PF is up during igbvf_probe it will
> > > > > > not be labelled as random although it is.
> > > > > > What about checking IGB_VF_FLAG_PF_SET_MAC and only set
> > > > > > NET_ADDR_RANDOM if the flag is cleared?
> > > > > >
> > > > >
> > > > > The difference in behavior is because we cannot get any MAC
> > > > > address at all if the PF is down. The interface won't operate at
> > > > > all in this case, but if the PF comes up sometime later, we can
> > > > > start working. The other alternative is to leave the MAC address
> > > > > as all zeros and forcing the user to assign an address manually.
> > > > > We chose to use a random address to at least give it a chance of
> > > > > working once the PF woke up.
> > > >
> > > > Having been around at the inception of SR-IOV in Linux I recall that
> > > > the primary reason we used a random ethernet address was so that the
> > > > VF could at least work because there was no infrastructure to allow
> > > > the host administrator to set the MAC address of the VF.
> > > > This hobbled testing and validation because the user would have to
> > > > go to each VM and use a command local to the VM to set the VF MAC
> > > > address to some LAA via ifconfig or ip.  When testing large numbers
> > > > of VFs this was a definite pain.
> > > >
> > > > Now that has changed and I wonder if maybe we shouldn't back out the
> > > > random ethernet address assignment and go ahead with all zeros,
> > > > leaving the device non-functional until the user has intentionally
> > > > set either an LAA through the VF itself, or an administratively
> > > > assigned MAC through the ip tool via the PF.
> > > >
> > > > Use of the random MAC address is not recommended by Intel's own best
> > > > known methods literature, it was used mostly so that we could get
> > > > the technology working and it should probably be at least considered
> > > > for deprecation or out right elimination.
> > > >
> > >
> > > It would be great to remove the bits that created random MAC addresses
> > > for VFs, but wouldn't that break Linus' rule to "not break userspace"
> > > if it was removed?
> >
> > It may, I'm not sure but before we make any changes we'd want to do our
> > due diligence.
> >
> > >
> > > There are 2 options that immediately come to mind when looking to
> > > resolve this:
> > >
> > > 1.  Use some of the left-over bits in the mailbox messages to pass
> > > along a flag with the E1000_VF_RESET messages to indicate whether the
> > > MAC was randomly generated.  This would be pretty easy, but there
> > > could be compatibility issues for a while.
> >
> > We recently introduced the concept of mailbox message API versions in
> > our PF and VF drivers to handle this sort of thing.  We could probably
> > leverage that method to introduce a new API version that supports the
> > additional bits in the reset message.  It would only be used if the VF
> > could negotiate to the proper mailbox message API version with the PF.
> >
> > >
> > > 2.  Default to a MAC address of all zeros, and as a device with
> > > all-zeros for a MAC is brought up, randomly create one with
> > > eth_hw_addr_random.  This may not immediately help cases where device
> > > assignment are a problem, but it would ensure that any device with a
> > > random MAC as assigned by the kernel, would have NET_ADDR_RANDOM set
> > > in addr_assign_type.
> >
> > Thanks for the suggestions.  We're considering some changes in this area
> > but we (Intel) need to give this a lot of thought and right now we're
> > just in a preliminary discussion mode about it.  Stay tuned.
> >
> > - Greg
>
> OK, here's what I'm thinking. We don't need to change the communications
> protocol for this, and it shouldn't break userspace.
>
> First, have the PF driver quit assigning random addresses. It will either
> give the VF the address assigned by the administrator, or it will give
> all zeros.
>
> Second, modify the VF driver init sequence slightly. If it gets all
> zeros from the PF driver, then it should give itself a random address
> and set NET_ADDR_RANDOM.
>
> If we do it this way, the VF will still come up with a random address if
> one has not been assigned, and it will always know whether or not the
> address that it is using is random.
>
> If there are no objections, I'll try to get some patches done in the next
> few days and get them into our internal test queue. These would then
> escape into the real world in a few weeks.
>
> -Mitch

Oh, I like this even better than my proposed option 2.  I say, go for it.

^ permalink raw reply

* Re: [PATCH V6 3/3] tuntap: allow polling/writing/reading when detached
From: Jason Wang @ 2013-01-17  1:16 UTC (permalink / raw)
  To: Michael S. Tsirkin; +Cc: davem, netdev, linux-kernel
In-Reply-To: <20130116170356.GC4257@redhat.com>

On 01/17/2013 01:03 AM, Michael S. Tsirkin wrote:
> On Wed, Jan 16, 2013 at 11:44:38PM +0800, Jason Wang wrote:
>> We forbid polling, writing and reading when the file were detached, this may
>> complex the user in several cases:
>>
>> - when guest pass some buffers to vhost/qemu and then disable some queues,
>>   host/qemu needs to do its own cleanup on those buffers which is complex
>>   sometimes. We can do this simply by allowing a user can still write to an
>>   disabled queue. Write to an disabled queue will cause the packet pass to the
>>   kernel and read will get nothing.
>> - align the polling behavior with macvtap which never fails when the queue is
>>   created. This can simplify the polling errors handling of its user (e.g vhost)
>>
>> In order to achieve this, tfile->tun were not assign to NULL when detached. And
>> tfile->tun were converted to be RCU protected in order to let the data path can
>> check whether the file is deated in a lockless manner. This will be used to
>> prevent the flow caches from being updated for a detached queue.
>>
>> Signed-off-by: Jason Wang <jasowang@redhat.com>
>> ---
>>  drivers/net/tun.c |   45 ++++++++++++++++++++++++++-------------------
>>  1 files changed, 26 insertions(+), 19 deletions(-)
>>
>> diff --git a/drivers/net/tun.c b/drivers/net/tun.c
>> index c81680d..ec539a9 100644
>> --- a/drivers/net/tun.c
>> +++ b/drivers/net/tun.c
>> @@ -139,7 +139,7 @@ struct tun_file {
>>  	unsigned int flags;
>>  	u16 queue_index;
>>  	struct list_head next;
>> -	struct tun_struct *detached;
>> +	struct tun_struct __rcu *detached;
>>  };
>>  
>>  struct tun_flow_entry {
>> @@ -295,11 +295,12 @@ static void tun_flow_cleanup(unsigned long data)
>>  }
>>  
>>  static void tun_flow_update(struct tun_struct *tun, u32 rxhash,
>> -			    u16 queue_index)
>> +			    struct tun_file *tfile)
>>  {
>>  	struct hlist_head *head;
>>  	struct tun_flow_entry *e;
>>  	unsigned long delay = tun->ageing_time;
>> +	u16 queue_index = tfile->queue_index;
>>  
>>  	if (!rxhash)
>>  		return;
>> @@ -308,7 +309,7 @@ static void tun_flow_update(struct tun_struct *tun, u32 rxhash,
>>  
>>  	rcu_read_lock();
>>  
>> -	if (tun->numqueues == 1)
>> +	if (tun->numqueues == 1 || rcu_dereference(tfile->detached))
>>  		goto unlock;
>>  
>>  	e = tun_flow_find(head, rxhash);
> Sorry, still an issue with this one.

No problem, thanks for the checking.
>
>                 u16 index = tfile->queue_index;
>                 BUG_ON(index >= tun->numqueues);
>                 dev = tun->dev;
>
>                 rcu_assign_pointer(tun->tfiles[index],
>                                    tun->tfiles[tun->numqueues - 1]);
>                 rcu_assign_pointer(tfile->tun, NULL);
>                 ntfile = rtnl_dereference(tun->tfiles[index]);
>                 ntfile->queue_index = index;
>
>                 --tun->numqueues;
>                 if (clean)
>                         sock_put(&tfile->sk);
>                 else
>                         tun_disable_queue(tun, tfile);
>
> You should first disable queue then synchronize network
> only then play with tfiles array.
> As it is you might have removed file from array but
> did not set detached flag yet, so queue_index
> above is stable.

I think the code is ok here. With this patch, before synchronize_net(), 
the only thing we do for the tfile that will be detached is to set the
tfile->detached (tun_disable_queue), and the queue_index is kept
unchanged. So if the data path don't see the new value of detached, it
still can treat the tfile is undetached and do the sending and receiving
as usual. We only do the cleanup after the synchronization which all
reader are guaranteed to see the new detached value.

For the tfile that will be moved to the new place, some (should be very
little) OOO will occur which I think is acceptable and can be optimized
in the future.
>
> On enable, clear detached last thing.
>
>> @@ -384,16 +385,16 @@ static void tun_set_real_num_queues(struct tun_struct *tun)
>>  
>>  static void tun_disable_queue(struct tun_struct *tun, struct tun_file *tfile)
>>  {
>> -	tfile->detached = tun;
>> +	rcu_assign_pointer(tfile->detached, tun);
>>  	list_add_tail(&tfile->next, &tun->disabled);
>>  	++tun->numdisabled;
>>  }
>>  
>>  static struct tun_struct *tun_enable_queue(struct tun_file *tfile)
>>  {
>> -	struct tun_struct *tun = tfile->detached;
>> +	struct tun_struct *tun = rtnl_dereference(tfile->detached);
>>  
>> -	tfile->detached = NULL;
>> +	rcu_assign_pointer(tfile->detached, NULL);
>>  	list_del_init(&tfile->next);
>>  	--tun->numdisabled;
>>  	return tun;
>> @@ -402,26 +403,27 @@ static struct tun_struct *tun_enable_queue(struct tun_file *tfile)
>>  static void __tun_detach(struct tun_file *tfile, bool clean)
>>  {
>>  	struct tun_file *ntfile;
>> -	struct tun_struct *tun;
>> +	struct tun_struct *tun, *detached;
>>  	struct net_device *dev;
>>  
>>  	tun = rtnl_dereference(tfile->tun);
>> +	detached = rtnl_dereference(tfile->detached);
>>  
>> -	if (tun) {
>> +	if (tun && !detached) {
>>  		u16 index = tfile->queue_index;
>>  		BUG_ON(index >= tun->numqueues);
>>  		dev = tun->dev;
>>  
>>  		rcu_assign_pointer(tun->tfiles[index],
>>  				   tun->tfiles[tun->numqueues - 1]);
>> -		rcu_assign_pointer(tfile->tun, NULL);
>>  		ntfile = rtnl_dereference(tun->tfiles[index]);
>>  		ntfile->queue_index = index;
>>  
>>  		--tun->numqueues;
>> -		if (clean)
>> +		if (clean) {
>> +			rcu_assign_pointer(tfile->tun, NULL);
>>  			sock_put(&tfile->sk);
>> -		else
>> +		} else
>>  			tun_disable_queue(tun, tfile);
>>  
>>  		synchronize_net();
>> @@ -429,7 +431,7 @@ static void __tun_detach(struct tun_file *tfile, bool clean)
>>  		/* Drop read queue */
>>  		skb_queue_purge(&tfile->sk.sk_receive_queue);
>>  		tun_set_real_num_queues(tun);
>> -	} else if (tfile->detached && clean) {
>> +	} else if (detached && clean) {
>>  		tun = tun_enable_queue(tfile);
>>  		sock_put(&tfile->sk);
>>  	}
>> @@ -466,6 +468,10 @@ static void tun_detach_all(struct net_device *dev)
>>  		rcu_assign_pointer(tfile->tun, NULL);
>>  		--tun->numqueues;
>>  	}
>> +	list_for_each_entry(tfile, &tun->disabled, next) {
>> +		wake_up_all(&tfile->wq.wait);
>> +		rcu_assign_pointer(tfile->tun, NULL);
>> +	}
>>  	BUG_ON(tun->numqueues != 0);
>>  
>>  	synchronize_net();
>> @@ -496,7 +502,7 @@ static int tun_attach(struct tun_struct *tun, struct file *file)
>>  		goto out;
>>  
>>  	err = -EINVAL;
>> -	if (rtnl_dereference(tfile->tun))
>> +	if (rtnl_dereference(tfile->tun) && !rtnl_dereference(tfile->detached))
>>  		goto out;
>>  
>>  	err = -EBUSY;
>> @@ -504,7 +510,7 @@ static int tun_attach(struct tun_struct *tun, struct file *file)
>>  		goto out;
>>  
>>  	err = -E2BIG;
>> -	if (!tfile->detached &&
>> +	if (!rtnl_dereference(tfile->detached) &&
>>  	    tun->numqueues + tun->numdisabled == MAX_TAP_QUEUES)
>>  		goto out;
>>  
>> @@ -521,7 +527,7 @@ static int tun_attach(struct tun_struct *tun, struct file *file)
>>  	rcu_assign_pointer(tun->tfiles[tun->numqueues], tfile);
>>  	tun->numqueues++;
>>  
>> -	if (tfile->detached)
>> +	if (rtnl_dereference(tfile->detached))
>>  		tun_enable_queue(tfile);
>>  	else
>>  		sock_hold(&tfile->sk);
>> @@ -1195,7 +1201,7 @@ static ssize_t tun_get_user(struct tun_struct *tun, struct tun_file *tfile,
>>  	tun->dev->stats.rx_packets++;
>>  	tun->dev->stats.rx_bytes += len;
>>  
>> -	tun_flow_update(tun, rxhash, tfile->queue_index);
>> +	tun_flow_update(tun, rxhash, tfile);
>>  	return total_len;
>>  }
>>  
>> @@ -1552,7 +1558,7 @@ static int tun_set_iff(struct net *net, struct file *file, struct ifreq *ifr)
>>  	struct net_device *dev;
>>  	int err;
>>  
>> -	if (tfile->detached)
>> +	if (rtnl_dereference(tfile->detached))
>>  		return -EINVAL;
>>  
>>  	dev = __dev_get_by_name(net, ifr->ifr_name);
>> @@ -1796,7 +1802,7 @@ static int tun_set_queue(struct file *file, struct ifreq *ifr)
>>  	rtnl_lock();
>>  
>>  	if (ifr->ifr_flags & IFF_ATTACH_QUEUE) {
>> -		tun = tfile->detached;
>> +		tun = rtnl_dereference(tfile->detached);
>>  		if (!tun) {
>>  			ret = -EINVAL;
>>  			goto unlock;
>> @@ -1807,7 +1813,8 @@ static int tun_set_queue(struct file *file, struct ifreq *ifr)
>>  		ret = tun_attach(tun, file);
>>  	} else if (ifr->ifr_flags & IFF_DETACH_QUEUE) {
>>  		tun = rtnl_dereference(tfile->tun);
>> -		if (!tun || !(tun->flags & TUN_TAP_MQ))
>> +		if (!tun || !(tun->flags & TUN_TAP_MQ) ||
>> +		    rtnl_dereference(tfile->detached))
>>  			ret = -EINVAL;
>>  		else
>>  			__tun_detach(tfile, false);
>> -- 
>> 1.7.1
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

^ permalink raw reply

* Re: [Patch net-next v4] netpoll: fix a rtnl lock assertion failure
From: Eric Dumazet @ 2013-01-17  1:24 UTC (permalink / raw)
  To: Cong Wang; +Cc: netdev, Jiri Pirko, David S. Miller
In-Reply-To: <1358242446-4273-1-git-send-email-amwang@redhat.com>

On Tue, 2013-01-15 at 17:34 +0800, Cong Wang wrote:
> From: Cong Wang <amwang@redhat.com>
> 
> v4: hold rtnl lock for the whole netpoll_setup()
> v3: remove the comment
> v2: use RCU read lock
> 
> This patch fixes the following warning:
> 
> [   72.013864] RTNL: assertion failed at net/core/dev.c (4955)
> [   72.017758] Pid: 668, comm: netpoll-prep-v6 Not tainted 3.8.0-rc1+ #474
> [   72.019582] Call Trace:
> [   72.020295]  [<ffffffff8176653d>] netdev_master_upper_dev_get+0x35/0x58
> [   72.022545]  [<ffffffff81784edd>] netpoll_setup+0x61/0x340
> [   72.024846]  [<ffffffff815d837e>] store_enabled+0x82/0xc3
> [   72.027466]  [<ffffffff815d7e51>] netconsole_target_attr_store+0x35/0x37
> [   72.029348]  [<ffffffff811c3479>] configfs_write_file+0xe2/0x10c
> [   72.030959]  [<ffffffff8115d239>] vfs_write+0xaf/0xf6
> [   72.032359]  [<ffffffff81978a05>] ? sysret_check+0x22/0x5d
> [   72.033824]  [<ffffffff8115d453>] sys_write+0x5c/0x84
> [   72.035328]  [<ffffffff819789d9>] system_call_fastpath+0x16/0x1b
> 
> In case of other races, hold rtnl lock for the entire netpoll_setup() function.
> 
> Cc: Eric Dumazet <eric.dumazet@gmail.com>
> Cc: Jiri Pirko <jiri@resnulli.us>
> Cc: David S. Miller <davem@davemloft.net>
> Signed-off-by: Cong Wang <amwang@redhat.com>
> ---
> diff --git a/net/core/netpoll.c b/net/core/netpoll.c

...

>  	if (np->dev_name)
> -		ndev = dev_get_by_name(&init_net, np->dev_name);
> +		ndev = __dev_get_by_name(&init_net, np->dev_name);

This change brings interesting bugs.

All the "goto put;" are basically wrong, and the section waiting for the
carrier and releasing/getting rtnl is buggy.

Please revert this part.

^ permalink raw reply

* Re: Redefinition of struct in6_addr in <netinet/in.h> and <linux/in6.h>
From: Carlos O'Donell @ 2013-01-17  1:58 UTC (permalink / raw)
  To: David Miller
  Cc: bhutchings, yoshfuji, amwang, tmb, eblake, netdev, linux-kernel,
	libvirt-list, tgraf, libc-alpha, schwab
In-Reply-To: <20130116.164511.2027039182184636075.davem@davemloft.net>

On 01/16/2013 04:45 PM, David Miller wrote:
> From: Ben Hutchings <bhutchings@solarflare.com>
> Date: Wed, 16 Jan 2013 15:47:12 +0000
> 
>> On Wed, 2013-01-16 at 23:21 +0900, YOSHIFUJI Hideaki wrote:
>>> Cong Wang wrote:
>>>> (Cc'ing some glibc developers...)
>>>>
>>>> Hello,
>>>>
>>>> In glibc source file inet/netinet/in.h and kernel source file
>>>> include/uapi/linux/in6.h, both define struct in6_addr, and both are
>>>> visible to user applications. Thomas reported a conflict below.
>>>>
>>>> So, how can we handle this? /me is wondering why we didn't see this
>>>> before.
>> [...]
>>> This is not a new issue.  In addition to this,
>>> netinet/in.h also conflits with linux/in.h.
>>>
>>> We might have
>>>  #if !defined(__GLIBC__) || !defined(_NETINET_IN_H)
>>>  :
>>>  #endif
>>> around those conflicting definitions in uapi/linux/in{,6}.h.
>>
>> This only solves half the problem, as <netinet/in.h> might be included
>> after <linux/in.h>.  Also, not all Linux userland uses glibc.
> 
> So I've been looking at reasonable ways to fix this.
> 
> What would be really nice is if GCC treated multiple identical
> definitions of structures the same way it handles multiple identical
> definitions of CPP defines.  Which is to silently accept them.
> 
> But that's not the case, so we need a different solution.
> 
> Another thing to do is to use a new structure for in6_addr in kernel
> headers exported to userland.
> 
> If we were to make the structure members and accessor macros identical
> we could avoid breaking most if not all apps.
> 
> However the type name is different so:
> 
> 	struct in6_addr *p = &kern_struct->member;
> 
> wouldn't work so well.  And you couldn't fix up the sources to these
> kinds of accesses in a way that work cleanly both before and after
> changing the kernel headers.
> 
> I'm out of ideas for today.

So I just went down the rabbit hole, and the further I get the
closer I get to having two exact copies of the same definitions
in both glibc and the kernel and using whichever one was included
first.

Is anyone opposed to that kind of solution?

There is some ugliness there.

Cheers,
Carlos.
 

^ permalink raw reply

* Re: Redefinition of struct in6_addr in <netinet/in.h> and <linux/in6.h>
From: David Miller @ 2013-01-17  2:05 UTC (permalink / raw)
  To: carlos
  Cc: bhutchings, yoshfuji, amwang, tmb, eblake, netdev, linux-kernel,
	libvirt-list, tgraf, libc-alpha, schwab
In-Reply-To: <50F75AD7.7000703@systemhalted.org>

From: Carlos O'Donell <carlos@systemhalted.org>
Date: Wed, 16 Jan 2013 20:58:47 -0500

> So I just went down the rabbit hole, and the further I get the
> closer I get to having two exact copies of the same definitions
> in both glibc and the kernel and using whichever one was included
> first.
> 
> Is anyone opposed to that kind of solution?

Sounds interesting, please share :-)

^ permalink raw reply

* Re: Redefinition of struct in6_addr in <netinet/in.h> and <linux/in6.h>
From: Carlos O'Donell @ 2013-01-17  2:15 UTC (permalink / raw)
  To: David Miller
  Cc: vapier, libc-alpha, bhutchings, yoshfuji, amwang, tmb, eblake,
	netdev, linux-kernel, libvirt-list, tgraf, schwab
In-Reply-To: <20130116.135744.697469565804508454.davem@davemloft.net>

On 01/16/2013 01:57 PM, David Miller wrote:
> From: Mike Frysinger <vapier@gentoo.org>
> Date: Wed, 16 Jan 2013 12:04:56 -0500
> 
>> certainly true, but the current expectation is that you don't mix your ABIs.  
>> if you're programming with the C library API, then use the C library headers.  
>> if you're banging directly on the kernel, then use the kernel headers.  not 
>> saying it's a perfect solution, but it works for the vast majority of use 
>> cases.
> 
> This isn't how real life works.
> 
> GLIBC itself brings in some of the kernel headers, as do various library
> headers for libraries other than glibc.
> 
> So you can get these conflicting headers included indirectly, and it is
> of no fault of any of the various parties involved.
> 
> We have to make them work when included at the same time somehow, and
> this is totally unavoidable.
> 

Just to put some code behind the comments I made earlier.

Solution:
=========

- Synchronize linux's `include/uapi/linux/in6.h' 
  with glibc's `inet/netinet/in.h'.
- Synchronize glibc's `inet/netinet/in.h with linux's
  `include/uapi/linux/in6.h'.
- Allow including the headers in either other.
- First header included defines the structures and macros.

Details:
========

The kernel promises not to break the UAPI ABI so I don't 
see why we can't just have the two userspace headers
coordinate?

If you include the kernel headers first you get those,
and if you include the glibc headers first you get those,
and the following patch arranges a coordination and
synchronization between the two.

Let's handle `include/uapi/linux/in6.h' from linux, 
and `inet/netinet/in.h' from glibc and ensure they compile
in any order and preserve the required ABI.

These two patches pass the following compile tests:

cat >> test1.c <<EOF
#include <netinet/in.h>
#include <linux/in6.h>
int main (void) {
  return 0;
}
EOF
gcc -c test1.c

cat >> test2.c <<EOF
#include <linux/in6.h>
#include <netinet/in.h>
int main (void) {
  return 0;
}
EOF
gcc -c test2.c

One wrinkle is that the kernel has a different name for one of
the members in ipv6_mreq. In the kernel patch we create a macro
to cover the uses of the old name, and while that's not entirely
clean it's one of the best solutions (aside from an anonymous
union which has other issues).

I've reviewed the code and it looks to me like the ABI is
assured and everything matches on both sides.

Comments?

Notes:
- You want netinet/in.h to include bits/in.h as early as possible,
  but it needs in_addr so define in_addr early.
- You want bits/in.h included as early as possible so you can use
  the linux specific code to define __USE_KERNEL_DEFS based on 
  the _UAPI_* macro definition and use those to cull in.h.
- glibc was missing IPPROTO_MH, added here.

glibc/

2012-01-16  Carlos O'Donell  <carlos@redhat.com>

	* sysdeps/unix/sysv/linux/bits/in.h
	[_UAPI_LINUX_IN6_H]: Define __USE_KERNEL_IPV6_DEFS.
	* inet/netinet/in.h: Move in_addr definition and bits/in.h inclusion
	before __USE_KERNEL_IPV6_DEFS uses.
	* inet/netinet/in.h [!__USE_KERNEL_IPV6_DEFS]: Define IPPROTO_MH, and
	IPPROTO_BEETPH.
	[__USE_KERNEL_IPV6_DEFS]: Don't define any of IPPROTO_*, in6_addr,
	sockaddr_in6, or ipv6_mreq.

diff --git a/inet/netinet/in.h b/inet/netinet/in.h
index 89e3813..882739d 100644
--- a/inet/netinet/in.h
+++ b/inet/netinet/in.h
@@ -26,6 +26,20 @@
 
 __BEGIN_DECLS
 
+/* Internet address.  */
+typedef uint32_t in_addr_t;
+struct in_addr
+  {
+    in_addr_t s_addr;
+  };
+
+/* Get system-specific definitions.  */
+#include <bits/in.h>
+
+/* If __USER_KERNEL_IPV6_DEFS is defined then the user has included the kernel
+   network headers first and we should use those ABI-identical definitions
+   instead of our own.  */
+#ifndef __USE_KERNEL_IPV6_DEFS
 /* Standard well-defined IP protocols.  */
 enum
   {
@@ -75,6 +89,8 @@ enum
 #define IPPROTO_DSTOPTS		IPPROTO_DSTOPTS
     IPPROTO_MTP = 92,	   /* Multicast Transport Protocol.  */
 #define IPPROTO_MTP		IPPROTO_MTP
+    IPPROTO_BEETPH = 94,   /* IP option pseudo header for BEET.  */
+#define IPPROTO_BEETPH		IPPROTO_BEETPH
     IPPROTO_ENCAP = 98,	   /* Encapsulation Header.  */
 #define IPPROTO_ENCAP		IPPROTO_ENCAP
     IPPROTO_PIM = 103,	   /* Protocol Independent Multicast.  */
@@ -83,13 +99,15 @@ enum
 #define IPPROTO_COMP		IPPROTO_COMP
     IPPROTO_SCTP = 132,	   /* Stream Control Transmission Protocol.  */
 #define IPPROTO_SCTP		IPPROTO_SCTP
+    IPPROTO_MH = 135,      /* IPv6 mobility header.  */
+#define IPPROTO_MH		IPPROTO_MH
     IPPROTO_UDPLITE = 136, /* UDP-Lite protocol.  */
 #define IPPROTO_UDPLITE		IPPROTO_UDPLITE
     IPPROTO_RAW = 255,	   /* Raw IP packets.  */
 #define IPPROTO_RAW		IPPROTO_RAW
     IPPROTO_MAX
   };
-
+#endif /* !__USE_KERNEL_IPV6_DEFS */
 
 /* Type to represent a port.  */
 typedef uint16_t in_port_t;
@@ -134,15 +152,6 @@ enum
     IPPORT_USERRESERVED = 5000
   };
 
-
-/* Internet address.  */
-typedef uint32_t in_addr_t;
-struct in_addr
-  {
-    in_addr_t s_addr;
-  };
-
-
 /* Definitions of the bits in an Internet address integer.
 
    On subnets, host and network parts are found according to
@@ -191,7 +200,7 @@ struct in_addr
 #define INADDR_ALLRTRS_GROUP    ((in_addr_t) 0xe0000002) /* 224.0.0.2 */
 #define INADDR_MAX_LOCAL_GROUP  ((in_addr_t) 0xe00000ff) /* 224.0.0.255 */
 
-
+#ifndef __USE_KERNEL_IPV6_DEFS
 /* IPv6 address */
 struct in6_addr
   {
@@ -209,6 +218,7 @@ struct in6_addr
 # define s6_addr32		__in6_u.__u6_addr32
 #endif
   };
+#endif /* !__USE_KERNEL_IPV6_DEFS */
 
 extern const struct in6_addr in6addr_any;        /* :: */
 extern const struct in6_addr in6addr_loopback;   /* ::1 */
@@ -218,7 +228,6 @@ extern const struct in6_addr in6addr_loopback;   /* ::1 */
 #define INET_ADDRSTRLEN 16
 #define INET6_ADDRSTRLEN 46
 
-
 /* Structure describing an Internet socket address.  */
 struct sockaddr_in
   {
@@ -233,6 +242,7 @@ struct sockaddr_in
 			   sizeof (struct in_addr)];
   };
 
+#ifndef __USE_KERNEL_IPV6_DEFS
 /* Ditto, for IPv6.  */
 struct sockaddr_in6
   {
@@ -242,7 +252,7 @@ struct sockaddr_in6
     struct in6_addr sin6_addr;	/* IPv6 address */
     uint32_t sin6_scope_id;	/* IPv6 scope-id */
   };
-
+#endif /* !__USE_KERNEL_IPV6_DEFS */
 
 #if defined __USE_MISC || defined __USE_GNU
 /* IPv4 multicast request.  */
@@ -268,7 +278,7 @@ struct ip_mreq_source
   };
 #endif
 
-
+#ifndef __USE_KERNEL_IPV6_DEFS
 /* Likewise, for IPv6.  */
 struct ipv6_mreq
   {
@@ -278,7 +288,7 @@ struct ipv6_mreq
     /* local interface */
     unsigned int ipv6mr_interface;
   };
-
+#endif /* !__USE_KERNEL_IPV6_DEFS */
 
 #if defined __USE_MISC || defined __USE_GNU
 /* Multicast group request.  */
@@ -349,10 +359,6 @@ struct group_filter
 				      * sizeof (struct sockaddr_storage)))
 #endif
 
-
-/* Get system-specific definitions.  */
-#include <bits/in.h>
-
 /* Functions to convert between host and network byte order.
 
    Please note that these functions normally take `unsigned long int' or
diff --git a/sysdeps/unix/sysv/linux/bits/in.h b/sysdeps/unix/sysv/linux/bits/in.h
index e959b33..36bb72b 100644
--- a/sysdeps/unix/sysv/linux/bits/in.h
+++ b/sysdeps/unix/sysv/linux/bits/in.h
@@ -21,6 +21,15 @@
 # error "Never use <bits/in.h> directly; include <netinet/in.h> instead."
 #endif
 
+/* If the application has already included linux/in6.h from a linux-based
+   kernel then we will not define IPPROTO_* defines, in6_addr (nor the 
+   defines), sockaddr_in6, or ip_mreq.  The ABI used by the linux-kernel
+   and glibc match exactly.  Neither the linux kernel nor glibc should 
+   break this ABI without coordination.  */
+#ifdef _UAPI_LINUX_IN6_H
+# define __USE_KERNEL_IPV6_DEFS 
+#endif
+
 /* Options for use with `getsockopt' and `setsockopt' at the IP level.
    The first word in the comment at the right is the data type used;
    "bool" means a boolean value stored in an `int'.  */
---

linux/

Compile tested and inspected.

Signed-off-by: Carlos O'Donell <carlos@redhat.com>

 include/uapi/linux/in.h      |   32 +-----------------------
 include/uapi/linux/in6.h     |   50 +++++++++++++++++++++++++++++++------
 include/uapi/linux/ipproto.h |    0 
 include/uapi/linux/ipproto.h |   57 +++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 101 insertions(+), 38 deletions(-)

diff --git a/include/uapi/linux/in.h b/include/uapi/linux/in.h
index 9edb441..998ecb2 100644
--- a/include/uapi/linux/in.h
+++ b/include/uapi/linux/in.h
@@ -21,36 +21,8 @@
 #include <linux/types.h>
 #include <linux/socket.h>
 
-/* Standard well-defined IP protocols.  */
-enum {
-  IPPROTO_IP = 0,		/* Dummy protocol for TCP		*/
-  IPPROTO_ICMP = 1,		/* Internet Control Message Protocol	*/
-  IPPROTO_IGMP = 2,		/* Internet Group Management Protocol	*/
-  IPPROTO_IPIP = 4,		/* IPIP tunnels (older KA9Q tunnels use 94) */
-  IPPROTO_TCP = 6,		/* Transmission Control Protocol	*/
-  IPPROTO_EGP = 8,		/* Exterior Gateway Protocol		*/
-  IPPROTO_PUP = 12,		/* PUP protocol				*/
-  IPPROTO_UDP = 17,		/* User Datagram Protocol		*/
-  IPPROTO_IDP = 22,		/* XNS IDP protocol			*/
-  IPPROTO_DCCP = 33,		/* Datagram Congestion Control Protocol */
-  IPPROTO_RSVP = 46,		/* RSVP protocol			*/
-  IPPROTO_GRE = 47,		/* Cisco GRE tunnels (rfc 1701,1702)	*/
-
-  IPPROTO_IPV6	 = 41,		/* IPv6-in-IPv4 tunnelling		*/
-
-  IPPROTO_ESP = 50,            /* Encapsulation Security Payload protocol */
-  IPPROTO_AH = 51,             /* Authentication Header protocol       */
-  IPPROTO_BEETPH = 94,	       /* IP option pseudo header for BEET */
-  IPPROTO_PIM    = 103,		/* Protocol Independent Multicast	*/
-
-  IPPROTO_COMP   = 108,                /* Compression Header protocol */
-  IPPROTO_SCTP   = 132,		/* Stream Control Transport Protocol	*/
-  IPPROTO_UDPLITE = 136,	/* UDP-Lite (RFC 3828)			*/
-
-  IPPROTO_RAW	 = 255,		/* Raw IP packets			*/
-  IPPROTO_MAX
-};
-
+/* Include IPPROTO_* defines. */
+#include <linux/ipproto.h>
 
 /* Internet address. */
 struct in_addr {
diff --git a/include/uapi/linux/in6.h b/include/uapi/linux/in6.h
index f79c372..a2b16a5 100644
--- a/include/uapi/linux/in6.h
+++ b/include/uapi/linux/in6.h
@@ -23,6 +23,13 @@
 
 #include <linux/types.h>
 
+/* If a glibc-based userspace has already included in.h, then we will not 
+ * define in6_addr (nor the defines), sockaddr_in6, or ipv6_mreq. The
+ * ABI used by the kernel and by glibc match exactly. Neither the kernel
+ * nor glibc should break this ABI without coordination.
+ */
+#ifndef _NETINET_IN_H
+
 /*
  *	IPv6 address structure
  */
@@ -30,12 +37,20 @@
 struct in6_addr {
 	union {
 		__u8		u6_addr8[16];
+#if !defined(__GLIBC__) \
+    || (defined(__GLIBC__) && (defined(__USE_MISC) || defined(__USE_GNU))) \
+    || defined(__KERNEL__)
 		__be16		u6_addr16[8];
 		__be32		u6_addr32[4];
+#endif
 	} in6_u;
+#if !defined(__GLIBC__) \
+    || (defined(__GLIBC__) && (defined(__USE_MISC) || defined(__USE_GNU))) \
+    || defined(__KERNEL__)
 #define s6_addr			in6_u.u6_addr8
 #define s6_addr16		in6_u.u6_addr16
 #define s6_addr32		in6_u.u6_addr32
+#endif
 };
 
 /* IPv6 Wildcard Address (::) and Loopback Address (::1) defined in RFC2553
@@ -56,9 +71,12 @@ struct ipv6_mreq {
 	struct in6_addr ipv6mr_multiaddr;
 
 	/* local IPv6 address of interface */
-	int		ipv6mr_ifindex;
+	int		ipv6mr_interface;
+#define ipv6mr_ifindex	ipv6mr_interface
 };
 
+#endif /* !_NET_INET_H */
+
 #define ipv6mr_acaddr	ipv6mr_multiaddr
 
 struct in6_flowlabel_req {
@@ -116,16 +134,32 @@ struct in6_flowlabel_req {
 #define IPV6_PRIORITY_14		0x0e00
 #define IPV6_PRIORITY_15		0x0f00
 
+
+#ifndef _NETINET_IN_H
+#if defined (__GLIBC__)
+/* Include all of the other IPPROTO_* defines for userspace. */
+#include <linux/ipproto.h>
+#endif
 /*
  *	IPV6 extension headers
  */
-#define IPPROTO_HOPOPTS		0	/* IPv6 hop-by-hop options	*/
-#define IPPROTO_ROUTING		43	/* IPv6 routing header		*/
-#define IPPROTO_FRAGMENT	44	/* IPv6 fragmentation header	*/
-#define IPPROTO_ICMPV6		58	/* ICMPv6			*/
-#define IPPROTO_NONE		59	/* IPv6 no next header		*/
-#define IPPROTO_DSTOPTS		60	/* IPv6 destination options	*/
-#define IPPROTO_MH		135	/* IPv6 mobility header		*/
+enum {
+  IPPROTO_HOPOPTS = 0,		/* IPv6 hop-by-hop options      */
+#define IPPROTO_HOPOPTS		IPPROTO_HOPOPTS
+  IPPROTO_ROUTING = 43,		/* IPv6 routing header          */
+#define IPPROTO_ROUTING		IPPROTO_ROUTING
+  IPPROTO_FRAGMENT = 44,	/* IPv6 fragmentation header    */
+#define IPPROTO_FRAGMENT	IPPROTO_FRAGMENT
+  IPPROTO_ICMPV6 = 58,		/* ICMPv6                       */
+#define IPPROTO_ICMPV6		IPPROTO_ICMPV6
+  IPPROTO_NONE = 59,		/* IPv6 no next header          */
+#define IPPROTO_NONE		IPPROTO_NONE
+  IPPROTO_DSTOPTS = 60,		/* IPv6 destination options     */
+#define IPPROTO_DSTOPTS		IPPROTO_DSTOPTS
+  IPPROTO_MH = 135,		/* IPv6 mobility header         */
+#define IPPROTO_MH		IPPROTO_MH
+};
+#endif /* !_NETINET_IN_H */
 
 /*
  *	IPv6 TLV options.
diff --git a/include/uapi/linux/ipproto.h b/include/uapi/linux/ipproto.h
new file mode 100644
index 0000000..5a183e6
--- /dev/null
+++ b/include/uapi/linux/ipproto.h
@@ -0,0 +1,57 @@
+#ifndef _UAPI_LINUX_IPPROTO_H
+#define _UAPI_LINUX_IPPROTO_H
+
+/* Standard well-defined IP protocols. */
+enum {
+  IPPROTO_IP = 0,		/* Dummy protocol for TCP. */
+#define IPPROTO_IP		IPPROTO_IP
+  IPPROTO_ICMP = 1,		/* Internet Control Message Protocol. */
+#define IPPROTO_ICMP		IPPROTO_ICMP
+  IPPROTO_IGMP = 2,		/* Internet Group Management Protocol. */
+#define IPPROTO_IGMP		IPPROTO_IGMP
+  IPPROTO_IPIP = 4,		/* IPIP tunnels (older KA9Q tunnels use 94). */
+#define IPPROTO_IPIP		IPPROTO_IPIP
+  IPPROTO_TCP = 6,		/* Transmission Control Protocol. */
+#define IPPROTO_TCP		IPPROTO_TCP
+  IPPROTO_EGP = 8,		/* Exterior Gateway Protocol. */
+#define IPPROTO_EGP		IPPROTO_EGP
+  IPPROTO_PUP = 12,		/* PUP protocol. */
+#define IPPROTO_PUP		IPPROTO_PUP
+  IPPROTO_UDP = 17,		/* User Datagram Protocol. */
+#define IPPROTO_UDP		IPPROTO_UDP
+  IPPROTO_IDP = 22,		/* XNS IDP protocol. */
+#define IPPROTO_IDP		IPPROTO_IDP
+  IPPROTO_TP = 29,		/* SO Transport Protocol Class 4. */
+#define IPPROTO_TP		IPPROTO_TP
+  IPPROTO_DCCP = 33,		/* Datagram Congestion Control Protocol. */
+#define IPPROTO_DCCP		IPPROTO_DCCP
+  IPPROTO_IPV6 = 41,		/* IPv6 header. */
+#define IPPROTO_IPV6		IPPROTO_IPV6
+  IPPROTO_RSVP = 46,		/* Reservation Protocol. */
+#define IPPROTO_RSVP		IPPROTO_RSVP
+  IPPROTO_GRE = 47,		/* General Routing Encapsulation. */
+#define IPPROTO_GRE		IPPROTO_GRE
+  IPPROTO_ESP = 50,		/* encapsulating security payload. */
+#define IPPROTO_ESP		IPPROTO_ESP
+  IPPROTO_AH = 51,		/* authentication header. */
+#define IPPROTO_AH		IPPROTO_AH
+  IPPROTO_MTP = 92,		/* Multicast Transport Protocol. */
+#define IPPROTO_MTP		IPPROTO_MTP
+  IPPROTO_BEETPH = 94,		/* IP option pseudo header for BEET. */
+#define IPPROTO_BEETPH		IPPROTO_BEETPH
+  IPPROTO_ENCAP = 98,		/* Encapsulation Header. */
+#define IPPROTO_ENCAP		IPPROTO_ENCAP
+  IPPROTO_PIM = 103,		/* Protocol Independent Multicast. */
+#define IPPROTO_PIM		IPPROTO_PIM
+  IPPROTO_COMP = 108,		/* Compression Header Protocol. */
+#define IPPROTO_COMP		IPPROTO_COMP
+  IPPROTO_SCTP = 132,		/* Stream Control Transmission Protocol. */
+#define IPPROTO_SCTP		IPPROTO_SCTP
+  IPPROTO_UDPLITE = 136,	/* UDP-Lite protocol. */
+#define IPPROTO_UDPLITE		IPPROTO_UDPLITE
+  IPPROTO_RAW = 255,		/* Raw IP packets. */
+#define IPPROTO_RAW		IPPROTO_RAW
+  IPPROTO_MAX
+};
+
+#endif /* _UAPI_LINUX_IPPROTO_H */
---

Cheers,
Carlos.

^ permalink raw reply related

* Re: [PATCH v2 1/1 net-next] net: fec: enable pause frame to improve rx prefomance for 1G network
From: Frank Li @ 2013-01-17  2:50 UTC (permalink / raw)
  To: Frank Li
  Cc: B38611, netdev, s.hauer, bhutchings, shawn.guo, davem,
	linux-arm-kernel
In-Reply-To: <1358153387-19275-1-git-send-email-Frank.Li@freescale.com>

Any feedback about this patch?

best regards
Frank Li

2013/1/14 Frank Li <Frank.Li@freescale.com>:
> The limition of imx6 internal bus cause fec can't achieve 1G perfomance.
> There will be many packages lost because FIFO over run.
>
> This patch enable pause frame flow control.
>
> Before this patch
> iperf -s -i 1
> TCP window size: 85.3 KByte (default)
> ------------------------------------------------------------
> [  4] local 10.192.242.153 port 5001 connected with 10.192.242.94 port 49773
> [ ID] Interval       Transfer     Bandwidth
> [  4]  0.0- 1.0 sec  6.35 MBytes  53.3 Mbits/sec
> [  4]  1.0- 2.0 sec  3.39 MBytes  28.5 Mbits/sec
> [  4]  2.0- 3.0 sec  2.63 MBytes  22.1 Mbits/sec
> [  4]  3.0- 4.0 sec  1.10 MBytes  9.23 Mbits/sec
>
> ifconfig
>    RX packets:46195 errors:1859 dropped:1 overruns:1859 frame:1859
>
> After this patch
> iperf -s -i 1
>
> [  4] local 10.192.242.153 port 5001 connected with 10.192.242.94 port 49757
> [ ID] Interval       Transfer     Bandwidth
> [  4]  0.0- 1.0 sec  49.8 MBytes   418 Mbits/sec
> [  4]  1.0- 2.0 sec  50.1 MBytes   420 Mbits/sec
> [  4]  2.0- 3.0 sec  47.5 MBytes   399 Mbits/sec
> [  4]  3.0- 4.0 sec  45.9 MBytes   385 Mbits/sec
> [  4]  4.0- 5.0 sec  44.8 MBytes   376 Mbits/sec
>
> ifconfig
>    RX packets:2348454 errors:0 dropped:16 overruns:0 frame:0
>
> Signed-off-by: Frank Li <Frank.Li@freescale.com>
> Signed-off-by: Fugang Duan <B38611@freescale.com>
> ---
>
> Change from V1 to V2
>  * using pause_setparam to enable/disable pause frame
>  * default enable pause frame auto negotiation
>
>  drivers/net/ethernet/freescale/fec.c |   89 +++++++++++++++++++++++++++++++++-
>  drivers/net/ethernet/freescale/fec.h |    5 ++
>  2 files changed, 93 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/net/ethernet/freescale/fec.c b/drivers/net/ethernet/freescale/fec.c
> index 6dc2094..ea30c26 100644
> --- a/drivers/net/ethernet/freescale/fec.c
> +++ b/drivers/net/ethernet/freescale/fec.c
> @@ -68,6 +68,14 @@
>
>  #define DRIVER_NAME    "fec"
>
> +/* Pause frame feild and FIFO threshold */
> +#define FEC_ENET_FCE   (1 << 5)
> +#define FEC_ENET_RSEM_V        0x84
> +#define FEC_ENET_RSFL_V        16
> +#define FEC_ENET_RAEM_V        0x8
> +#define FEC_ENET_RAFL_V        0x8
> +#define FEC_ENET_OPD_V 0xFFF0
> +
>  /* Controller is ENET-MAC */
>  #define FEC_QUIRK_ENET_MAC             (1 << 0)
>  /* Controller needs driver to swap frame */
> @@ -193,6 +201,9 @@ MODULE_PARM_DESC(macaddr, "FEC Ethernet MAC address");
>  /* Transmitter timeout */
>  #define TX_TIMEOUT (2 * HZ)
>
> +#define FEC_PAUSE_FLAG_AUTONEG 0x1
> +#define FEC_PAUSE_FLAG_ENABLE  0x2
> +
>  static int mii_cnt;
>
>  static struct bufdesc *fec_enet_get_nextdesc(struct bufdesc *bdp, int is_ex)
> @@ -470,6 +481,26 @@ fec_restart(struct net_device *ndev, int duplex)
>                 }
>  #endif
>         }
> +
> +       /* enable pause frame*/
> +       if ((fep->pause_flag & FEC_PAUSE_FLAG_ENABLE) ||
> +               ((fep->pause_flag & FEC_PAUSE_FLAG_AUTONEG) &&
> +                       fep->phy_dev && fep->phy_dev->pause)) {
> +
> +               rcntl |= FEC_ENET_FCE;
> +
> +               /* set FIFO thresh hold parameter to reduce overrun */
> +               writel(FEC_ENET_RSEM_V, fep->hwp + FEC_R_FIFO_RSEM);
> +               writel(FEC_ENET_RSFL_V, fep->hwp + FEC_R_FIFO_RSFL);
> +               writel(FEC_ENET_RAEM_V, fep->hwp + FEC_R_FIFO_RAEM);
> +               writel(FEC_ENET_RAFL_V, fep->hwp + FEC_R_FIFO_RAFL);
> +
> +               /* OPD */
> +               writel(FEC_ENET_OPD_V, fep->hwp + FEC_OPD);
> +       } else {
> +               rcntl &= ~FEC_ENET_FCE;
> +       }
> +
>         writel(rcntl, fep->hwp + FEC_R_CNTRL);
>
>         if (id_entry->driver_data & FEC_QUIRK_ENET_MAC) {
> @@ -1016,8 +1047,10 @@ static int fec_enet_mii_probe(struct net_device *ndev)
>         }
>
>         /* mask with MAC supported features */
> -       if (id_entry->driver_data & FEC_QUIRK_HAS_GBIT)
> +       if (id_entry->driver_data & FEC_QUIRK_HAS_GBIT) {
>                 phy_dev->supported &= PHY_GBIT_FEATURES;
> +               phy_dev->supported |= SUPPORTED_Pause;
> +       }
>         else
>                 phy_dev->supported &= PHY_BASIC_FEATURES;
>
> @@ -1203,7 +1236,56 @@ static int fec_enet_get_ts_info(struct net_device *ndev,
>         }
>  }
>
> +static void fec_enet_get_pauseparam(struct net_device *ndev,
> +                                   struct ethtool_pauseparam *pause)
> +{
> +       struct fec_enet_private *fep = netdev_priv(ndev);
> +
> +       pause->autoneg = (fep->pause_flag & FEC_PAUSE_FLAG_AUTONEG) != 0;
> +       pause->tx_pause = (fep->pause_flag & FEC_PAUSE_FLAG_ENABLE) != 0;
> +       pause->rx_pause = pause->tx_pause;
> +
> +}
> +
> +static int fec_enet_set_pauseparam(struct net_device *ndev,
> +                                  struct ethtool_pauseparam *pause)
> +{
> +       struct fec_enet_private *fep = netdev_priv(ndev);
> +
> +       if (pause->tx_pause != pause->rx_pause) {
> +               netdev_info(ndev,
> +                       "hardware only support enable/disable both tx and rx");
> +               return -EINVAL;
> +       }
> +
> +       fep->pause_flag = 0;
> +
> +       /* tx pause must be same as rx pause */
> +       fep->pause_flag |= pause->rx_pause ? FEC_PAUSE_FLAG_ENABLE : 0;
> +       fep->pause_flag |= pause->autoneg ? FEC_PAUSE_FLAG_AUTONEG : 0;
> +
> +       if (pause->rx_pause || pause->autoneg) {
> +               fep->phy_dev->supported |= ADVERTISED_Pause;
> +               fep->phy_dev->advertising |= ADVERTISED_Pause;
> +       } else {
> +               fep->phy_dev->supported &= ~ADVERTISED_Pause;
> +               fep->phy_dev->advertising &= ~ADVERTISED_Pause;
> +       }
> +
> +       if (pause->autoneg) {
> +               if (netif_running(ndev))
> +                       fec_stop(ndev);
> +               phy_start_aneg(fep->phy_dev);
> +       }
> +       if (netif_running(ndev))
> +               fec_restart(ndev, 0);
> +
> +       return 0;
> +}
> +
>  static const struct ethtool_ops fec_enet_ethtool_ops = {
> +       .get_pauseparam         = fec_enet_get_pauseparam,
> +       .set_pauseparam         = fec_enet_set_pauseparam,
>         .get_settings           = fec_enet_get_settings,
>         .set_settings           = fec_enet_set_settings,
>         .get_drvinfo            = fec_enet_get_drvinfo,
> @@ -1643,6 +1725,11 @@ fec_probe(struct platform_device *pdev)
>         /* setup board info structure */
>         fep = netdev_priv(ndev);
>
> +       /* default enable pause frame auto negotiation */
> +       if (pdev->id_entry &&
> +                       (pdev->id_entry->driver_data & FEC_QUIRK_HAS_GBIT))
> +               fep->pause_flag |= FEC_PAUSE_FLAG_AUTONEG;
> +
>         fep->hwp = ioremap(r->start, resource_size(r));
>         fep->pdev = pdev;
>         fep->dev_id = dev_id++;
> diff --git a/drivers/net/ethernet/freescale/fec.h b/drivers/net/ethernet/freescale/fec.h
> index 4862394..2ebedaf 100644
> --- a/drivers/net/ethernet/freescale/fec.h
> +++ b/drivers/net/ethernet/freescale/fec.h
> @@ -48,6 +48,10 @@
>  #define FEC_R_DES_START                0x180 /* Receive descriptor ring */
>  #define FEC_X_DES_START                0x184 /* Transmit descriptor ring */
>  #define FEC_R_BUFF_SIZE                0x188 /* Maximum receive buff size */
> +#define FEC_R_FIFO_RSFL                0x190 /* Receive FIFO section full threshold */
> +#define FEC_R_FIFO_RSEM                0x194 /* Receive FIFO section empty threshold */
> +#define FEC_R_FIFO_RAEM                0x198 /* Receive FIFO almost empty threshold */
> +#define FEC_R_FIFO_RAFL                0x19c /* Receive FIFO almost full threshold */
>  #define FEC_MIIGSK_CFGR                0x300 /* MIIGSK Configuration reg */
>  #define FEC_MIIGSK_ENR         0x308 /* MIIGSK Enable reg */
>
> @@ -243,6 +247,7 @@ struct fec_enet_private {
>         struct  completion mdio_done;
>         int     irq[FEC_IRQ_NUM];
>         int     bufdesc_ex;
> +       int     pause_flag;
>
>         struct ptp_clock *ptp_clock;
>         struct ptp_clock_info ptp_caps;
> --
> 1.7.1
>
>

^ permalink raw reply

* Re: [PATCH 1/1] net: mvneta: Remove unneeded version.h header inclusion
From: Sachin Kamat @ 2013-01-17  3:00 UTC (permalink / raw)
  To: Thomas Petazzoni; +Cc: netdev
In-Reply-To: <20121203195826.679f70d4@skate>

Hi Thomas,

On 4 December 2012 00:28, Thomas Petazzoni
<thomas.petazzoni@free-electrons.com> wrote:
> Dear David Miller,
>
> On Mon, 03 Dec 2012 13:46:20 -0500 (EST), David Miller wrote:
>> From: Sachin Kamat <sachin.kamat@linaro.org>
>> Date: Mon,  3 Dec 2012 15:15:28 +0530
>>
>> > linux/version.h inclusion is not necessary.
>> >
>> > Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org>
>>
>> Acked-by: David S. Miller <davem@davemloft.net>
>>
>> I hope the mvneta folks will pick this up and apply it where it
>> needs to be as the mvneta driver isn't in the net-next tree.
>
> Yes, I will pick it up, and send a pull request to the Marvell
> maintainers. I have other mvneta fixes that I'll send together with
> this one.

Can you please share the git repo link where this patch is lined up ATM?


>
> Thanks,
>
> Thomas
> --
> Thomas Petazzoni, Free Electrons
> Kernel, drivers, real-time and embedded Linux
> development, consulting, training and support.
> http://free-electrons.com



-- 
With warm regards,
Sachin

^ permalink raw reply

* Re: Redefinition of struct in6_addr in <netinet/in.h> and <linux/in6.h>
From: YOSHIFUJI Hideaki @ 2013-01-17  3:10 UTC (permalink / raw)
  To: Carlos O'Donell
  Cc: David Miller, vapier, libc-alpha, bhutchings, amwang, tmb, eblake,
	netdev, linux-kernel, libvirt-list, tgraf, schwab,
	YOSHIFUJI Hideaki
In-Reply-To: <50F75EA7.4060309@systemhalted.org>

Carlos O'Donell wrote:
> diff --git a/include/uapi/linux/in6.h b/include/uapi/linux/in6.h
> index f79c372..a2b16a5 100644
> --- a/include/uapi/linux/in6.h
> +++ b/include/uapi/linux/in6.h
:
>  #define IPV6_PRIORITY_14		0x0e00
>  #define IPV6_PRIORITY_15		0x0f00
>  
> +
> +#ifndef _NETINET_IN_H
> +#if defined (__GLIBC__)
> +/* Include all of the other IPPROTO_* defines for userspace. */
> +#include <linux/ipproto.h>
> +#endif
>  /*
>   *	IPV6 extension headers

Overall, it looks good, but why including linux/ipproto.h?

--yoshfuji

^ permalink raw reply

* Re: [PATCH v2 1/1 net-next] net: fec: enable pause frame to improve rx prefomance for 1G network
From: David Miller @ 2013-01-17  3:10 UTC (permalink / raw)
  To: lznuaa
  Cc: Frank.Li, shawn.guo, B38611, linux-arm-kernel, netdev, bhutchings,
	s.hauer
In-Reply-To: <CAHrpEqRV7PonDpqAvGkDf7QqSeUj+mVbnF2rA-Zjb9xfWh-_2g@mail.gmail.com>

From: Frank Li <lznuaa@gmail.com>
Date: Thu, 17 Jan 2013 10:50:36 +0800

> Any feedback about this patch?

Well it's full of stylistic problems.

>> +       /* enable pause frame*/
>> +       if ((fep->pause_flag & FEC_PAUSE_FLAG_ENABLE) ||
>> +               ((fep->pause_flag & FEC_PAUSE_FLAG_AUTONEG) &&
>> +                       fep->phy_dev && fep->phy_dev->pause)) {

This is mis-idented.

>> +
>> +               rcntl |= FEC_ENET_FCE;

That empty line before this assignment is spurious.

>> +       pause->rx_pause = pause->tx_pause;
>> +
>> +}

That empty line is unnecessary, remove it.

>> +       /* default enable pause frame auto negotiation */
>> +       if (pdev->id_entry &&
>> +                       (pdev->id_entry->driver_data & FEC_QUIRK_HAS_GBIT))

You can't possibly tell me that this indentation looks right
to you.

You must style things like this:

	if (condition1 &&
	    condition2)

That is, you must line up the first character on the second and
subsequent lines at the first column after the openning parenthesis of
the first line.

^ permalink raw reply

* [PATCH] ipv6: Fix endianess warning in ip6_flow_hdr().
From: YOSHIFUJI Hideaki @ 2013-01-17  3:10 UTC (permalink / raw)
  To: David Miller, netdev; +Cc: fengguang.wu, YOSHIFUJI Hideaki

Commit 3e4e4c1f ("ipv6: Introduce ip6_flow_hdr() to fill version,
tclass and flowlabel.) uses ntohl(), which should be htonl().

Found by Fengguang Wu <fengguang.wu@intel.com>.

Signed-off-by: YOSHIFUJI Hideaki <yoshfuji@linux-ipv6.org>
---
 include/net/ipv6.h |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/net/ipv6.h b/include/net/ipv6.h
index 459a5a4..be3358b 100644
--- a/include/net/ipv6.h
+++ b/include/net/ipv6.h
@@ -629,7 +629,7 @@ extern void ipv6_select_ident(struct frag_hdr *fhdr, struct rt6_info *rt);
 static inline void ip6_flow_hdr(struct ipv6hdr *hdr, unsigned int tclass,
 				__be32 flowlabel)
 {
-	*(__be32 *)hdr = ntohl(0x60000000 | (tclass << 20)) | flowlabel;
+	*(__be32 *)hdr = htonl(0x60000000 | (tclass << 20)) | flowlabel;
 }
 
 static inline __be32 ip6_flowinfo(const struct ipv6hdr *hdr)
-- 
1.7.9.5

^ permalink raw reply related

* Re: [PATCH] ipv6: Fix endianess warning in ip6_flow_hdr().
From: David Miller @ 2013-01-17  3:13 UTC (permalink / raw)
  To: yoshfuji; +Cc: netdev, fengguang.wu
In-Reply-To: <50F76BC1.90300@linux-ipv6.org>

From: YOSHIFUJI Hideaki <yoshfuji@linux-ipv6.org>
Date: Thu, 17 Jan 2013 12:10:57 +0900

> Commit 3e4e4c1f ("ipv6: Introduce ip6_flow_hdr() to fill version,
> tclass and flowlabel.) uses ntohl(), which should be htonl().
> 
> Found by Fengguang Wu <fengguang.wu@intel.com>.
> 
> Signed-off-by: YOSHIFUJI Hideaki <yoshfuji@linux-ipv6.org>

Applied, thanks.

^ permalink raw reply

* Re: Redefinition of struct in6_addr in <netinet/in.h> and <linux/in6.h>
From: David Miller @ 2013-01-17  3:15 UTC (permalink / raw)
  To: carlos
  Cc: vapier, libc-alpha, bhutchings, yoshfuji, amwang, tmb, eblake,
	netdev, linux-kernel, libvirt-list, tgraf, schwab
In-Reply-To: <50F75EA7.4060309@systemhalted.org>

From: Carlos O'Donell <carlos@systemhalted.org>
Date: Wed, 16 Jan 2013 21:15:03 -0500

> +/* If a glibc-based userspace has already included in.h, then we will not 
> + * define in6_addr (nor the defines), sockaddr_in6, or ipv6_mreq. The
> + * ABI used by the kernel and by glibc match exactly. Neither the kernel
> + * nor glibc should break this ABI without coordination.
> + */
> +#ifndef _NETINET_IN_H
> +

I think we should shoot for a non-glibc-centric solution.

I can't imagine that other libc's won't have the same exact problem
with their netinet/in.h conflicting with the kernel's, redefining
structures like in6_addr, that we'd want to provide a protection
scheme for here as well.

Let's pick some more generic names and themes for the CPP macros and
comments we use to protect the header file blocks and describe that
protection scheme.

Thanks.

^ permalink raw reply

* Re: Redefinition of struct in6_addr in <netinet/in.h> and <linux/in6.h>
From: YOSHIFUJI Hideaki @ 2013-01-17  3:22 UTC (permalink / raw)
  To: Carlos O'Donell
  Cc: David Miller, vapier, libc-alpha, bhutchings, amwang, tmb, eblake,
	netdev, linux-kernel, libvirt-list, tgraf, schwab,
	YOSHIFUJI Hideaki
In-Reply-To: <50F75EA7.4060309@systemhalted.org>

Carlos O'Donell wrote:
> diff --git a/include/uapi/linux/in6.h b/include/uapi/linux/in6.h
> index f79c372..a2b16a5 100644
> --- a/include/uapi/linux/in6.h
> +++ b/include/uapi/linux/in6.h
> @@ -23,6 +23,13 @@
>  
>  #include <linux/types.h>
>  
> +/* If a glibc-based userspace has already included in.h, then we will not 
> + * define in6_addr (nor the defines), sockaddr_in6, or ipv6_mreq. The
> + * ABI used by the kernel and by glibc match exactly. Neither the kernel
> + * nor glibc should break this ABI without coordination.
> + */
> +#ifndef _NETINET_IN_H
> +
>  /*
>   *	IPv6 address structure
>   */

This should be
#if !defined(__GLIBC__) || !defined(_NETINET_IN_H)

> @@ -30,12 +37,20 @@
>  struct in6_addr {
>  	union {
>  		__u8		u6_addr8[16];
> +#if !defined(__GLIBC__) \
> +    || (defined(__GLIBC__) && (defined(__USE_MISC) || defined(__USE_GNU))) \
> +    || defined(__KERNEL__)
>  		__be16		u6_addr16[8];
>  		__be32		u6_addr32[4];
> +#endif
>  	} in6_u;
> +#if !defined(__GLIBC__) \
> +    || (defined(__GLIBC__) && (defined(__USE_MISC) || defined(__USE_GNU))) \
> +    || defined(__KERNEL__)
>  #define s6_addr			in6_u.u6_addr8
>  #define s6_addr16		in6_u.u6_addr16
>  #define s6_addr32		in6_u.u6_addr32
> +#endif
>  };

2nd "if" be after s6_addr?

--yoshfuji

^ permalink raw reply

* [PATCH] ipv6: add anti-spoofing checks for 6to4 and 6rd
From: Hannes Frederic Sowa @ 2013-01-17  3:32 UTC (permalink / raw)
  To: netdev

This patch adds anti-spoofing checks in sit.c as specified in RFC3964
section 5.2 for 6to4 and RFC5969 section 12 for 6rd. I left out the
checks which could easily be implemented with netfilter.

Signed-off-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
---
 net/ipv6/sit.c | 27 +++++++++++++++++++++++++--
 1 file changed, 25 insertions(+), 2 deletions(-)

diff --git a/net/ipv6/sit.c b/net/ipv6/sit.c
index cfba99b..2b4c15a 100644
--- a/net/ipv6/sit.c
+++ b/net/ipv6/sit.c
@@ -590,6 +590,22 @@ out:
 	return err;
 }
 
+static int sit_chksrc(struct ip_tunnel *tunnel, const __be32 *addr,
+		const struct in6_addr *addr6)
+{
+#ifdef CONFIG_IPV6_SIT_6RD
+	if (ipv6_prefix_equal(addr6, &tunnel->ip6rd.prefix,
+				tunnel->ip6rd.prefixlen) &&
+		memcmp(addr, &addr6->s6_addr16[1], 4))
+		return 0;
+#else
+	if (addr6->s6_addr16[0] == htons(0x2002) &&
+		memcmp(addr, &addr6->s6_addr16[1], 4))
+		return 0;
+#endif
+	return 1;
+}
+
 static int ipip6_rcv(struct sk_buff *skb)
 {
 	const struct iphdr *iph;
@@ -613,8 +629,15 @@ static int ipip6_rcv(struct sk_buff *skb)
 		skb->protocol = htons(ETH_P_IPV6);
 		skb->pkt_type = PACKET_HOST;
 
-		if ((tunnel->dev->priv_flags & IFF_ISATAP) &&
-		    !isatap_chksrc(skb, iph, tunnel)) {
+		if (tunnel->dev->priv_flags & IFF_ISATAP) {
+			if (!isatap_chksrc(skb, iph, tunnel)) {
+				tunnel->dev->stats.rx_errors++;
+				goto out;
+			}
+		} else if (!sit_chksrc(tunnel, &iph->saddr,
+					&ipv6_hdr(skb)->saddr) ||
+			!sit_chksrc(tunnel, &iph->daddr,
+				&ipv6_hdr(skb)->daddr)) {
 			tunnel->dev->stats.rx_errors++;
 			goto out;
 		}
-- 
1.7.11.7

^ permalink raw reply related

* [PATCH v3 1/1 net-next] net: fec: enable pause frame to improve rx prefomance for 1G network
From: Frank Li @ 2013-01-17  2:55 UTC (permalink / raw)
  To: lznuaa, shawn.guo, B38611, davem, linux-arm-kernel, netdev,
	bhutchings
  Cc: s.hauer, Frank Li

The limition of imx6 internal bus cause fec can't achieve 1G perfomance.
There will be many packages lost because FIFO over run.

This patch enable pause frame flow control.

Before this patch
iperf -s -i 1
TCP window size: 85.3 KByte (default)
------------------------------------------------------------
[  4] local 10.192.242.153 port 5001 connected with 10.192.242.94 port 49773
[ ID] Interval       Transfer     Bandwidth
[  4]  0.0- 1.0 sec  6.35 MBytes  53.3 Mbits/sec
[  4]  1.0- 2.0 sec  3.39 MBytes  28.5 Mbits/sec
[  4]  2.0- 3.0 sec  2.63 MBytes  22.1 Mbits/sec
[  4]  3.0- 4.0 sec  1.10 MBytes  9.23 Mbits/sec

ifconfig
   RX packets:46195 errors:1859 dropped:1 overruns:1859 frame:1859

After this patch
iperf -s -i 1

[  4] local 10.192.242.153 port 5001 connected with 10.192.242.94 port 49757
[ ID] Interval       Transfer     Bandwidth
[  4]  0.0- 1.0 sec  49.8 MBytes   418 Mbits/sec
[  4]  1.0- 2.0 sec  50.1 MBytes   420 Mbits/sec
[  4]  2.0- 3.0 sec  47.5 MBytes   399 Mbits/sec
[  4]  3.0- 4.0 sec  45.9 MBytes   385 Mbits/sec
[  4]  4.0- 5.0 sec  44.8 MBytes   376 Mbits/sec

ifconfig
   RX packets:2348454 errors:0 dropped:16 overruns:0 frame:0

Signed-off-by: Frank Li <Frank.Li@freescale.com>
Signed-off-by: Fugang Duan <B38611@freescale.com>
---
Change from v2 to v3
 * Fix code style problem reviewed by David

Change from V1 to V2
 * using pause_setparam to enable/disable pause frame
 * default enable pause frame auto negotiation

 drivers/net/ethernet/freescale/fec.c |   87 +++++++++++++++++++++++++++++++++-
 drivers/net/ethernet/freescale/fec.h |    5 ++
 2 files changed, 91 insertions(+), 1 deletions(-)

diff --git a/drivers/net/ethernet/freescale/fec.c b/drivers/net/ethernet/freescale/fec.c
index 6dc2094..8336157 100644
--- a/drivers/net/ethernet/freescale/fec.c
+++ b/drivers/net/ethernet/freescale/fec.c
@@ -68,6 +68,14 @@
 
 #define DRIVER_NAME	"fec"
 
+/* Pause frame feild and FIFO threshold */
+#define FEC_ENET_FCE	(1 << 5)
+#define FEC_ENET_RSEM_V	0x84
+#define FEC_ENET_RSFL_V	16
+#define FEC_ENET_RAEM_V	0x8
+#define FEC_ENET_RAFL_V	0x8
+#define FEC_ENET_OPD_V	0xFFF0
+
 /* Controller is ENET-MAC */
 #define FEC_QUIRK_ENET_MAC		(1 << 0)
 /* Controller needs driver to swap frame */
@@ -193,6 +201,9 @@ MODULE_PARM_DESC(macaddr, "FEC Ethernet MAC address");
 /* Transmitter timeout */
 #define TX_TIMEOUT (2 * HZ)
 
+#define FEC_PAUSE_FLAG_AUTONEG	0x1
+#define FEC_PAUSE_FLAG_ENABLE	0x2
+
 static int mii_cnt;
 
 static struct bufdesc *fec_enet_get_nextdesc(struct bufdesc *bdp, int is_ex)
@@ -470,6 +481,25 @@ fec_restart(struct net_device *ndev, int duplex)
 		}
 #endif
 	}
+
+	/* enable pause frame*/
+	if ((fep->pause_flag & FEC_PAUSE_FLAG_ENABLE) ||
+	    ((fep->pause_flag & FEC_PAUSE_FLAG_AUTONEG) &&
+	     fep->phy_dev && fep->phy_dev->pause)) {
+		rcntl |= FEC_ENET_FCE;
+
+		/* set FIFO thresh hold parameter to reduce overrun */
+		writel(FEC_ENET_RSEM_V, fep->hwp + FEC_R_FIFO_RSEM);
+		writel(FEC_ENET_RSFL_V, fep->hwp + FEC_R_FIFO_RSFL);
+		writel(FEC_ENET_RAEM_V, fep->hwp + FEC_R_FIFO_RAEM);
+		writel(FEC_ENET_RAFL_V, fep->hwp + FEC_R_FIFO_RAFL);
+
+		/* OPD */
+		writel(FEC_ENET_OPD_V, fep->hwp + FEC_OPD);
+	} else {
+		rcntl &= ~FEC_ENET_FCE;
+	}
+
 	writel(rcntl, fep->hwp + FEC_R_CNTRL);
 
 	if (id_entry->driver_data & FEC_QUIRK_ENET_MAC) {
@@ -1016,8 +1046,10 @@ static int fec_enet_mii_probe(struct net_device *ndev)
 	}
 
 	/* mask with MAC supported features */
-	if (id_entry->driver_data & FEC_QUIRK_HAS_GBIT)
+	if (id_entry->driver_data & FEC_QUIRK_HAS_GBIT) {
 		phy_dev->supported &= PHY_GBIT_FEATURES;
+		phy_dev->supported |= SUPPORTED_Pause;
+	}
 	else
 		phy_dev->supported &= PHY_BASIC_FEATURES;
 
@@ -1203,7 +1235,55 @@ static int fec_enet_get_ts_info(struct net_device *ndev,
 	}
 }
 
+static void fec_enet_get_pauseparam(struct net_device *ndev,
+				    struct ethtool_pauseparam *pause)
+{
+	struct fec_enet_private *fep = netdev_priv(ndev);
+
+	pause->autoneg = (fep->pause_flag & FEC_PAUSE_FLAG_AUTONEG) != 0;
+	pause->tx_pause = (fep->pause_flag & FEC_PAUSE_FLAG_ENABLE) != 0;
+	pause->rx_pause = pause->tx_pause;
+}
+
+static int fec_enet_set_pauseparam(struct net_device *ndev,
+				   struct ethtool_pauseparam *pause)
+{
+	struct fec_enet_private *fep = netdev_priv(ndev);
+
+	if (pause->tx_pause != pause->rx_pause) {
+		netdev_info(ndev,
+			"hardware only support enable/disable both tx and rx");
+		return -EINVAL;
+	}
+
+	fep->pause_flag = 0;
+
+	/* tx pause must be same as rx pause */
+	fep->pause_flag |= pause->rx_pause ? FEC_PAUSE_FLAG_ENABLE : 0;
+	fep->pause_flag |= pause->autoneg ? FEC_PAUSE_FLAG_AUTONEG : 0;
+
+	if (pause->rx_pause || pause->autoneg) {
+		fep->phy_dev->supported |= ADVERTISED_Pause;
+		fep->phy_dev->advertising |= ADVERTISED_Pause;
+	} else {
+		fep->phy_dev->supported &= ~ADVERTISED_Pause;
+		fep->phy_dev->advertising &= ~ADVERTISED_Pause;
+	}
+
+	if (pause->autoneg) {
+		if (netif_running(ndev))
+			fec_stop(ndev);
+		phy_start_aneg(fep->phy_dev);
+	}
+	if (netif_running(ndev))
+		fec_restart(ndev, 0);
+
+	return 0;
+}
+
 static const struct ethtool_ops fec_enet_ethtool_ops = {
+	.get_pauseparam		= fec_enet_get_pauseparam,
+	.set_pauseparam		= fec_enet_set_pauseparam,
 	.get_settings		= fec_enet_get_settings,
 	.set_settings		= fec_enet_set_settings,
 	.get_drvinfo		= fec_enet_get_drvinfo,
@@ -1643,6 +1723,11 @@ fec_probe(struct platform_device *pdev)
 	/* setup board info structure */
 	fep = netdev_priv(ndev);
 
+	/* default enable pause frame auto negotiation */
+	if (pdev->id_entry &&
+	    (pdev->id_entry->driver_data & FEC_QUIRK_HAS_GBIT))
+		fep->pause_flag |= FEC_PAUSE_FLAG_AUTONEG;
+
 	fep->hwp = ioremap(r->start, resource_size(r));
 	fep->pdev = pdev;
 	fep->dev_id = dev_id++;
diff --git a/drivers/net/ethernet/freescale/fec.h b/drivers/net/ethernet/freescale/fec.h
index 4862394..2ebedaf 100644
--- a/drivers/net/ethernet/freescale/fec.h
+++ b/drivers/net/ethernet/freescale/fec.h
@@ -48,6 +48,10 @@
 #define FEC_R_DES_START		0x180 /* Receive descriptor ring */
 #define FEC_X_DES_START		0x184 /* Transmit descriptor ring */
 #define FEC_R_BUFF_SIZE		0x188 /* Maximum receive buff size */
+#define FEC_R_FIFO_RSFL		0x190 /* Receive FIFO section full threshold */
+#define FEC_R_FIFO_RSEM		0x194 /* Receive FIFO section empty threshold */
+#define FEC_R_FIFO_RAEM		0x198 /* Receive FIFO almost empty threshold */
+#define FEC_R_FIFO_RAFL		0x19c /* Receive FIFO almost full threshold */
 #define FEC_MIIGSK_CFGR		0x300 /* MIIGSK Configuration reg */
 #define FEC_MIIGSK_ENR		0x308 /* MIIGSK Enable reg */
 
@@ -243,6 +247,7 @@ struct fec_enet_private {
 	struct	completion mdio_done;
 	int	irq[FEC_IRQ_NUM];
 	int	bufdesc_ex;
+	int	pause_flag;
 
 	struct ptp_clock *ptp_clock;
 	struct ptp_clock_info ptp_caps;
-- 
1.7.1

^ permalink raw reply related

* Re: [PATCH net-next] ipv6: fix ipv6_prefix_equal64_half mask conversion
From: YOSHIFUJI Hideaki @ 2013-01-17  3:36 UTC (permalink / raw)
  To: David Miller; +Cc: YOSHIFUJI Hideaki, netdev
In-Reply-To: <20130116.163729.1934084260354327196.davem@davemloft.net>

David Miller wrote:
> From: Fabio Baltieri <fabio.baltieri@linaro.org>
> Date: Wed, 16 Jan 2013 22:30:17 +0100
> 
>> Fix the 64bit optimized version of ipv6_prefix_equal to convert the
>> bitmask to network byte order only after the bit-shift.
>>
>> The bug was introduced in:
>>
>> 3867517 ipv6: 64bit version of ipv6_prefix_equal().
>>
>> Signed-off-by: Fabio Baltieri <fabio.baltieri@linaro.org>
> 
> Applied, thanks.
> 

Sorry for the mess.

I'm not seeing this in net-next so far, though I can see my
patch for ip6_flow_hdr() which you "applied" later.
Do I miss something?

--yoshfuji

^ permalink raw reply

* Re: Redefinition of struct in6_addr in <netinet/in.h> and <linux/in6.h>
From: Cong Wang @ 2013-01-17  3:40 UTC (permalink / raw)
  To: Mike Frysinger
  Cc: David Miller, bhutchings, libc-alpha, yoshfuji, tmb, eblake,
	netdev, linux-kernel, libvirt-list, tgraf, schwab, carlos
In-Reply-To: <201301161422.19202.vapier@gentoo.org>

On Wed, 2013-01-16 at 14:22 -0500, Mike Frysinger wrote:
> 
> but this is still too vague.  what headers/definitions do people want to see 
> simultaneously included ?  changes would be needed on both sides (kernel & C 
> library).
> 

Hi, Mike,

Please take a look at my first email in this thread. The user
application includes <linux/if_bridge.h> and <netinet/in.h>.

<linux/if_bridge.h> uses struct_in6 but doesn't include <linux/in6.h>
(this is my bad, sorry), an obvious fix is just including <linux/in6.h>.
But this immediately breaks applications which include
<linux/if_bridge.h> and <netinet/in.h>, just as what Thomas reported.

And if_bridge.h is kernel-specific, there is no corresponding glibc one,
so you can't blame applications which include both of them.

Thanks.

^ permalink raw reply

* Re: [PATCH net-next] ipv6: fix ipv6_prefix_equal64_half mask conversion
From: David Miller @ 2013-01-17  3:50 UTC (permalink / raw)
  To: yoshfuji; +Cc: netdev
In-Reply-To: <50F771BE.7070309@linux-ipv6.org>

From: YOSHIFUJI Hideaki <yoshfuji@linux-ipv6.org>
Date: Thu, 17 Jan 2013 12:36:30 +0900

> I'm not seeing this in net-next so far, though I can see my
> patch for ip6_flow_hdr() which you "applied" later.
> Do I miss something?

It's sitting in my local GIT tree on a machine I use during the day at
an office, I forgot to push some changes out before I headed home.

I'll fix it up tomorrow.

^ permalink raw reply

* Re: [Patch net-next v4] netpoll: fix a rtnl lock assertion failure
From: Cong Wang @ 2013-01-17  3:30 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: netdev, Jiri Pirko, David S. Miller
In-Reply-To: <1358385885.32167.21.camel@edumazet-glaptop>

On Wed, 2013-01-16 at 17:24 -0800, Eric Dumazet wrote:
> On Tue, 2013-01-15 at 17:34 +0800, Cong Wang wrote:
> > From: Cong Wang <amwang@redhat.com>
> > 
> > v4: hold rtnl lock for the whole netpoll_setup()
> > v3: remove the comment
> > v2: use RCU read lock
> > 
> > This patch fixes the following warning:
> > 
> > [   72.013864] RTNL: assertion failed at net/core/dev.c (4955)
> > [   72.017758] Pid: 668, comm: netpoll-prep-v6 Not tainted 3.8.0-rc1+ #474
> > [   72.019582] Call Trace:
> > [   72.020295]  [<ffffffff8176653d>] netdev_master_upper_dev_get+0x35/0x58
> > [   72.022545]  [<ffffffff81784edd>] netpoll_setup+0x61/0x340
> > [   72.024846]  [<ffffffff815d837e>] store_enabled+0x82/0xc3
> > [   72.027466]  [<ffffffff815d7e51>] netconsole_target_attr_store+0x35/0x37
> > [   72.029348]  [<ffffffff811c3479>] configfs_write_file+0xe2/0x10c
> > [   72.030959]  [<ffffffff8115d239>] vfs_write+0xaf/0xf6
> > [   72.032359]  [<ffffffff81978a05>] ? sysret_check+0x22/0x5d
> > [   72.033824]  [<ffffffff8115d453>] sys_write+0x5c/0x84
> > [   72.035328]  [<ffffffff819789d9>] system_call_fastpath+0x16/0x1b
> > 
> > In case of other races, hold rtnl lock for the entire netpoll_setup() function.
> > 
> > Cc: Eric Dumazet <eric.dumazet@gmail.com>
> > Cc: Jiri Pirko <jiri@resnulli.us>
> > Cc: David S. Miller <davem@davemloft.net>
> > Signed-off-by: Cong Wang <amwang@redhat.com>
> > ---
> > diff --git a/net/core/netpoll.c b/net/core/netpoll.c
> 
> ...
> 
> >  	if (np->dev_name)
> > -		ndev = dev_get_by_name(&init_net, np->dev_name);
> > +		ndev = __dev_get_by_name(&init_net, np->dev_name);
> 
> This change brings interesting bugs.

Hmm, I didn't realize __dev_get_by_name() doesn't hold the device, so
just call dev_hold() after this?

diff --git a/net/core/netpoll.c b/net/core/netpoll.c
index a5ad1c1..a9b1004 100644
--- a/net/core/netpoll.c
+++ b/net/core/netpoll.c
@@ -1056,6 +1056,7 @@ int netpoll_setup(struct netpoll *np)
                err = -ENODEV;
                goto unlock;
        }
+       dev_hold(ndev);
 
        if (netdev_master_upper_dev_get(ndev)) {
                np_err(np, "%s is a slave device, aborting\n",
np->dev_name);


> 
> All the "goto put;" are basically wrong, and the section waiting for the
> carrier and releasing/getting rtnl is buggy.

Either we have to sleep for few seconds with rtnl lock held, or leave as
it is. The original code doesn't hold rtnl lock either.

Thanks!

^ permalink raw reply related

* Re: [Patch net-next v4] netpoll: fix a rtnl lock assertion failure
From: David Miller @ 2013-01-17  3:52 UTC (permalink / raw)
  To: eric.dumazet; +Cc: amwang, netdev, jiri
In-Reply-To: <1358385885.32167.21.camel@edumazet-glaptop>

From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Wed, 16 Jan 2013 17:24:45 -0800

>>  	if (np->dev_name)
>> -		ndev = dev_get_by_name(&init_net, np->dev_name);
>> +		ndev = __dev_get_by_name(&init_net, np->dev_name);
 ...
> Please revert this part.

You mean just revert that hunk above that made it use the
non-refcounting version of dev_get_by_name()?

^ permalink raw reply

* Re: [Patch net-next v4] netpoll: fix a rtnl lock assertion failure
From: David Miller @ 2013-01-17  3:54 UTC (permalink / raw)
  To: amwang; +Cc: eric.dumazet, netdev, jiri
In-Reply-To: <1358393418.3855.3.camel@cr0>

From: Cong Wang <amwang@redhat.com>
Date: Thu, 17 Jan 2013 11:30:18 +0800

> On Wed, 2013-01-16 at 17:24 -0800, Eric Dumazet wrote:
>> On Tue, 2013-01-15 at 17:34 +0800, Cong Wang wrote:
>> >  	if (np->dev_name)
>> > -		ndev = dev_get_by_name(&init_net, np->dev_name);
>> > +		ndev = __dev_get_by_name(&init_net, np->dev_name);
>> 
>> This change brings interesting bugs.
> 
> Hmm, I didn't realize __dev_get_by_name() doesn't hold the device, so
> just call dev_hold() after this?

Why not just... call dev_get_by_name()?  It doesn't hurt to over-RCU
lock.

^ permalink raw reply

* Re: [libvirt] Redefinition of struct in6_addr in <netinet/in.h> and <linux/in6.h>
From: Jike Song @ 2013-01-17  3:55 UTC (permalink / raw)
  To: David Miller
  Cc: libc-alpha, amwang, yoshfuji, netdev, linux-kernel, libvirt-list,
	tgraf, carlos, schwab, bhutchings, vapier, tmb
In-Reply-To: <20130116.135959.90676245194307972.davem@davemloft.net>


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

On Thu, Jan 17, 2013 at 2:59 AM, David Miller <davem@davemloft.net> wrote:

>
> When GLIBC doesn't provide it's own definition of some networking
> macros or interfaces that the kernel provides, people include the
> kernel header.
>

Recently I got a problem when copying a structure from kernel to userspace,
after debugging I found:

kernel:   include/linux/inet.h

#define INET6_ADDRSTRLEN        (48)

glibc:  /usr/include/netinet/in.h

#define INET6_ADDRSTRLEN 46


Any reason to differentiate them from each other?

-- 
Thanks,
Jike

[-- Attachment #1.2: Type: text/html, Size: 1198 bytes --]

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



^ permalink raw reply

* [PATCH] ipv6: check if dereference of ipv6 header is safe
From: Hannes Frederic Sowa @ 2013-01-17  3:56 UTC (permalink / raw)
  To: netdev

When ipip6_rcv gets called we are sure that we have a full blown
ipv4 packet header in the linear skb buffer (this is checked by
xfrm4_mode_tunnel_input). Because we dereference fields of the inner
ipv6 header we should actually check for the length of the sum of the
ipv4 and ipv6 header.

If the skb is too short this packet could very well be destined for
another tunnel. So we should notify the caller accordingly (albeit
currently xfrm4_mode_tunnel_input does not care; this could need another
patch).

Signed-off-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
---
 net/ipv6/sit.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/ipv6/sit.c b/net/ipv6/sit.c
index 2b4c15a..389d6e3 100644
--- a/net/ipv6/sit.c
+++ b/net/ipv6/sit.c
@@ -612,8 +612,8 @@ static int ipip6_rcv(struct sk_buff *skb)
 	struct ip_tunnel *tunnel;
 	int err;
 
-	if (!pskb_may_pull(skb, sizeof(struct ipv6hdr)))
-		goto out;
+	if (!pskb_may_pull(skb, sizeof(struct iphdr) + sizeof(struct ipv6hdr)))
+		return 1;
 
 	iph = ip_hdr(skb);
 
-- 
1.7.11.7

^ permalink raw reply related

* Re: [Patch net-next v4] netpoll: fix a rtnl lock assertion failure
From: Cong Wang @ 2013-01-17  4:00 UTC (permalink / raw)
  To: David Miller; +Cc: eric.dumazet, netdev, jiri
In-Reply-To: <20130116.225238.238898828520417471.davem@davemloft.net>

On Wed, 2013-01-16 at 22:52 -0500, David Miller wrote:
> From: Eric Dumazet <eric.dumazet@gmail.com>
> Date: Wed, 16 Jan 2013 17:24:45 -0800
> 
> >>  	if (np->dev_name)
> >> -		ndev = dev_get_by_name(&init_net, np->dev_name);
> >> +		ndev = __dev_get_by_name(&init_net, np->dev_name);
>  ...
> > Please revert this part.
> 
> You mean just revert that hunk above that made it use the
> non-refcounting version of dev_get_by_name()?

But there is no reason to take both rtnl lock and RCU read lock,
although that is fine.

I think just adding dev_hold() is enough.

^ permalink raw reply


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