Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH net v2] be2net: add dma_mapping_error() check for dma_map_page()
From: David Miller @ 2014-01-16  0:50 UTC (permalink / raw)
  To: ivecera; +Cc: netdev, sathya.perla, subbu.seetharaman, ajit.khaparde
In-Reply-To: <1389780694-6299-1-git-send-email-ivecera@redhat.com>

From: Ivan Vecera <ivecera@redhat.com>
Date: Wed, 15 Jan 2014 11:11:34 +0100

> The driver does not check value returned by dma_map_page. The patch
> fixes this.
> 
> v2: Removed the bugfix for non-bug ;-) (thanks Sathya)
> 
> Signed-off-by: Ivan Vecera <ivecera@redhat.com>

Applied, thank you.

^ permalink raw reply

* Re: [PATCH v4 net-next 2/4] sh_eth: Add support for r7s72100
From: Simon Horman @ 2014-01-16  0:49 UTC (permalink / raw)
  To: Sergei Shtylyov
  Cc: David S. Miller, netdev, linux-sh, linux-arm-kernel, Magnus Damm
In-Reply-To: <52D70F12.9080800@cogentembedded.com>

On Thu, Jan 16, 2014 at 01:43:30AM +0300, Sergei Shtylyov wrote:
> Hello.
> 
> On 01/09/2014 08:03 AM, Simon Horman wrote:
> 
> >>>This is a fast ethernet controller.
> 
> >>>Signed-off-by: Simon Horman <horms+renesas@verge.net.au>
> 
> >>[...]
> 
> >>>diff --git a/drivers/net/ethernet/renesas/sh_eth.c b/drivers/net/ethernet/renesas/sh_eth.c
> >>>index 4b38533..cc6d4af 100644
> >>>--- a/drivers/net/ethernet/renesas/sh_eth.c
> >>>+++ b/drivers/net/ethernet/renesas/sh_eth.c
> >>>@@ -190,6 +190,59 @@ static const u16 sh_eth_offset_fast_rcar[SH_ETH_MAX_REGISTER_OFFSET] = {
> [...]
> >>>@@ -701,6 +762,35 @@ static struct sh_eth_cpu_data r8a7740_data = {
> >>>  	.shift_rd0	= 1,
> >>>  };
> >>>
> >>>+/* R7S72100 */
> >>>+static struct sh_eth_cpu_data r7s72100_data = {
> >>>+	.chip_reset	= sh_eth_chip_reset,
> >>>+	.set_duplex	= sh_eth_set_duplex,
> >>>+
> >>>+	.register_type	= SH_ETH_REG_FAST_RZ,
> >>>+
> >>>+	.ecsr_value	= ECSR_ICD,
> >>>+	.ecsipr_value	= ECSIPR_ICDIP,
> >>>+	.eesipr_value	= 0xff7f009f,
> >>>+
> >>>+	.tx_check	= EESR_TC1 | EESR_FTC,
> >>>+	.eesr_err_check	= EESR_TWB1 | EESR_TWB | EESR_TABT | EESR_RABT |
> >>>+			  EESR_RFE | EESR_RDE | EESR_RFRMER | EESR_TFE |
> >>>+			  EESR_TDE | EESR_ECI,
> >>>+	.fdr_value	= 0x0000070f,
> >>>+	.rmcr_value	= RMCR_RNC,
> >>>+
> >>>+	.apr		= 1,
> >>>+	.mpr		= 1,
> >>>+	.tpauser	= 1,
> >>>+	.hw_swap	= 1,
> >>>+	.rpadir		= 1,
> >>>+	.rpadir_value   = 2 << 16,
> >>>+	.no_trimd	= 1,
> >>>+	.tsu		= 1,
> >>>+	.shift_rd0	= 1,
> 
> >>    Perhaps this field should be renamed to something talking about
> >>check summing support (since bits 0..15 of RD0 contain a frame check
> >>sum for those SoCs). Or maybe it should be just merged with the
> >>'hw_crc' field...
> 
> >I have no feelings about that one way or another.
> 
>    Do you happen to have R8A7740 manual by chance? If so, does it
> talk about RX check summing support and using RD0 for that?

Yes and yes.

I have taken a quick look and the documentation for RX checksumming on the
R8A7740 appears to be very similar if not the same as that of the R7S72100.

In particular both refer to using the bottom 16 bits of RD0 as
containing the packet checksum.

> >But it seems to be orthogonal to this patch.
> 
>    Of course, was a note to self. :-)
> 
> [...]
> >>>@@ -880,6 +970,8 @@ static unsigned long sh_eth_get_edtrr_trns(struct sh_eth_private *mdp)
> >>>  {
> >>>  	if (sh_eth_is_gether(mdp))
> >>>  		return EDTRR_TRNS_GETHER;
> >>>+	else if (sh_eth_is_rz_fast_ether(mdp))
> >>>+		return EDTRR_TRNS_RZ_ETHER;
> 
> >>    I'd just merge this with the GEther case.
> 
> >Sure, but in that case should we change the name.
> >As both you and Magnus pointed out to me, the rz is not gigabit.
> 
>    See below.
> 
> >>>  	else
> >>>  		return EDTRR_TRNS_ETHER;
> >>>  }
> >>[...]
> >>>@@ -1062,7 +1155,8 @@ static void sh_eth_ring_format(struct net_device *ndev)
> >>>  		if (i == 0) {
> >>>  			/* Tx descriptor address set */
> >>>  			sh_eth_write(ndev, mdp->tx_desc_dma, TDLAR);
> >>>-			if (sh_eth_is_gether(mdp))
> >>>+			if (sh_eth_is_gether(mdp) ||
> >>>+			    sh_eth_is_rz_fast_ether(mdp))
> >>>  				sh_eth_write(ndev, mdp->tx_desc_dma, TDFAR);
> 
> >>    Hmm, TDFAR exists also on SH4 Ethers...
> 
> >Lets fix that separately.
> 
>    Of course, was just another not to self.
> 
> [...]
> >>>diff --git a/drivers/net/ethernet/renesas/sh_eth.h b/drivers/net/ethernet/renesas/sh_eth.h
> >>>index 0fe35b7..0bcde90 100644
> >>>--- a/drivers/net/ethernet/renesas/sh_eth.h
> >>>+++ b/drivers/net/ethernet/renesas/sh_eth.h
> [...]
> >>>@@ -191,6 +192,7 @@ enum DMAC_M_BIT {
> >>>  /* EDTRR */
> >>>  enum DMAC_T_BIT {
> >>>  	EDTRR_TRNS_GETHER = 0x03,
> >>>+	EDTRR_TRNS_RZ_ETHER = 0x03,
> 
> >>    I doubt we need a special case here. You didn't introduce one for
> >>the software reset bits.
> 
> >True, but RZ is not Gigabit. So I think we either need two names
> >or to choose a more generic name.
> 
>    Well, R7S72100 manual calls these bits just TR[1:0]. Don't know
> what SoCs having Gigabit call it in the manuals...
> 
> >>>  	EDTRR_TRNS_ETHER = 0x01,
> 
>    R-Car manuals seem to call the bit TRNS (as well as the
> prehistoric SH manuals probably). Perhaps we could use that
> difference, TRNS vs TR, don't know...

Perhaps we should just leave it as-is, using EDTRR_TRNS_GETHER and
EDTRR_TRNS_RZ_ETHER, after all.

At least until we can think of a better names :)

^ permalink raw reply

* Re: [PATCH net] bnx2x: Don't release PCI bars on shutdown
From: David Miller @ 2014-01-16  0:49 UTC (permalink / raw)
  To: yuvalmin; +Cc: netdev, ariele
In-Reply-To: <1389780330-6927-1-git-send-email-yuvalmin@broadcom.com>

From: Yuval Mintz <yuvalmin@broadcom.com>
Date: Wed, 15 Jan 2014 12:05:30 +0200

> The bnx2x driver in its pci shutdown() callback releases its pci bars (in the
> same manner it does during its pci remove() callback).
> During a system reboot while VFs are enabled, its possible for the VF's remove
> to be called (as a result of pci_disable_sriov()) after its shutdown callback
> has already finished running; This will cause a paging request fault as the VF
> tries to access the pci bar which it has previously released, crashing the
> system.
> 
> This patch further differentiates the shutdown and remove callbacks, preventing the
> pci release procedures from being called during shutdown.
> 
> Signed-off-by: Yuval Mintz <yuvalmin@broadcom.com>
> Signed-off-by: Ariel Elior <ariele@broadcom.com>

Applied, thanks.

^ permalink raw reply

* Re: [PATCH net-next] sctp: create helper function to enable|disable sackdelay
From: David Miller @ 2014-01-16  0:48 UTC (permalink / raw)
  To: wangweidong1; +Cc: vyasevich, nhorman, linux-sctp, netdev
In-Reply-To: <52D653B1.4040708@huawei.com>

From: Wang Weidong <wangweidong1@huawei.com>
Date: Wed, 15 Jan 2014 17:24:01 +0800

> add sctp_spp_sackdelay_{enable|disable} helper function for
> avoiding code duplication. 
> 
> Signed-off-by: Wang Weidong <wangweidong1@huawei.com>

Applied.

^ permalink raw reply

* Re: [PATCH net-next RFC] virtio-net: drop rq->max and rq->num
From: David Miller @ 2014-01-16  0:46 UTC (permalink / raw)
  To: rusty; +Cc: netdev, virtualization, linux-kernel, mst
In-Reply-To: <87zjmwvlzl.fsf@rustcorp.com.au>

From: Rusty Russell <rusty@rustcorp.com.au>
Date: Thu, 16 Jan 2014 10:25:26 +1030

> Rusty Russell <rusty@rustcorp.com.au> writes:
>> Jason Wang <jasowang@redhat.com> writes:
>>> It looks like there's no need for those two fields:
>>>
>>> - Unless there's a failure for the first refill try, rq->max should be always
>>>   equal to the vring size.
>>> - rq->num is only used to determine the condition that we need to do the refill,
>>>   we could check vq->num_free instead.
>>> - rq->num was required to be increased or decreased explicitly after each
>>>   get/put which results a bad API.
>>>
>>> So this patch removes them both to make the code simpler.
>>
>> Nice.  These fields date from when the vq struct was opaque.
>>
>> Applied,
>> Rusty.
> 
> Oops, this doesn't require any core virtio changes, so it's for DaveM:
> 
> Acked-by: Rusty Russell <rusty@rustcorp.com.au>

Jason please repost this with Rusty's ACK, thanks.

^ permalink raw reply

* Re: [PATCH net-next 2/2] reciprocal_divide: correction/update of the algorithm
From: Hannes Frederic Sowa @ 2014-01-16  0:46 UTC (permalink / raw)
  To: Ben Hutchings
  Cc: Daniel Borkmann, davem, netdev, linux-kernel, Eric Dumazet,
	Austin S Hemmelgarn, Jesse Gross, Jamal Hadi Salim,
	Stephen Hemminger, Matt Mackall, Pekka Enberg, Christoph Lameter,
	Andy Gospodarek, Veaceslav Falico, Jay Vosburgh, Jakub Zawadzki
In-Reply-To: <1389831470.11912.40.camel@bwh-desktop.uk.level5networks.com>

On Thu, Jan 16, 2014 at 12:17:50AM +0000, Ben Hutchings wrote:
> On Thu, 2014-01-16 at 00:23 +0100, Daniel Borkmann wrote:
> [...]
> > --- a/include/linux/reciprocal_div.h
> > +++ b/include/linux/reciprocal_div.h
> [...]
> > + * RECIPROCAL_VALUE_TO_ZERO can be used to express an element, which
> > + * used as the argument to reciprocal_divide always yields zero.
> >   */
> [...]
> > +#define RECIPROCAL_VALUE_RESULT_TO_ZERO ((struct reciprocal_value){.sh1 = 32})
> [...]
> > +static inline u32 reciprocal_divide(u32 a, struct reciprocal_value R)
> >  {
> > -	return (u32)(((u64)A * R) >> 32);
> > +	u32 t = (u32)(((u64)a * R.m) >> 32);
> > +	return (t + ((a - t) >> R.sh1)) >> R.sh2;
> [...]
> 
> (a - t) has type u32.
> So (a - t) >> 32 has undefined behaviour.

Good catch, totally overlooked that.

Thanks,

  Hannes

^ permalink raw reply

* Re: [RFC PATCH net-next] etherdevice: Use ether_addr_copy to copy an Ethernet address
From: David Miller @ 2014-01-16  0:45 UTC (permalink / raw)
  To: joe; +Cc: netdev, linux-arm-kernel, linux-arch
In-Reply-To: <1389830878.14001.39.camel@joe-AO722>

From: Joe Perches <joe@perches.com>
Date: Wed, 15 Jan 2014 16:07:58 -0800

> If you want the ones for net-next net/ now (but not
> for batman-adv, that maybe could use a new function like
> ether_addr_copy_unaligned) here's a changestat.
> 
> Otherwise, I'll wait for the next cycle.

This looks fine, why don't you toss it my way over the weekend as I
still have some backlog to process at the moment?

Thanks.

^ permalink raw reply

* Re: [PATCH net-next 1/2] random32: add prandom_u32_lt_N and convert "misuses" of reciprocal_divide
From: Joe Perches @ 2014-01-16  0:29 UTC (permalink / raw)
  To: Daniel Borkmann
  Cc: davem, netdev, linux-kernel, Jakub Zawadzki, Eric Dumazet,
	Hannes Frederic Sowa
In-Reply-To: <1389828228-30312-2-git-send-email-dborkman@redhat.com>

On Thu, 2014-01-16 at 00:23 +0100, Daniel Borkmann wrote:
> Many functions have open coded a function that return a random
> number in range [0,N-1]. Also, only because we have a function
> that is named reciprocal_divide(), it has not much to do with
> the pupose where it is being used when a previous reciprocal_value()
> has not been obtained.

prandom_u32_lt_N?

I do not like the camelcase name and thought the
prandom_u32_max was better.

How about using

u32 prandom_u32_max(u32 max)
{
	return (u32)(((u64)prandom_u32() * max) >> 32);
}

u32 prandom_u32_range(u32 a, u32 b)
{
	if (b < a)
		swap(a, b);

	return a + (u32)(((u64)prandom_u32() * (b - a)) >> 32);
}

^ permalink raw reply

* Re: [PATCH net-next RFC] virtio-net: drop rq->max and rq->num
From: Rusty Russell @ 2014-01-15 23:55 UTC (permalink / raw)
  To: Jason Wang, mst, virtualization, netdev, linux-kernel; +Cc: Jason Wang
In-Reply-To: <87mwixx670.fsf@rustcorp.com.au>

Rusty Russell <rusty@rustcorp.com.au> writes:
> Jason Wang <jasowang@redhat.com> writes:
>> It looks like there's no need for those two fields:
>>
>> - Unless there's a failure for the first refill try, rq->max should be always
>>   equal to the vring size.
>> - rq->num is only used to determine the condition that we need to do the refill,
>>   we could check vq->num_free instead.
>> - rq->num was required to be increased or decreased explicitly after each
>>   get/put which results a bad API.
>>
>> So this patch removes them both to make the code simpler.
>
> Nice.  These fields date from when the vq struct was opaque.
>
> Applied,
> Rusty.

Oops, this doesn't require any core virtio changes, so it's for DaveM:

Acked-by: Rusty Russell <rusty@rustcorp.com.au>

Thanks,
Rusty.

>> Cc: Rusty Russell <rusty@rustcorp.com.au>
>> Cc: Michael S. Tsirkin <mst@redhat.com>
>> Signed-off-by: Jason Wang <jasowang@redhat.com>
>> ---
>>  drivers/net/virtio_net.c | 16 +++-------------
>>  1 file changed, 3 insertions(+), 13 deletions(-)
>>
>> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
>> index c51a988..4e1bce3 100644
>> --- a/drivers/net/virtio_net.c
>> +++ b/drivers/net/virtio_net.c
>> @@ -72,9 +72,6 @@ struct receive_queue {
>>  
>>  	struct napi_struct napi;
>>  
>> -	/* Number of input buffers, and max we've ever had. */
>> -	unsigned int num, max;
>> -
>>  	/* Chain pages by the private ptr. */
>>  	struct page *pages;
>>  
>> @@ -360,7 +357,6 @@ static struct sk_buff *receive_mergeable(struct net_device *dev,
>>  		}
>>  
>>  		page = virt_to_head_page(buf);
>> -		--rq->num;
>>  
>>  		num_skb_frags = skb_shinfo(curr_skb)->nr_frags;
>>  		if (unlikely(num_skb_frags == MAX_SKB_FRAGS)) {
>> @@ -406,7 +402,6 @@ err_skb:
>>  		}
>>  		page = virt_to_head_page(buf);
>>  		put_page(page);
>> -		--rq->num;
>>  	}
>>  err_buf:
>>  	dev->stats.rx_dropped++;
>> @@ -628,10 +623,7 @@ static bool try_fill_recv(struct receive_queue *rq, gfp_t gfp)
>>  		oom = err == -ENOMEM;
>>  		if (err)
>>  			break;
>> -		++rq->num;
>>  	} while (rq->vq->num_free);
>> -	if (unlikely(rq->num > rq->max))
>> -		rq->max = rq->num;
>>  	if (unlikely(!virtqueue_kick(rq->vq)))
>>  		return false;
>>  	return !oom;
>> @@ -699,11 +691,10 @@ again:
>>  	while (received < budget &&
>>  	       (buf = virtqueue_get_buf(rq->vq, &len)) != NULL) {
>>  		receive_buf(rq, buf, len);
>> -		--rq->num;
>>  		received++;
>>  	}
>>  
>> -	if (rq->num < rq->max / 2) {
>> +	if (rq->vq->num_free > virtqueue_get_vring_size(rq->vq) / 2) {
>>  		if (!try_fill_recv(rq, GFP_ATOMIC))
>>  			schedule_delayed_work(&vi->refill, 0);
>>  	}
>> @@ -1398,9 +1389,7 @@ static void free_unused_bufs(struct virtnet_info *vi)
>>  				give_pages(&vi->rq[i], buf);
>>  			else
>>  				dev_kfree_skb(buf);
>> -			--vi->rq[i].num;
>>  		}
>> -		BUG_ON(vi->rq[i].num != 0);
>>  	}
>>  }
>>  
>> @@ -1671,7 +1660,8 @@ static int virtnet_probe(struct virtio_device *vdev)
>>  		try_fill_recv(&vi->rq[i], GFP_KERNEL);
>>  
>>  		/* If we didn't even get one input buffer, we're useless. */
>> -		if (vi->rq[i].num == 0) {
>> +		if (vi->rq[i].vq->num_free ==
>> +		    virtqueue_get_vring_size(vi->rq[i].vq)) {
>>  			free_unused_bufs(vi);
>>  			err = -ENOMEM;
>>  			goto free_recv_bufs;
>> -- 
>> 1.8.3.2
> --
> 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 v2 00/16] wl1251 patches from linux-n900 tree
From: Pavel Machek @ 2014-01-16  0:21 UTC (permalink / raw)
  To: John W. Linville
  Cc: Pali Rohár, Luciano Coelho, linux-wireless, netdev,
	linux-kernel, freemangordon, aaro.koskinen, sre, joni.lapilainen,
	Johannes Berg, Felipe Contreras
In-Reply-To: <20140106200050.GG10885@tuxdriver.com>

On Mon 2014-01-06 15:00:50, John W. Linville wrote:
> On Tue, Dec 31, 2013 at 10:47:29AM +0100, Pali Rohár wrote:
> > On Sunday 08 December 2013 10:24:58 Pali Rohár wrote:
> > > Hello, I'm sending wl1251 patches from linux-n900 tree [1] for
> > > comments. More patches come from David's monitor & packet
> > > injection work. Patches are tested with 3.12 rc5 kernel on
> > > Nokia N900.
> > > 
> > > Second version contains new patch for fixing NULL pointer
> > > dereference which sometimes cause kernel panic and fixes code
> > > suggested by Pavel Machek.
>  
> > So far, there are no new comments for patches 01, 03-13 and 16. 
> > Are there any other problems with that patches or can be accepted 
> > for upstream?
>  
> They are probably fine to merge now.  Of course, I was still deleting
> wl12xx patches when they were originally posted, so I'll need someone
> to resend the patches to me...

Ping? Did they get merged? Did you receive the patches?

Regards,
								Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

^ permalink raw reply

* Re: [PATCH net-next 2/2] reciprocal_divide: correction/update of the algorithm
From: Ben Hutchings @ 2014-01-16  0:17 UTC (permalink / raw)
  To: Daniel Borkmann
  Cc: davem, netdev, linux-kernel, Hannes Frederic Sowa, Eric Dumazet,
	Austin S Hemmelgarn, Jesse Gross, Jamal Hadi Salim,
	Stephen Hemminger, Matt Mackall, Pekka Enberg, Christoph Lameter,
	Andy Gospodarek, Veaceslav Falico, Jay Vosburgh, Jakub Zawadzki
In-Reply-To: <1389828228-30312-3-git-send-email-dborkman@redhat.com>

On Thu, 2014-01-16 at 00:23 +0100, Daniel Borkmann wrote:
[...]
> --- a/include/linux/reciprocal_div.h
> +++ b/include/linux/reciprocal_div.h
[...]
> + * RECIPROCAL_VALUE_TO_ZERO can be used to express an element, which
> + * used as the argument to reciprocal_divide always yields zero.
>   */
[...]
> +#define RECIPROCAL_VALUE_RESULT_TO_ZERO ((struct reciprocal_value){.sh1 = 32})
[...]
> +static inline u32 reciprocal_divide(u32 a, struct reciprocal_value R)
>  {
> -	return (u32)(((u64)A * R) >> 32);
> +	u32 t = (u32)(((u64)a * R.m) >> 32);
> +	return (t + ((a - t) >> R.sh1)) >> R.sh2;
[...]

(a - t) has type u32.
So (a - t) >> 32 has undefined behaviour.

Ben.

-- 
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.

^ permalink raw reply

* Re: [PATCH v5 net-next 2/4] sh_eth: Add support for r7s72100
From: Simon Horman @ 2014-01-16  0:14 UTC (permalink / raw)
  To: Sergei Shtylyov
  Cc: David S. Miller, netdev, linux-sh, linux-arm-kernel, Magnus Damm
In-Reply-To: <52D70B03.9040506@cogentembedded.com>

On Thu, Jan 16, 2014 at 01:26:11AM +0300, Sergei Shtylyov wrote:
> Hello.
> 
> On 01/15/2014 09:12 AM, Simon Horman wrote:
> 
> >This is a fast ethernet controller.
> 
>    I have to say it's not exact enough patch description: R7S72100
> is not Ethernet controller itself, it's a SoC containing the
> Ethernet controller.

I will update the text to the following:

The r7s72100 SoC includes a fast ethernet controller.

> 
> >Signed-off-by: Simon Horman <horms+renesas@verge.net.au>
> 
> >---
> >v5
> >* As suggested by Sergei Shtylyov
> >   - Do not use sh_eth_chip_reset_r8a7740 as it accesses non-existent
> >     RMII registers. Instead use sh_eth_chip_reset.
> >   - Do not use sh_eth_set_rate_gether as it accesses non-existent registers.
> >   - Do not use reserved LCHNG bit of ECSR
> >   - Do not use reserved LCHNGIP bit of ECSIPR
> >   - Document that R8A779x also needs a 16 bit shift of the RFS bits
> >   - Do not document that the R7S72100 has GECMR, it does not
> 
>   The above change list was moved from v2 section and doesn't match
> the real changes done in v5. ;-)

I'll fix that up so that when its automatically discarded by git
it can rest in peace in /dev/null.

> 
> >v4
> >* As requested by David Miller
> >   - Use a boolean for the return value of sh_eth_is_rz_fast_ether()
> >   - Correct coding style in sh_eth_get_stats()
> 
> >v3
> >* No change
> 
> >v2
> >* As suggested by Magnus Damm and Sergei Shtylyov
> >   - r7s72100 ethernet is not gigabit so do not refer to it as such
> 
> >* As suggested by Magnus Damm
> >   - As RZ specific register layout rather than using the gigabit layout
> >     which includes registers that do not exist on this chip.
> >---
> >  drivers/net/ethernet/renesas/sh_eth.c | 126 ++++++++++++++++++++++++++++++++--
> >  drivers/net/ethernet/renesas/sh_eth.h |   3 +-
> >  2 files changed, 121 insertions(+), 8 deletions(-)
> 
> >diff --git a/drivers/net/ethernet/renesas/sh_eth.c b/drivers/net/ethernet/renesas/sh_eth.c
> >index 4f5cfad..a7a0555 100644
> >--- a/drivers/net/ethernet/renesas/sh_eth.c
> >+++ b/drivers/net/ethernet/renesas/sh_eth.c
> >@@ -190,6 +190,64 @@ static const u16 sh_eth_offset_fast_rcar[SH_ETH_MAX_REGISTER_OFFSET] = {
> >  	[TRIMD]		= 0x027c,
> >  };
> >
> >+static const u16 sh_eth_offset_fast_rz[SH_ETH_MAX_REGISTER_OFFSET] = {
> 
>    Shouldn't this map precede R-Car one since this SoC is newer the
> same way you've reordered *enum* values, etc.? Sorry for not
> noticing in the previous review...

I will move it, thanks.

> [...]
> >+	[ARSTR]		= 0x0000,
> >+	[TSU_CTRST]	= 0x0004,
> >+	[TSU_VTAG0]	= 0x0058,
> >+	[TSU_ADSBSY]	= 0x0060,
> >+	[TSU_TEN]	= 0x0064,
> >+	[TXNLCR0]	= 0x0080,
> >+	[TXALCR0]	= 0x0084,
> >+	[RXNLCR0]	= 0x0088,
> >+	[RXALCR0]	= 0x008C,
> 
>    Well, the above counter register subgroup stands out from the
> TSU_* registers in the Gigabit mapping, not sure if we should follow
> that. These registers are not currently used anyway...

I will add a blank line.

> 
> >+	[TSU_ADRH0]	= 0x0100,
> >+	[TSU_ADRL0]	= 0x0104,
> >+	[TSU_ADRH31]	= 0x01f8,
> >+	[TSU_ADRL31]	= 0x01fc,
> >+};
> >+
> >  static const u16 sh_eth_offset_fast_sh4[SH_ETH_MAX_REGISTER_OFFSET] = {
> >  	[ECMR]		= 0x0100,
> >  	[RFLR]		= 0x0108,
> >@@ -318,6 +376,14 @@ static bool sh_eth_is_gether(struct sh_eth_private *mdp)
> >  		return false;
> >  }
> >
> >+static bool sh_eth_is_rz_fast_ether(struct sh_eth_private *mdp)
> >+{
> >+	if (mdp->reg_offset == sh_eth_offset_fast_rz)
> >+		return true;
> >+	else
> >+		return false;
> 
>    Perhaps you should compress the above functions to one-liners as
> Joe has suggested. Or I/you could do it in a separate patch...

Will do, Joe's suggestion is a good one.

> 
> >+}
> >+
> >  static void sh_eth_select_mii(struct net_device *ndev)
> >  {
> >  	u32 value = 0x0;
> [...]
> >@@ -1309,9 +1409,9 @@ static int sh_eth_rx(struct net_device *ndev, u32 intr_status, int *quota)
> >
> >  		/* In case of almost all GETHER/ETHERs, the Receive Frame State
> >  		 * (RFS) bits in the Receive Descriptor 0 are from bit 9 to
> >-		 * bit 0. However, in case of the R8A7740's GETHER, the RFS
> >-		 * bits are from bit 25 to bit 16. So, the driver needs right
> >-		 * shifting by 16.
> >+		 * bit 0. However, in case of the R8A7740, R8A779x and
> 
>    Small nit: comma needed before "and" as far as I know English grammar.

To be honest I don't think it is. But I'll add one for you.

> 
> >+		 * R7S72100 the RFS bits are from bit 25 to bit 16. So, the
> >+		 * driver needs right shifting by 16.
> >  		 */
> >  		if (mdp->cd->shift_rd0)
> >  			desc_status >>= 16;
> 
>    Other than that, this looks fine now, you can add my:
> 
> Acked-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>

Thanks.

^ permalink raw reply

* Re: [RFC] sysfs_rename_link() and its usage
From: Veaceslav Falico @ 2014-01-16  0:11 UTC (permalink / raw)
  To: Eric W. Biederman; +Cc: Tejun Heo, Greg KH, linux-kernel, netdev
In-Reply-To: <87fvoo25gj.fsf@xmission.com>

On Wed, Jan 15, 2014 at 03:25:16PM -0800, Eric W. Biederman wrote:
>Tejun Heo <tj@kernel.org> writes:
>
>> Hey, Veaceslav, Eric.

Hi Tejun, Eric,

>>
>> On Tue, Jan 14, 2014 at 05:35:23PM -0800, Eric W. Biederman wrote:
>>> >>> >>This works like a charm. However, if I want to use (obviously, with the
>>> >>> >>symlink present):
>>> >>> >>
>>> >>> >>sysfs_rename_link(&(a->dev.kobj), &(b->dev.kobj), oldname, newname);
>>> >>> >
>>> >>> >You forgot the namespace option to this call, what kernel version are
>>> >>> >you using here?
>>> >>>
>>> >>> It's git://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next ,
>>> >>> 3.13-rc6 with some networking patches on top of it.
>>
>> Does this work on 3.12?  How about Greg's driver-core-next?  Do you
>> have a minimal test case that I can use to reproduce the issue?

Sorry for the latency in responses, I'll update once I'll manage to test it
on those.

...snip...
>> Veaceslav, please confirm whether the issue is reproducible w/ v3.12.
>
>Anyway since a symlink living in a different namespace from it's target
>is just nonsense this (only compile tested) patch should fix the issue,
>and make sysfs_rename_link usable for people without a masters degree in
>sysfs again.

It's still there :-(. I've used your patch and added my small addition[1] to
test the sysfs_rename_link() (on top of net-next, 3.13-rc7), however the
issue is still there:

[   79.038340] net bond0: renaming to bondbla
[   79.038380] ------------[ cut here ]------------
[   79.038411] WARNING: CPU: 1 PID: 5318 at fs/sysfs/dir.c:618
sysfs_find_dirent+0x84/0x110()
[   79.038449] sysfs: ns invalid in 'bridge0' for 'lower_bond0'
...snip...
[   79.038877]  [<ffffffff810ae826>] warn_slowpath_fmt+0x46/0x50
[   79.038903]  [<ffffffff812ba890>] ? sysfs_get_dirent_ns+0x30/0x80
[   79.038930]  [<ffffffff812b97c4>] sysfs_find_dirent+0x84/0x110
[   79.038957]  [<ffffffff812ba89e>] sysfs_get_dirent_ns+0x3e/0x80
[   79.038983]  [<ffffffff812baf87>] sysfs_rename_link+0x57/0xe0
[   79.039030]  [<ffffffff81689e72>] netdev_adjacent_rename_links+0xa2/0x160

The current scheme (sysfs_remove_link() + sysfs_add_link()) works perfectly
well without any namespaces. I'll dig into it once I have some spare time,
it's not at all critical.

[1]: the patch (I've included your patch too, just in case):

diff --git a/drivers/base/core.c b/drivers/base/core.c
index 67b180d..0c9377a 100644
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -1825,9 +1825,8 @@ int device_rename(struct device *dev, const char *new_name)
  	}
  
  	if (dev->class) {
-		error = sysfs_rename_link_ns(&dev->class->p->subsys.kobj,
-					     kobj, old_device_name,
-					     new_name, kobject_namespace(kobj));
+		error = sysfs_rename_link(&dev->class->p->subsys.kobj,
+					  kobj, old_device_name, new_name);
  		if (error)
  			goto out;
  	}
diff --git a/fs/sysfs/symlink.c b/fs/sysfs/symlink.c
index 3ae3f1b..651444a 100644
--- a/fs/sysfs/symlink.c
+++ b/fs/sysfs/symlink.c
@@ -194,12 +194,10 @@ EXPORT_SYMBOL_GPL(sysfs_remove_link);
   *	@targ:	object we're pointing to.
   *	@old:	previous name of the symlink.
   *	@new:	new name of the symlink.
- *	@new_ns: new namespace of the symlink.
- *
   *	A helper function for the common rename symlink idiom.
   */
-int sysfs_rename_link_ns(struct kobject *kobj, struct kobject *targ,
-			 const char *old, const char *new, const void *new_ns)
+int sysfs_rename_link(struct kobject *kobj, struct kobject *targ,
+		      const char *old, const char *new)
  {
  	struct sysfs_dirent *parent_sd, *sd = NULL;
  	const void *old_ns = NULL;
@@ -224,13 +222,13 @@ int sysfs_rename_link_ns(struct kobject *kobj, struct kobject *targ,
  	if (sd->s_symlink.target_sd->s_dir.kobj != targ)
  		goto out;
  
-	result = sysfs_rename(sd, parent_sd, new, new_ns);
+	result = sysfs_rename(sd, parent_sd, new, kobject_namespace(targ));
  
  out:
  	sysfs_put(sd);
  	return result;
  }
-EXPORT_SYMBOL_GPL(sysfs_rename_link_ns);
+EXPORT_SYMBOL_GPL(sysfs_rename_link);
  
  static int sysfs_get_target_path(struct sysfs_dirent *parent_sd,
  				 struct sysfs_dirent *target_sd, char *path)
diff --git a/include/linux/sysfs.h b/include/linux/sysfs.h
index 6695040..093d992 100644
--- a/include/linux/sysfs.h
+++ b/include/linux/sysfs.h
@@ -213,9 +213,8 @@ int __must_check sysfs_create_link_nowarn(struct kobject *kobj,
  					  const char *name);
  void sysfs_remove_link(struct kobject *kobj, const char *name);
  
-int sysfs_rename_link_ns(struct kobject *kobj, struct kobject *target,
-			 const char *old_name, const char *new_name,
-			 const void *new_ns);
+int sysfs_rename_link(struct kobject *kobj, struct kobject *target,
+		      const char *old_name, const char *new_name);
  
  void sysfs_delete_link(struct kobject *dir, struct kobject *targ,
  			const char *name);
@@ -341,9 +340,8 @@ static inline void sysfs_remove_link(struct kobject *kobj, const char *name)
  {
  }
  
-static inline int sysfs_rename_link_ns(struct kobject *k, struct kobject *t,
-				       const char *old_name,
-				       const char *new_name, const void *ns)
+static inline int sysfs_rename_link(struct kobject *k, struct kobject *t,
+				    const char *old_name, const char *new_name)
  {
  	return 0;
  }
@@ -455,12 +453,6 @@ static inline void sysfs_remove_file(struct kobject *kobj,
  	return sysfs_remove_file_ns(kobj, attr, NULL);
  }
  
-static inline int sysfs_rename_link(struct kobject *kobj, struct kobject *target,
-				    const char *old_name, const char *new_name)
-{
-	return sysfs_rename_link_ns(kobj, target, old_name, new_name, NULL);
-}
-
  static inline struct sysfs_dirent *
  sysfs_get_dirent(struct sysfs_dirent *parent_sd, const unsigned char *name)
  {
diff --git a/net/core/dev.c b/net/core/dev.c
index 9957557..5d24d8e 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -5005,19 +5005,20 @@ EXPORT_SYMBOL(netdev_upper_dev_unlink);
  void netdev_adjacent_rename_links(struct net_device *dev, char *oldname)
  {
  	struct netdev_adjacent *iter;
+	char old_linkname[IFNAMSIZ+7], new_linkname[IFNAMSIZ+7];
  
  	list_for_each_entry(iter, &dev->adj_list.upper, list) {
-		netdev_adjacent_sysfs_del(iter->dev, oldname,
-					  &iter->dev->adj_list.lower);
-		netdev_adjacent_sysfs_add(iter->dev, dev,
-					  &iter->dev->adj_list.lower);
+		sprintf(old_linkname, "lower_%s", oldname);
+		sprintf(new_linkname, "lower_%s", dev->name);
+		sysfs_rename_link(&(iter->dev->dev.kobj), &(dev->dev.kobj),
+				  old_linkname, new_linkname);
  	}
  
  	list_for_each_entry(iter, &dev->adj_list.lower, list) {
-		netdev_adjacent_sysfs_del(iter->dev, oldname,
-					  &iter->dev->adj_list.upper);
-		netdev_adjacent_sysfs_add(iter->dev, dev,
-					  &iter->dev->adj_list.upper);
+		sprintf(old_linkname, "upper_%s", oldname);
+		sprintf(new_linkname, "upper_%s", dev->name);
+		sysfs_rename_link(&(iter->dev->dev.kobj), &(dev->dev.kobj),
+				  old_linkname, new_linkname);
  	}
  }
  

^ permalink raw reply related

* Re: [RFC PATCH net-next] etherdevice: Use ether_addr_copy to copy an Ethernet address
From: Joe Perches @ 2014-01-16  0:07 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, linux-arm-kernel, linux-arch
In-Reply-To: <20140115.153951.743060257410154565.davem@davemloft.net>

On Wed, 2014-01-15 at 15:39 -0800, David Miller wrote:
> From: Joe Perches <joe@perches.com>
> Date: Tue, 14 Jan 2014 15:18:47 -0800
> 
> > Some systems can use the normally known u16 alignment of
> > Ethernet addresses to save some code/text bytes and cycles.
> > 
> > This does not change currently emitted code on x86 by gcc 4.8.
> > 
> > Signed-off-by: Joe Perches <joe@perches.com>
> 
> This looks fine, in fact I'll apply it.

OK, good.

There are a couple thousand memcpy(foo, bar, ETH_ALEN)
in the kernel tree that could be converted.

Some will have a small performance improvement for
arm and powerpc.

If you want the ones for net-next net/ now (but not
for batman-adv, that maybe could use a new function like
ether_addr_copy_unaligned) here's a changestat.

Otherwise, I'll wait for the next cycle.

(done via coccinelle)

 net/8021q/vlan.c                          |  2 +-
 net/8021q/vlan_dev.c                      |  6 ++--
 net/appletalk/aarp.c                      | 10 +++---
 net/atm/lec.c                             |  9 ++---
 net/atm/mpc.c                             |  2 +-
 net/bluetooth/bnep/core.c                 | 19 +++++-----
 net/bluetooth/bnep/netdev.c               | 14 ++++----
 net/bridge/br_device.c                    |  4 +--
 net/bridge/br_fdb.c                       |  4 +--
 net/bridge/br_multicast.c                 |  4 +--
 net/bridge/br_netfilter.c                 |  2 +-
 net/bridge/br_stp_if.c                    | 10 +++---
 net/bridge/netfilter/ebt_among.c          |  2 +-
 net/bridge/netfilter/ebt_dnat.c           |  2 +-
 net/bridge/netfilter/ebt_redirect.c       |  6 ++--
 net/bridge/netfilter/ebt_snat.c           |  2 +-
 net/caif/caif_usb.c                       |  4 +--
 net/core/netpoll.c                        |  4 +--
 net/core/pktgen.c                         |  8 ++---
 net/decnet/dn_dev.c                       |  2 +-
 net/decnet/dn_neigh.c                     |  6 ++--
 net/decnet/dn_route.c                     |  6 ++--
 net/dsa/slave.c                           |  2 +-
 net/ethernet/eth.c                        | 18 +++++-----
 net/hsr/hsr_device.c                      |  8 ++---
 net/hsr/hsr_framereg.c                    | 21 +++++------
 net/hsr/hsr_main.c                        |  4 +--
 net/ipv4/netfilter/ipt_CLUSTERIP.c        |  2 +-
 net/mac80211/agg-rx.c                     | 10 +++---
 net/mac80211/agg-tx.c                     | 18 +++++-----
 net/mac80211/cfg.c                        | 34 +++++++++---------
 net/mac80211/debugfs_netdev.c             | 12 +++----
 net/mac80211/ht.c                         | 16 ++++-----
 net/mac80211/ibss.c                       | 14 ++++----
 net/mac80211/iface.c                      | 27 +++++++-------
 net/mac80211/mesh.c                       | 30 ++++++++--------
 net/mac80211/mesh_hwmp.c                  | 32 ++++++++---------
 net/mac80211/mesh_pathtbl.c               | 20 +++++------
 net/mac80211/mesh_plink.c                 |  6 ++--
 net/mac80211/mesh_ps.c                    |  2 +-
 net/mac80211/mlme.c                       | 32 ++++++++---------
 net/mac80211/rx.c                         | 14 ++++----
 net/mac80211/spectmgmt.c                  |  6 ++--
 net/mac80211/sta_info.c                   |  8 ++---
 net/mac80211/tx.c                         | 60 +++++++++++++++----------------
 net/mac80211/util.c                       | 22 ++++++------
 net/mac80211/wpa.c                        |  4 +--
 net/netfilter/ipset/ip_set_bitmap_ipmac.c |  6 ++--
 net/openvswitch/actions.c                 |  4 +--
 net/openvswitch/flow.c                    | 16 ++++-----
 net/openvswitch/flow_netlink.c            | 14 ++++----
 net/tipc/eth_media.c                      |  2 +-
 net/wireless/core.c                       |  2 +-
 net/wireless/ibss.c                       | 11 +++---
 net/wireless/lib80211_crypt_ccmp.c        |  4 +--
 net/wireless/lib80211_crypt_tkip.c        | 18 +++++-----
 net/wireless/mlme.c                       |  2 +-
 net/wireless/nl80211.c                    |  6 ++--
 net/wireless/scan.c                       |  6 ++--
 net/wireless/sme.c                        | 18 +++++-----
 net/wireless/util.c                       | 42 +++++++++++-----------
 net/wireless/wext-compat.c                |  8 ++---
 net/wireless/wext-sme.c                   |  5 +--
 net/wireless/wext-spy.c                   |  8 ++---
 64 files changed, 365 insertions(+), 357 deletions(-)

^ permalink raw reply

* Re: [PATCH net-next v4 8/9] xen-netback: Timeout packets in RX path
From: Wei Liu @ 2014-01-16  0:03 UTC (permalink / raw)
  To: Zoltan Kiss
  Cc: ian.campbell, wei.liu2, xen-devel, netdev, linux-kernel,
	jonathan.davies
In-Reply-To: <1389731995-9887-9-git-send-email-zoltan.kiss@citrix.com>

On Tue, Jan 14, 2014 at 08:39:54PM +0000, Zoltan Kiss wrote:
[...]
> diff --git a/drivers/net/xen-netback/common.h b/drivers/net/xen-netback/common.h
> index 109c29f..d1cd8ce 100644
> --- a/drivers/net/xen-netback/common.h
> +++ b/drivers/net/xen-netback/common.h
> @@ -129,6 +129,9 @@ struct xenvif {
>  	struct xen_netif_rx_back_ring rx;
>  	struct sk_buff_head rx_queue;
>  	RING_IDX rx_last_skb_slots;

Hmm... You seemed to mix your other patch with this series. :-)

> +	bool rx_queue_purge;
> +
> +	struct timer_list wake_queue;
>  
>  	/* This array is allocated seperately as it is large */
>  	struct gnttab_copy *grant_copy_op;
> @@ -225,4 +228,7 @@ void xenvif_idx_unmap(struct xenvif *vif, u16 pending_idx);
>  
>  extern bool separate_tx_rx_irq;
>  
[...]
> @@ -559,7 +579,7 @@ void xenvif_free(struct xenvif *vif)
>  		if (vif->grant_tx_handle[i] != NETBACK_INVALID_HANDLE) {
>  			unmap_timeout++;
>  			schedule_timeout(msecs_to_jiffies(1000));
> -			if (unmap_timeout > 9 &&
> +			if (unmap_timeout > ((rx_drain_timeout_msecs/1000) * DIV_ROUND_UP(XENVIF_QUEUE_LENGTH, (XEN_NETIF_RX_RING_SIZE / MAX_SKB_FRAGS))) &&

This line is really too long. And what's the rationale behind this long
expression?

Wei.

^ permalink raw reply

* Re: [PATCH net-next v4 6/9] xen-netback: Handle guests with too many frags
From: Wei Liu @ 2014-01-16  0:03 UTC (permalink / raw)
  To: Zoltan Kiss
  Cc: ian.campbell, wei.liu2, xen-devel, netdev, linux-kernel,
	jonathan.davies
In-Reply-To: <1389731995-9887-7-git-send-email-zoltan.kiss@citrix.com>

On Tue, Jan 14, 2014 at 08:39:52PM +0000, Zoltan Kiss wrote:
[...]
>  	/* Skip first skb fragment if it is on same page as header fragment. */
> @@ -832,6 +851,29 @@ static struct gnttab_map_grant_ref *xenvif_get_requests(struct xenvif *vif,
>  
>  	BUG_ON(shinfo->nr_frags > MAX_SKB_FRAGS);
>  
> +	if (frag_overflow) {
> +		struct sk_buff *nskb = xenvif_alloc_skb(0);
> +		if (unlikely(nskb == NULL)) {
> +			netdev_err(vif->dev,
> +				   "Can't allocate the frag_list skb.\n");

This, and other occurences of netdev_* logs need to be rate limit.
Otherwise you risk flooding kernel log when system is under memory
pressure.

> +			return NULL;
> +		}
> +
> +		shinfo = skb_shinfo(nskb);
> +		frags = shinfo->frags;
> +
> +		for (shinfo->nr_frags = 0; shinfo->nr_frags < frag_overflow;
> +		     shinfo->nr_frags++, txp++, gop++) {
> +			index = pending_index(vif->pending_cons++);
> +			pending_idx = vif->pending_ring[index];
> +			xenvif_tx_create_gop(vif, pending_idx, txp, gop);
> +			frag_set_pending_idx(&frags[shinfo->nr_frags],
> +					     pending_idx);
> +		}
> +
> +		skb_shinfo(skb)->frag_list = nskb;
> +	}
> +
>  	return gop;
>  }
>  
[...]
> @@ -1537,6 +1613,32 @@ static int xenvif_tx_submit(struct xenvif *vif)
>  				  pending_idx :
>  				  INVALID_PENDING_IDX);
>  
> +		if (skb_shinfo(skb)->frag_list) {
> +			nskb = skb_shinfo(skb)->frag_list;
> +			xenvif_fill_frags(vif, nskb, INVALID_PENDING_IDX);
> +			skb->len += nskb->len;
> +			skb->data_len += nskb->len;
> +			skb->truesize += nskb->truesize;
> +			skb_shinfo(skb)->tx_flags |= SKBTX_DEV_ZEROCOPY;
> +			skb_shinfo(nskb)->tx_flags |= SKBTX_DEV_ZEROCOPY;
> +			vif->tx_zerocopy_sent += 2;
> +			nskb = skb;
> +
> +			skb = skb_copy_expand(skb,
> +					      0,
> +					      0,
> +					      GFP_ATOMIC | __GFP_NOWARN);
> +			if (!skb) {
> +				netdev_dbg(vif->dev,
> +					   "Can't consolidate skb with too many fragments\n");

Rate limit.

> +				if (skb_shinfo(nskb)->destructor_arg)
> +					skb_shinfo(nskb)->tx_flags |=
> +						SKBTX_DEV_ZEROCOPY;

Why is this needed? nskb is the saved pointer to original skb, which has
already had SKBTX_DEV_ZEROCOPY in tx_flags. Did I miss something?

Wei.

> +				kfree_skb(nskb);
> +				continue;
> +			}
> +			skb_shinfo(skb)->destructor_arg = NULL;
> +		}
>  		if (skb_is_nonlinear(skb) && skb_headlen(skb) < PKT_PROT_LEN) {
>  			int target = min_t(int, skb->len, PKT_PROT_LEN);
>  			__pskb_pull_tail(skb, target - skb_headlen(skb));
> @@ -1590,6 +1692,9 @@ static int xenvif_tx_submit(struct xenvif *vif)
>  		}
>  
>  		netif_receive_skb(skb);
> +
> +		if (nskb)
> +			kfree_skb(nskb);
>  	}
>  
>  	return work_done;

^ permalink raw reply

* Re: [PATCH net-next v4 3/9] xen-netback: Remove old TX grant copy definitons and fix indentations
From: Wei Liu @ 2014-01-16  0:02 UTC (permalink / raw)
  To: Zoltan Kiss
  Cc: ian.campbell, wei.liu2, xen-devel, netdev, linux-kernel,
	jonathan.davies
In-Reply-To: <1389731995-9887-4-git-send-email-zoltan.kiss@citrix.com>

On Tue, Jan 14, 2014 at 08:39:49PM +0000, Zoltan Kiss wrote:
> These became obsolate with grant mapping. I've left intentionally the
               ^ obsolete
> indentations in this way, to improve readability of previous patches.
> 

^ permalink raw reply

* Re: [PATCH net-next v4 2/9] xen-netback: Change TX path from grant copy to mapping
From: Wei Liu @ 2014-01-16  0:01 UTC (permalink / raw)
  To: Zoltan Kiss
  Cc: ian.campbell, wei.liu2, xen-devel, netdev, linux-kernel,
	jonathan.davies
In-Reply-To: <1389731995-9887-3-git-send-email-zoltan.kiss@citrix.com>

On Tue, Jan 14, 2014 at 08:39:48PM +0000, Zoltan Kiss wrote:
> This patch changes the grant copy on the TX patch to grant mapping
> 
> v2:
> - delete branch for handling fragmented packets fit PKT_PROT_LEN sized first
>   request
> - mark the effect of using ballooned pages in a comment
> - place setting of skb_shinfo(skb)->tx_flags |= SKBTX_DEV_ZEROCOPY right
>   before netif_receive_skb, and mark the importance of it
> - grab dealloc_lock before __napi_complete to avoid contention with the
>   callback's napi_schedule
> - handle fragmented packets where first request < PKT_PROT_LEN
> - fix up error path when checksum_setup failed
> - check before teardown for pending grants, and start complain if they are
>   there after 10 second
> 
> v3:
> - delete a surplus checking from tx_action
> - remove stray line
> - squash xenvif_idx_unmap changes into the first patch
> - init spinlocks
> - call map hypercall directly instead of gnttab_map_refs()
> - fix unmapping timeout in xenvif_free()
> 
> v4:
> - fix indentations and comments
> - handle errors of set_phys_to_machine

There's no call to set_phys_to_machine in this patch. Did I miss
something?

> - go back to gnttab_map_refs instead of direct hypercall. Now we rely on the
>   modified API
> 
> Signed-off-by: Zoltan Kiss <zoltan.kiss@citrix.com>
> ---
>  drivers/net/xen-netback/interface.c |   60 +++++++-
>  drivers/net/xen-netback/netback.c   |  256 ++++++++++++++---------------------
>  2 files changed, 159 insertions(+), 157 deletions(-)
> 
> diff --git a/drivers/net/xen-netback/interface.c b/drivers/net/xen-netback/interface.c
> index a7855b3..1e0bf71 100644
> --- a/drivers/net/xen-netback/interface.c
> +++ b/drivers/net/xen-netback/interface.c
> @@ -123,7 +123,9 @@ static int xenvif_start_xmit(struct sk_buff *skb, struct net_device *dev)
>  	BUG_ON(skb->dev != dev);
>  
>  	/* Drop the packet if vif is not ready */
> -	if (vif->task == NULL || !xenvif_schedulable(vif))
> +	if (vif->task == NULL ||
> +	    vif->dealloc_task == NULL ||
> +	    !xenvif_schedulable(vif))
>  		goto drop;
>  
>  	/* At best we'll need one slot for the header and one for each
> @@ -345,8 +347,26 @@ struct xenvif *xenvif_alloc(struct device *parent, domid_t domid,

At the beginning of the function there's BUG_ON checks for vif->task. I
would suggest you do the same for vif->dealloc_task, just to be
consistent.

>  	vif->pending_prod = MAX_PENDING_REQS;
>  	for (i = 0; i < MAX_PENDING_REQS; i++)
>  		vif->pending_ring[i] = i;
> -	for (i = 0; i < MAX_PENDING_REQS; i++)
> -		vif->mmap_pages[i] = NULL;
> +	spin_lock_init(&vif->dealloc_lock);
> +	spin_lock_init(&vif->response_lock);
> +	/* If ballooning is disabled, this will consume real memory, so you
> +	 * better enable it. The long term solution would be to use just a
> +	 * bunch of valid page descriptors, without dependency on ballooning
> +	 */
> +	err = alloc_xenballooned_pages(MAX_PENDING_REQS,
> +				       vif->mmap_pages,
> +				       false);
> +	if (err) {
> +		netdev_err(dev, "Could not reserve mmap_pages\n");
> +		return NULL;
> +	}
> +	for (i = 0; i < MAX_PENDING_REQS; i++) {
> +		vif->pending_tx_info[i].callback_struct = (struct ubuf_info)
> +			{ .callback = xenvif_zerocopy_callback,
> +			  .ctx = NULL,
> +			  .desc = i };
> +		vif->grant_tx_handle[i] = NETBACK_INVALID_HANDLE;
> +	}
>  
>  	/*
>  	 * Initialise a dummy MAC address. We choose the numerically
> @@ -390,6 +410,7 @@ int xenvif_connect(struct xenvif *vif, unsigned long tx_ring_ref,
>  		goto err;
>  
>  	init_waitqueue_head(&vif->wq);
> +	init_waitqueue_head(&vif->dealloc_wq);
>  
>  	if (tx_evtchn == rx_evtchn) {
>  		/* feature-split-event-channels == 0 */
> @@ -431,6 +452,16 @@ int xenvif_connect(struct xenvif *vif, unsigned long tx_ring_ref,
>  		goto err_rx_unbind;
>  	}
>  
> +	vif->dealloc_task = kthread_create(xenvif_dealloc_kthread,
> +					   (void *)vif,
> +					   "%s-dealloc",
> +					   vif->dev->name);
> +	if (IS_ERR(vif->dealloc_task)) {
> +		pr_warn("Could not allocate kthread for %s\n", vif->dev->name);
> +		err = PTR_ERR(vif->dealloc_task);
> +		goto err_rx_unbind;
> +	}
> +
>  	vif->task = task;

Please move this line before the above hunk. Don't separate it from
corresponding kthread_create.

Last but not least, though I've looked at this patch for several rounds
and and the basic logic looks correct to me, I would like it to go
through XenRT tests if possible -- eye inspection is error-prone to such
complicated change. (If I'm not mistaken you once told me you've done
regression tests already. That would be neat!)

Wei.

^ permalink raw reply

* Re: [PATCH net-next v4 1/9] xen-netback: Introduce TX grant map definitions
From: Wei Liu @ 2014-01-16  0:00 UTC (permalink / raw)
  To: Zoltan Kiss
  Cc: ian.campbell, wei.liu2, xen-devel, netdev, linux-kernel,
	jonathan.davies
In-Reply-To: <1389731995-9887-2-git-send-email-zoltan.kiss@citrix.com>

There is a stray blank line change in xenvif_tx_create_gop. (I removed
that part too early and didn't bother to paste it back...)

On Tue, Jan 14, 2014 at 08:39:47PM +0000, Zoltan Kiss wrote:
[...]
> +void xenvif_idx_unmap(struct xenvif *vif, u16 pending_idx)
> +{
> +	int ret;
> +	struct gnttab_unmap_grant_ref tx_unmap_op;
> +
> +	if (vif->grant_tx_handle[pending_idx] == NETBACK_INVALID_HANDLE) {
> +		netdev_err(vif->dev,
> +			   "Trying to unmap invalid handle! pending_idx: %x\n",
> +			   pending_idx);
> +		return;
> +	}
> +	gnttab_set_unmap_op(&tx_unmap_op,
> +			    idx_to_kaddr(vif, pending_idx),
> +			    GNTMAP_host_map,
> +			    vif->grant_tx_handle[pending_idx]);
> +	ret = gnttab_unmap_refs(&tx_unmap_op,
> +				&vif->mmap_pages[pending_idx],
> +				1);
> +
> +	ret = HYPERVISOR_grant_table_op(GNTTABOP_unmap_grant_ref,
> +					&tx_unmap_op,
> +					1);

As you said in your other email, this should be removed. :-)

> +	BUG_ON(ret);
> +	vif->grant_tx_handle[pending_idx] = NETBACK_INVALID_HANDLE;
> +}
>  
>  static void make_tx_response(struct xenvif *vif,
>  			     struct xen_netif_tx_request *txp,
> @@ -1738,6 +1879,14 @@ static inline int tx_work_todo(struct xenvif *vif)
>  	return 0;
>  }
>  
> +static inline bool tx_dealloc_work_todo(struct xenvif *vif)
> +{
> +	if (vif->dealloc_cons != vif->dealloc_prod)
> +		return true;
> +
> +	return false;

This can be simplified as
  return vif->dealloc_cons != vif->dealloc_prod;

Wei.

^ permalink raw reply

* Re: [PATCH net-next v3] ipv6: move IPV6_TCLASS_SHIFT into ipv6.h and define a helper
From: David Miller @ 2014-01-15 23:54 UTC (permalink / raw)
  To: roy.qing.li; +Cc: netdev
In-Reply-To: <1389776610-19703-1-git-send-email-roy.qing.li@gmail.com>

From: roy.qing.li@gmail.com
Date: Wed, 15 Jan 2014 17:03:30 +0800

> From: Li RongQing <roy.qing.li@gmail.com>
> 
> Two places defined IPV6_TCLASS_SHIFT, so we should move it into ipv6.h,
> and use this macro as possible. And define ip6_tclass helper to return
> tclass
> 
> Signed-off-by: Li RongQing <roy.qing.li@gmail.com>

Applied, thank you.

^ permalink raw reply

* Re: [PATCH net-next 00/11] be2net: patch set
From: David Miller @ 2014-01-15 23:52 UTC (permalink / raw)
  To: sathya.perla; +Cc: netdev
In-Reply-To: <1389772421-24925-1-git-send-email-sathya.perla@emulex.com>

From: Sathya Perla <sathya.perla@emulex.com>
Date: Wed, 15 Jan 2014 13:23:30 +0530

> The following patch set is best suited for net-next as it
> contains code-cleanup, support for newer versions of FW cmds and
> a few minor fixes. Please apply. Thanks!

Series applied, thanks.

^ permalink raw reply

* Re: throughput problems with realtek
From: Francois Romieu @ 2014-01-15 23:51 UTC (permalink / raw)
  To: Dmitry Kasatkin; +Cc: Rick Jones, nic_swsd, netdev, l.moiseichuk
In-Reply-To: <CACE9dm92Wu=XwCChXJjjsz=FEWAgJW-1iLRw5-d4c5ZTCYVe3w@mail.gmail.com>

Dmitry Kasatkin <dmitry.kasatkin@gmail.com> :
[...]
> I do not see any link speed changes... it stays the same...
> The same problem is visible on absolutely different computers.

No netdev watchdog message either ?

A serving 8168c does not seem to fluctuate (3.12.5, Intel 82578 client).
My hardware is a bit old though. Please send XID message from the r8169
driver. It should be seen in dmesg.

Thanks.

-- 
Ueimor

^ permalink raw reply

* [Patch net-next] net_sched: act: remove capab from struct tc_action_ops
From: Cong Wang @ 2014-01-15 23:49 UTC (permalink / raw)
  To: netdev; +Cc: Cong Wang, Jamal Hadi Salim, David S. Miller

It is not actually implemented.

Cc: Jamal Hadi Salim <jhs@mojatatu.com>
Cc: David S. Miller <davem@davemloft.net>
Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
---
 include/net/act_api.h   | 2 --
 net/sched/act_csum.c    | 1 -
 net/sched/act_gact.c    | 1 -
 net/sched/act_ipt.c     | 2 --
 net/sched/act_mirred.c  | 1 -
 net/sched/act_nat.c     | 1 -
 net/sched/act_pedit.c   | 1 -
 net/sched/act_police.c  | 1 -
 net/sched/act_simple.c  | 1 -
 net/sched/act_skbedit.c | 1 -
 10 files changed, 12 deletions(-)

diff --git a/include/net/act_api.h b/include/net/act_api.h
index e171387..8ed9746 100644
--- a/include/net/act_api.h
+++ b/include/net/act_api.h
@@ -81,13 +81,11 @@ struct tc_action {
 	struct list_head	list;
 };
 
-#define TCA_CAP_NONE 0
 struct tc_action_ops {
 	struct list_head head;
 	struct tcf_hashinfo *hinfo;
 	char    kind[IFNAMSIZ];
 	__u32   type; /* TBD to match kind */
-	__u32 	capab;  /* capabilities includes 4 bit version */
 	struct module		*owner;
 	int     (*act)(struct sk_buff *, const struct tc_action *, struct tcf_result *);
 	int     (*dump)(struct sk_buff *, struct tc_action *, int, int);
diff --git a/net/sched/act_csum.c b/net/sched/act_csum.c
index ee28e1c..9d5c1d3 100644
--- a/net/sched/act_csum.c
+++ b/net/sched/act_csum.c
@@ -572,7 +572,6 @@ static struct tc_action_ops act_csum_ops = {
 	.kind		= "csum",
 	.hinfo		= &csum_hash_info,
 	.type		= TCA_ACT_CSUM,
-	.capab		= TCA_CAP_NONE,
 	.owner		= THIS_MODULE,
 	.act		= tcf_csum,
 	.dump		= tcf_csum_dump,
diff --git a/net/sched/act_gact.c b/net/sched/act_gact.c
index 3133307..72c49de 100644
--- a/net/sched/act_gact.c
+++ b/net/sched/act_gact.c
@@ -194,7 +194,6 @@ static struct tc_action_ops act_gact_ops = {
 	.kind		=	"gact",
 	.hinfo		=	&gact_hash_info,
 	.type		=	TCA_ACT_GACT,
-	.capab		=	TCA_CAP_NONE,
 	.owner		=	THIS_MODULE,
 	.act		=	tcf_gact,
 	.dump		=	tcf_gact_dump,
diff --git a/net/sched/act_ipt.c b/net/sched/act_ipt.c
index bc9f498..67d701e 100644
--- a/net/sched/act_ipt.c
+++ b/net/sched/act_ipt.c
@@ -287,7 +287,6 @@ static struct tc_action_ops act_ipt_ops = {
 	.kind		=	"ipt",
 	.hinfo		=	&ipt_hash_info,
 	.type		=	TCA_ACT_IPT,
-	.capab		=	TCA_CAP_NONE,
 	.owner		=	THIS_MODULE,
 	.act		=	tcf_ipt,
 	.dump		=	tcf_ipt_dump,
@@ -299,7 +298,6 @@ static struct tc_action_ops act_xt_ops = {
 	.kind		=	"xt",
 	.hinfo		=	&ipt_hash_info,
 	.type		=	TCA_ACT_XT,
-	.capab		=	TCA_CAP_NONE,
 	.owner		=	THIS_MODULE,
 	.act		=	tcf_ipt,
 	.dump		=	tcf_ipt_dump,
diff --git a/net/sched/act_mirred.c b/net/sched/act_mirred.c
index 5d05b57..376234e 100644
--- a/net/sched/act_mirred.c
+++ b/net/sched/act_mirred.c
@@ -257,7 +257,6 @@ static struct tc_action_ops act_mirred_ops = {
 	.kind		=	"mirred",
 	.hinfo		=	&mirred_hash_info,
 	.type		=	TCA_ACT_MIRRED,
-	.capab		=	TCA_CAP_NONE,
 	.owner		=	THIS_MODULE,
 	.act		=	tcf_mirred,
 	.dump		=	tcf_mirred_dump,
diff --git a/net/sched/act_nat.c b/net/sched/act_nat.c
index a49fa23..46e1aa3 100644
--- a/net/sched/act_nat.c
+++ b/net/sched/act_nat.c
@@ -296,7 +296,6 @@ static struct tc_action_ops act_nat_ops = {
 	.kind		=	"nat",
 	.hinfo		=	&nat_hash_info,
 	.type		=	TCA_ACT_NAT,
-	.capab		=	TCA_CAP_NONE,
 	.owner		=	THIS_MODULE,
 	.act		=	tcf_nat,
 	.dump		=	tcf_nat_dump,
diff --git a/net/sched/act_pedit.c b/net/sched/act_pedit.c
index f361e4e..109265d 100644
--- a/net/sched/act_pedit.c
+++ b/net/sched/act_pedit.c
@@ -233,7 +233,6 @@ static struct tc_action_ops act_pedit_ops = {
 	.kind		=	"pedit",
 	.hinfo		=	&pedit_hash_info,
 	.type		=	TCA_ACT_PEDIT,
-	.capab		=	TCA_CAP_NONE,
 	.owner		=	THIS_MODULE,
 	.act		=	tcf_pedit,
 	.dump		=	tcf_pedit_dump,
diff --git a/net/sched/act_police.c b/net/sched/act_police.c
index a719fdf..7fd0993 100644
--- a/net/sched/act_police.c
+++ b/net/sched/act_police.c
@@ -385,7 +385,6 @@ static struct tc_action_ops act_police_ops = {
 	.kind		=	"police",
 	.hinfo		=	&police_hash_info,
 	.type		=	TCA_ID_POLICE,
-	.capab		=	TCA_CAP_NONE,
 	.owner		=	THIS_MODULE,
 	.act		=	tcf_act_police,
 	.dump		=	tcf_act_police_dump,
diff --git a/net/sched/act_simple.c b/net/sched/act_simple.c
index f7d5406..92236da 100644
--- a/net/sched/act_simple.c
+++ b/net/sched/act_simple.c
@@ -190,7 +190,6 @@ static struct tc_action_ops act_simp_ops = {
 	.kind		=	"simple",
 	.hinfo		=	&simp_hash_info,
 	.type		=	TCA_ACT_SIMP,
-	.capab		=	TCA_CAP_NONE,
 	.owner		=	THIS_MODULE,
 	.act		=	tcf_simp,
 	.dump		=	tcf_simp_dump,
diff --git a/net/sched/act_skbedit.c b/net/sched/act_skbedit.c
index 74af461..c36b520 100644
--- a/net/sched/act_skbedit.c
+++ b/net/sched/act_skbedit.c
@@ -189,7 +189,6 @@ static struct tc_action_ops act_skbedit_ops = {
 	.kind		=	"skbedit",
 	.hinfo		=	&skbedit_hash_info,
 	.type		=	TCA_ACT_SKBEDIT,
-	.capab		=	TCA_CAP_NONE,
 	.owner		=	THIS_MODULE,
 	.act		=	tcf_skbedit,
 	.dump		=	tcf_skbedit_dump,
-- 
1.8.3.1

^ permalink raw reply related

* Re: [BUG] eth_type_trans() can access stale data
From: David Miller @ 2014-01-15 23:48 UTC (permalink / raw)
  To: eric.dumazet; +Cc: netdev
In-Reply-To: <1389743541.31367.279.camel@edumazet-glaptop2.roam.corp.google.com>

From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Tue, 14 Jan 2014 15:52:21 -0800

> @@ -194,7 +194,8 @@ __be16 eth_type_trans(struct sk_buff *skb, struct net_device *dev)
>  	 *      layer. We look for FFFF which isn't a used 802.2 SSAP/DSAP. This
>  	 *      won't work for fault tolerant netware but does for the rest.
>  	 */
> -	if (unlikely(skb->len >= 2 && *(unsigned short *)(skb->data) == 0xFFFF))
> +	if (unlikely(skb_headlen(skb) >= 2 &&
> +		     *(unsigned short *)(skb->data) == 0xFFFF))
>  		return htons(ETH_P_802_3);

I think using skb_header_pointer() will essentially have the same cost,
why not use it instead?

Thanks.

^ permalink raw reply

* Re: [PATCH net-next 0/2] net: mvneta: simple cleanups
From: David Miller @ 2014-01-15 23:41 UTC (permalink / raw)
  To: arno; +Cc: w, thomas.petazzoni, netdev, linux-arm-kernel
In-Reply-To: <cover.1389742334.git.arno@natisbad.org>

From: Arnaud Ebalard <arno@natisbad.org>
Date: Wed, 15 Jan 2014 00:45:31 +0100

> 
> Those two patches are intended for net-next. They apply on top of
> performance improvements patches from Willy for mvneta driver.
> They provide some simple cleanups for unused variables, function
> params or return values.
> 
> Arnaud Ebalard (2):
>   net: mvneta: mvneta_tx_done_gbe() cleanups
>   net: mvneta: make mvneta_txq_done() return void

These patches do not apply to net-next, please respin.

Thanks.

^ 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