Netdev List
 help / color / mirror / Atom feed
* Re: Debugging Ethernet issues
From: Florian Fainelli @ 2016-11-14 19:19 UTC (permalink / raw)
  To: Måns Rullgård
  Cc: Sebastian Frias, Mason, Andrew Lunn, netdev, Sergei Shtylyov,
	Tom Lendacky, Zach Brown, Shaohui Xie, Tim Beale, Brian Hill,
	Vince Bridgers, Balakumaran Kannan, David S. Miller,
	Kirill Kapranov
In-Reply-To: <yw1xpolxga3o.fsf@unicorn.mansr.com>

On 11/14/2016 11:00 AM, Måns Rullgård wrote:
> Florian Fainelli <f.fainelli@gmail.com> writes:
> 
>> On 11/14/2016 10:20 AM, Florian Fainelli wrote:
>>> On 11/14/2016 09:59 AM, Sebastian Frias wrote:
>>>> On 11/14/2016 06:32 PM, Florian Fainelli wrote:
>>>>> On 11/14/2016 07:33 AM, Mason wrote:
>>>>>> On 14/11/2016 15:58, Mason wrote:
>>>>>>
>>>>>>> nb8800 26000.ethernet eth0: Link is Up - 100Mbps/Full - flow control rx/tx
>>>>>>> vs
>>>>>>> nb8800 26000.ethernet eth0: Link is Up - 100Mbps/Full - flow control off
>>>>>>>
>>>>>>> I'm not sure whether "flow control" is relevant...
>>>>>>
>>>>>> Based on phy_print_status()
>>>>>> phydev->pause ? "rx/tx" : "off"
>>>>>> I added the following patch.
>>>>>>
>>>>>> diff --git a/drivers/net/ethernet/aurora/nb8800.c b/drivers/net/ethernet/aurora/nb8800.c
>>>>>> index defc22a15f67..4e758c1cfa4e 100644
>>>>>> --- a/drivers/net/ethernet/aurora/nb8800.c
>>>>>> +++ b/drivers/net/ethernet/aurora/nb8800.c
>>>>>> @@ -667,6 +667,8 @@ static void nb8800_link_reconfigure(struct net_device *dev)
>>>>>>         struct phy_device *phydev = priv->phydev;
>>>>>>         int change = 0;
>>>>>>  
>>>>>> +       printk("%s from %pf\n", __func__, __builtin_return_address(0));
>>>>>> +
>>>>>>         if (phydev->link) {
>>>>>>                 if (phydev->speed != priv->speed) {
>>>>>>                         priv->speed = phydev->speed;
>>>>>> @@ -1274,9 +1276,9 @@ static int nb8800_hw_init(struct net_device *dev)
>>>>>>         nb8800_writeb(priv, NB8800_PQ2, val & 0xff);
>>>>>>  
>>>>>>         /* Auto-negotiate by default */
>>>>>> -       priv->pause_aneg = true;
>>>>>> -       priv->pause_rx = true;
>>>>>> -       priv->pause_tx = true;
>>>>>> +       priv->pause_aneg = false;
>>>>>> +       priv->pause_rx = false;
>>>>>> +       priv->pause_tx = false;
>>>>>>  
>>>>>>         nb8800_mc_init(dev, 0);
>>>>>>  
>>>>>>
> 
> [...]
> 
>>>>> And the time difference is clearly accounted for auto-negotiation time
>>>>> here, as you can see it takes about 3 seconds for Gigabit Ethernet to
>>>>> auto-negotiate and that seems completely acceptable and normal to me
>>>>> since it is a more involved process than lower speeds.
>>>>>
>>>>>>
>>>>>>
>>>>>> OK, so now it works (by accident?) even on 100 Mbps switch, but it still
>>>>>> prints "flow control rx/tx"...
>>>>>
>>>>> Because your link partner advertises flow control, and that's what
>>>>> phydev->pause and phydev->asym_pause report (I know it's confusing, but
>>>>> that's what it is at the moment).
>>>>
>>>> Thanks.
>>>> Could you confirm that Mason's patch is correct and/or that it does not
>>>> has negative side-effects?
>>>
>>> The patch is not correct nor incorrect per-se, it changes the default
>>> policy of having pause frames advertised by default to not having them
>>> advertised by default.
> 
> I was advised to advertise flow control by default back when I was
> working on the driver, and I think it makes sense to do so.
> 
>>> This influences both your Ethernet MAC and the link partner in that
>>> the result is either flow control is enabled (before) or it is not
>>> (with the patch). There must be something amiss if you see packet
>>> loss or some kind of problem like that with an early exchange such as
>>> DHCP. Flow control tend to kick in under higher packet rates (at
>>> least, that's what you expect).
>>>
>>>>
>>>> Right now we know that Mason's patch makes this work, but we do not
>>>> understand why nor its implications.
>>>
>>> You need to understand why, right now, the way this problem is
>>> presented, you came up with a workaround, not with the root cause or the
>>> solution. What does your link partner (switch?) reports, that is, what
>>> is the ethtool output when you have a link up from  your nb8800 adapter?
>>
>> Actually, nb8800_pause_config() seems to be doing a complete MAC/DMA
>> reconfiguration when pause frames get auto-negotiated while the link is
>> UP,
> 
> This is due to a silly hardware limitation.  The register containing the
> flow control bits can't be written while rx is enabled.

You do a DMA stop, but you don't disable the MAC receiver unlike what
nb8800_stop() does, why is not calling nb8800_mac_rx() necessary here?

> 
>> and it does not differentiate being called from
>> ethtool::set_pauseparam or the PHYLIB adjust_link callback (which it
>> probably should),
> 
> Differentiate how?

Differentiate in that when you are called from adjust_link, why bother
checking with netif_running() since you are only configuring the pause
settings when phydev->link is set. Not that this matters much, but
that's something the caller can tell you.

> 
>> wondering if there is a not a remote chance you can get the reply to
>> arrive right when you just got signaled a link UP?
> 
> If you're attempting to send or receive things before you get the link
> up notification, you shouldn't expect anything to work reliably.

No kidding.
-- 
Florian

^ permalink raw reply

* Haster Response.//..
From: UNCLAIMED ASSET @ 2016-11-14 19:15 UTC (permalink / raw)
  To: netdev

Mr. Steve Bhatti,
Drift / regionsjef
Santander Bank Plc,
47-48 Piccadilly
PICCADILLY
W1J0DT
London, Storbritannia
 
Hei,

Jeg er Steve Bhatti, fra Harlesden North West London, leder for regnskap Revisjonen og formell senior programmerer sjef hos Deutsche bank, her i England (Santander Bank Plc). Jeg har også jobbet for Nyeste ministrene Bank Plc.
 
Jeg trenger din haster hjelp til å overføre summen av (£ 21,5) millioner britiske pund på kontoen din innen 11 eller 15 bankdager. Disse pengene har vært sovende i mange år i vår Bank uten krav. Jeg ønsker banken å frigjøre penger til deg som nærmeste person til vår avdøde kunden (eieren av kontoen) som dessverre mistet livet februar 2003 gjennom det sørlige USA romfergen Columbia, døde han en enkelt mann. Jeg ønsker ikke penger til å gå inn i bankens egne konto som en forlatt fondet. Så dette er grunnen til at jeg kontakter deg slik at banken kan frigjøre penger til deg som skal pårørende til den avdøde kunden. (Late David McDowell Brown) en amerikansk statsborger av Arlington Virginia.
Vennligst Jeg vil at du skal holde Forslaget som en topp hemmelig og slette den hvis du ikke er interessert.
 
Her deler ratio; 50% til meg og 40% til deg mens 10% er for eventuelle utgifter i løpet av transaksjonen. Hvis du er interessert, ta kontakt med meg umiddelbart via min private / direkte post: steve.s.bhatti@gmail.com

Gi meg følgende under, som vi har 7 dager for å kjøre den gjennom. Dette er veldig HASTER.
 
1. Fullt navn:
2. Direkte Mobilnummer:
3. Kontakt Adresse:
4. Yrke:
5. Alder:
6. Kjønn:
7. Nasjonalitet:
 
Vennlig hilsen,
Dr. Steve Bhatti.

^ permalink raw reply

* Re: AF_VSOCK loopback
From: Stefan Hajnoczi @ 2016-11-14 19:14 UTC (permalink / raw)
  To: Jorgen S. Hansen; +Cc: cavery@redhat.com, netdev@vger.kernel.org
In-Reply-To: <BY2PR0501MB20561F98987156779668ECB1DABB0@BY2PR0501MB2056.namprd05.prod.outlook.com>

[-- Attachment #1: Type: text/plain, Size: 749 bytes --]

On Fri, Nov 11, 2016 at 02:14:44PM +0000, Jorgen S. Hansen wrote:
> Hi Stefan,
> 
> All datagram communication in VMCI based AF_VSOCK is going through the host - also for loopback communication. The only difference wrt loopback is that the VMCI queue pairs implementing the shared queues for the stream protocols aren't registered with the hypervisor - they are created specifying the VMCI_QPFLAG_LOCAL flag, and exist only as local guest memory.
> 
> So in the current form, there isn't much loopback code in the vmci AF_VSOCK implementation, so it doesn't seem like there would be much to share either.

Thanks for clarifying.  I'm playing with a virtio-vsock implementation
of loopback and expect send patches later this week.

Stefan

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]

^ permalink raw reply

* Re: [PATCH net 2/3] bpf, mlx5: fix various refcount/prog issues in mlx5e_xdp_set
From: Daniel Borkmann @ 2016-11-14 19:05 UTC (permalink / raw)
  To: Saeed Mahameed, davem
  Cc: alexei.starovoitov, bblanco, tariqt, zhiyisun, ranas, netdev
In-Reply-To: <af02aa53-0df6-e8b7-bcfe-5f765f5a8c09@mellanox.com>

On 11/14/2016 07:27 PM, Saeed Mahameed wrote:
> On 11/14/2016 02:43 AM, Daniel Borkmann wrote:
>> There are multiple issues in mlx5e_xdp_set():
>>
>> 1) prog can be NULL, so calling unconditionally into bpf_prog_add(prog,
>>     priv->params.num_channels) can end badly.
>
> not correct, if prog is null we will never get to bpf_prog_add:
>
>          reset = (!priv->xdp_prog || !prog);
>          [...]
> 	if (!test_bit(MLX5E_STATE_OPENED, &priv->state) || reset)
> 		goto unlock;
>          bpf_prog_add...

Yep, I noticed already, dropped this from the changelog for net-next
rebase anyway.

>> 2) The batched bpf_prog_add() should be done at an earlier point in
>>     time. This makes sure that we cannot fail anymore at the time we
>>     want to set the program for each channel. This only means that we
>>     have to undo the bpf_prog_add() in case we return early due to
>>     reset or device not in MLX5E_STATE_OPENED yet. Note, err is 0 here.
>
> It is delayed for a reason, we do delayed batched bpf_prog_add()
> only when reset is not required (exchanging prog/old_prg) when both prog and old_prog are not null,
> which means the only thing that could fail in this case is bpf_prog_add.

Right, plus 3) below.

> so i don't see any reason for changing the logic, checking for  bpf_prog_add return value would be sufficient.

Yes, but if that fails, it would be better to check early for bailing out
since at this point in time undoing logic becomes unnecessary ugly.

> Sorry I need to go for now, I will continue reviewing this patch later.  but this patch looks a little bit exaggerated.
>
>> 3) When swapping the priv->xdp_prog, then no extra reference count must
>>     be taken since we got that from call path via dev_change_xdp_fd()
>>     already. Otherwise, we'd never be able to free the program. Also,
>>     bpf_prog_add() without checking the return code could fail.
>>
>> Fixes: 86994156c736 ("net/mlx5e: XDP fast RX drop bpf programs support")
>> Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>

^ permalink raw reply

* Re: Debugging Ethernet issues
From: Måns Rullgård @ 2016-11-14 19:00 UTC (permalink / raw)
  To: Florian Fainelli
  Cc: Sebastian Frias, Mason, Andrew Lunn, netdev, Sergei Shtylyov,
	Tom Lendacky, Zach Brown, Shaohui Xie, Tim Beale, Brian Hill,
	Vince Bridgers, Balakumaran Kannan, David S. Miller,
	Kirill Kapranov
In-Reply-To: <4d4f8bca-e084-2f49-1db8-56674debaf9c@gmail.com>

Florian Fainelli <f.fainelli@gmail.com> writes:

> On 11/14/2016 10:20 AM, Florian Fainelli wrote:
>> On 11/14/2016 09:59 AM, Sebastian Frias wrote:
>>> On 11/14/2016 06:32 PM, Florian Fainelli wrote:
>>>> On 11/14/2016 07:33 AM, Mason wrote:
>>>>> On 14/11/2016 15:58, Mason wrote:
>>>>>
>>>>>> nb8800 26000.ethernet eth0: Link is Up - 100Mbps/Full - flow control rx/tx
>>>>>> vs
>>>>>> nb8800 26000.ethernet eth0: Link is Up - 100Mbps/Full - flow control off
>>>>>>
>>>>>> I'm not sure whether "flow control" is relevant...
>>>>>
>>>>> Based on phy_print_status()
>>>>> phydev->pause ? "rx/tx" : "off"
>>>>> I added the following patch.
>>>>>
>>>>> diff --git a/drivers/net/ethernet/aurora/nb8800.c b/drivers/net/ethernet/aurora/nb8800.c
>>>>> index defc22a15f67..4e758c1cfa4e 100644
>>>>> --- a/drivers/net/ethernet/aurora/nb8800.c
>>>>> +++ b/drivers/net/ethernet/aurora/nb8800.c
>>>>> @@ -667,6 +667,8 @@ static void nb8800_link_reconfigure(struct net_device *dev)
>>>>>         struct phy_device *phydev = priv->phydev;
>>>>>         int change = 0;
>>>>>  
>>>>> +       printk("%s from %pf\n", __func__, __builtin_return_address(0));
>>>>> +
>>>>>         if (phydev->link) {
>>>>>                 if (phydev->speed != priv->speed) {
>>>>>                         priv->speed = phydev->speed;
>>>>> @@ -1274,9 +1276,9 @@ static int nb8800_hw_init(struct net_device *dev)
>>>>>         nb8800_writeb(priv, NB8800_PQ2, val & 0xff);
>>>>>  
>>>>>         /* Auto-negotiate by default */
>>>>> -       priv->pause_aneg = true;
>>>>> -       priv->pause_rx = true;
>>>>> -       priv->pause_tx = true;
>>>>> +       priv->pause_aneg = false;
>>>>> +       priv->pause_rx = false;
>>>>> +       priv->pause_tx = false;
>>>>>  
>>>>>         nb8800_mc_init(dev, 0);
>>>>>  
>>>>>

[...]

>>>> And the time difference is clearly accounted for auto-negotiation time
>>>> here, as you can see it takes about 3 seconds for Gigabit Ethernet to
>>>> auto-negotiate and that seems completely acceptable and normal to me
>>>> since it is a more involved process than lower speeds.
>>>>
>>>>>
>>>>>
>>>>> OK, so now it works (by accident?) even on 100 Mbps switch, but it still
>>>>> prints "flow control rx/tx"...
>>>>
>>>> Because your link partner advertises flow control, and that's what
>>>> phydev->pause and phydev->asym_pause report (I know it's confusing, but
>>>> that's what it is at the moment).
>>>
>>> Thanks.
>>> Could you confirm that Mason's patch is correct and/or that it does not
>>> has negative side-effects?
>> 
>> The patch is not correct nor incorrect per-se, it changes the default
>> policy of having pause frames advertised by default to not having them
>> advertised by default.

I was advised to advertise flow control by default back when I was
working on the driver, and I think it makes sense to do so.

>> This influences both your Ethernet MAC and the link partner in that
>> the result is either flow control is enabled (before) or it is not
>> (with the patch). There must be something amiss if you see packet
>> loss or some kind of problem like that with an early exchange such as
>> DHCP. Flow control tend to kick in under higher packet rates (at
>> least, that's what you expect).
>> 
>>>
>>> Right now we know that Mason's patch makes this work, but we do not
>>> understand why nor its implications.
>> 
>> You need to understand why, right now, the way this problem is
>> presented, you came up with a workaround, not with the root cause or the
>> solution. What does your link partner (switch?) reports, that is, what
>> is the ethtool output when you have a link up from  your nb8800 adapter?
>
> Actually, nb8800_pause_config() seems to be doing a complete MAC/DMA
> reconfiguration when pause frames get auto-negotiated while the link is
> UP,

This is due to a silly hardware limitation.  The register containing the
flow control bits can't be written while rx is enabled.

> and it does not differentiate being called from
> ethtool::set_pauseparam or the PHYLIB adjust_link callback (which it
> probably should),

Differentiate how?

> wondering if there is a not a remote chance you can get the reply to
> arrive right when you just got signaled a link UP?

If you're attempting to send or receive things before you get the link
up notification, you shouldn't expect anything to work reliably.

-- 
Måns Rullgård

^ permalink raw reply

* [PATCH 1/3] net: ethernet: stmmac: change dma descriptors to __le32
From: Michael Weiser @ 2016-11-14 17:58 UTC (permalink / raw)
  To: netdev; +Cc: Michael Weiser, Giuseppe Cavallaro, Alexandre Torgue
In-Reply-To: <20161114175807.4747-1-michael.weiser@gmx.de>

The stmmac driver does not take into account the processor may be big
endian when writing the DMA descriptors. This causes the ethernet
interface not to be initialised correctly when running a big-endian
kernel. Change the descriptors for DMA to use __le32 and ensure they are
suitably swapped before writing. Tested successfully on the
Cubieboard2.

Signed-off-by: Michael Weiser <michael.weiser@gmx.de>
Cc: Giuseppe Cavallaro <peppe.cavallaro@st.com>
Cc: Alexandre Torgue <alexandre.torgue@st.com>
Cc: netdev@vger.kernel.org
---
 drivers/net/ethernet/stmicro/stmmac/chain_mode.c   | 55 ++++++++++----------
 drivers/net/ethernet/stmicro/stmmac/descs.h        | 20 ++++----
 drivers/net/ethernet/stmicro/stmmac/descs_com.h    | 48 +++++++++--------
 drivers/net/ethernet/stmicro/stmmac/dwmac4_descs.c | 60 +++++++++++-----------
 drivers/net/ethernet/stmicro/stmmac/enh_desc.c     | 55 ++++++++++----------
 drivers/net/ethernet/stmicro/stmmac/norm_desc.c    | 48 ++++++++---------
 drivers/net/ethernet/stmicro/stmmac/ring_mode.c    | 39 +++++++-------
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c  | 51 +++++++++---------
 8 files changed, 195 insertions(+), 181 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/chain_mode.c b/drivers/net/ethernet/stmicro/stmmac/chain_mode.c
index b3e669a..026e8e9 100644
--- a/drivers/net/ethernet/stmicro/stmmac/chain_mode.c
+++ b/drivers/net/ethernet/stmicro/stmmac/chain_mode.c
@@ -34,7 +34,7 @@ static int stmmac_jumbo_frm(void *p, struct sk_buff *skb, int csum)
 	unsigned int entry = priv->cur_tx;
 	struct dma_desc *desc = priv->dma_tx + entry;
 	unsigned int nopaged_len = skb_headlen(skb);
-	unsigned int bmax;
+	unsigned int bmax, des2;
 	unsigned int i = 1, len;
 
 	if (priv->plat->enh_desc)
@@ -44,11 +44,12 @@ static int stmmac_jumbo_frm(void *p, struct sk_buff *skb, int csum)
 
 	len = nopaged_len - bmax;
 
-	desc->des2 = dma_map_single(priv->device, skb->data,
-				    bmax, DMA_TO_DEVICE);
-	if (dma_mapping_error(priv->device, desc->des2))
+	des2 = dma_map_single(priv->device, skb->data,
+			      bmax, DMA_TO_DEVICE);
+	desc->des2 = cpu_to_le32(des2);
+	if (dma_mapping_error(priv->device, des2))
 		return -1;
-	priv->tx_skbuff_dma[entry].buf = desc->des2;
+	priv->tx_skbuff_dma[entry].buf = des2;
 	priv->tx_skbuff_dma[entry].len = bmax;
 	/* do not close the descriptor and do not set own bit */
 	priv->hw->desc->prepare_tx_desc(desc, 1, bmax, csum, STMMAC_CHAIN_MODE,
@@ -60,12 +61,13 @@ static int stmmac_jumbo_frm(void *p, struct sk_buff *skb, int csum)
 		desc = priv->dma_tx + entry;
 
 		if (len > bmax) {
-			desc->des2 = dma_map_single(priv->device,
-						    (skb->data + bmax * i),
-						    bmax, DMA_TO_DEVICE);
-			if (dma_mapping_error(priv->device, desc->des2))
+			des2 = dma_map_single(priv->device,
+					      (skb->data + bmax * i),
+					      bmax, DMA_TO_DEVICE);
+			desc->des2 = cpu_to_le32(des2);
+			if (dma_mapping_error(priv->device, des2))
 				return -1;
-			priv->tx_skbuff_dma[entry].buf = desc->des2;
+			priv->tx_skbuff_dma[entry].buf = des2;
 			priv->tx_skbuff_dma[entry].len = bmax;
 			priv->hw->desc->prepare_tx_desc(desc, 0, bmax, csum,
 							STMMAC_CHAIN_MODE, 1,
@@ -73,12 +75,13 @@ static int stmmac_jumbo_frm(void *p, struct sk_buff *skb, int csum)
 			len -= bmax;
 			i++;
 		} else {
-			desc->des2 = dma_map_single(priv->device,
-						    (skb->data + bmax * i), len,
-						    DMA_TO_DEVICE);
-			if (dma_mapping_error(priv->device, desc->des2))
+			des2 = dma_map_single(priv->device,
+					      (skb->data + bmax * i), len,
+					      DMA_TO_DEVICE);
+			desc->des2 = cpu_to_le32(des2);
+			if (dma_mapping_error(priv->device, des2))
 				return -1;
-			priv->tx_skbuff_dma[entry].buf = desc->des2;
+			priv->tx_skbuff_dma[entry].buf = des2;
 			priv->tx_skbuff_dma[entry].len = len;
 			/* last descriptor can be set now */
 			priv->hw->desc->prepare_tx_desc(desc, 0, len, csum,
@@ -119,19 +122,19 @@ static void stmmac_init_dma_chain(void *des, dma_addr_t phy_addr,
 		struct dma_extended_desc *p = (struct dma_extended_desc *)des;
 		for (i = 0; i < (size - 1); i++) {
 			dma_phy += sizeof(struct dma_extended_desc);
-			p->basic.des3 = (unsigned int)dma_phy;
+			p->basic.des3 = cpu_to_le32((unsigned int)dma_phy);
 			p++;
 		}
-		p->basic.des3 = (unsigned int)phy_addr;
+		p->basic.des3 = cpu_to_le32((unsigned int)phy_addr);
 
 	} else {
 		struct dma_desc *p = (struct dma_desc *)des;
 		for (i = 0; i < (size - 1); i++) {
 			dma_phy += sizeof(struct dma_desc);
-			p->des3 = (unsigned int)dma_phy;
+			p->des3 = cpu_to_le32((unsigned int)dma_phy);
 			p++;
 		}
-		p->des3 = (unsigned int)phy_addr;
+		p->des3 = cpu_to_le32((unsigned int)phy_addr);
 	}
 }
 
@@ -144,10 +147,10 @@ static void stmmac_refill_desc3(void *priv_ptr, struct dma_desc *p)
 		 * 1588-2002 time stamping is enabled, hence reinitialize it
 		 * to keep explicit chaining in the descriptor.
 		 */
-		p->des3 = (unsigned int)(priv->dma_rx_phy +
-					 (((priv->dirty_rx) + 1) %
-					  DMA_RX_SIZE) *
-					 sizeof(struct dma_desc));
+		p->des3 = cpu_to_le32((unsigned int)(priv->dma_rx_phy +
+				      (((priv->dirty_rx) + 1) %
+				       DMA_RX_SIZE) *
+				      sizeof(struct dma_desc)));
 }
 
 static void stmmac_clean_desc3(void *priv_ptr, struct dma_desc *p)
@@ -161,9 +164,9 @@ static void stmmac_clean_desc3(void *priv_ptr, struct dma_desc *p)
 		 * 1588-2002 time stamping is enabled, hence reinitialize it
 		 * to keep explicit chaining in the descriptor.
 		 */
-		p->des3 = (unsigned int)((priv->dma_tx_phy +
-					  ((priv->dirty_tx + 1) % DMA_TX_SIZE))
-					  * sizeof(struct dma_desc));
+		p->des3 = cpu_to_le32((unsigned int)((priv->dma_tx_phy +
+				      ((priv->dirty_tx + 1) % DMA_TX_SIZE))
+				      * sizeof(struct dma_desc)));
 }
 
 const struct stmmac_mode_ops chain_mode_ops = {
diff --git a/drivers/net/ethernet/stmicro/stmmac/descs.h b/drivers/net/ethernet/stmicro/stmmac/descs.h
index 2e4c171..4000af4 100644
--- a/drivers/net/ethernet/stmicro/stmmac/descs.h
+++ b/drivers/net/ethernet/stmicro/stmmac/descs.h
@@ -87,7 +87,7 @@
 #define	TDES0_ERROR_SUMMARY		BIT(15)
 #define	TDES0_IP_HEADER_ERROR		BIT(16)
 #define	TDES0_TIME_STAMP_STATUS		BIT(17)
-#define	TDES0_OWN			BIT(31)
+#define	TDES0_OWN			((u32)BIT(31))	/* silence sparse */
 /* TDES1 */
 #define	TDES1_BUFFER1_SIZE_MASK		GENMASK(10, 0)
 #define	TDES1_BUFFER2_SIZE_MASK		GENMASK(21, 11)
@@ -130,7 +130,7 @@
 #define	ETDES0_FIRST_SEGMENT		BIT(28)
 #define	ETDES0_LAST_SEGMENT		BIT(29)
 #define	ETDES0_INTERRUPT		BIT(30)
-#define	ETDES0_OWN			BIT(31)
+#define	ETDES0_OWN			((u32)BIT(31))	/* silence sparse */
 /* TDES1 */
 #define	ETDES1_BUFFER1_SIZE_MASK	GENMASK(12, 0)
 #define	ETDES1_BUFFER2_SIZE_MASK	GENMASK(28, 16)
@@ -166,19 +166,19 @@
 
 /* Basic descriptor structure for normal and alternate descriptors */
 struct dma_desc {
-	unsigned int des0;
-	unsigned int des1;
-	unsigned int des2;
-	unsigned int des3;
+	__le32 des0;
+	__le32 des1;
+	__le32 des2;
+	__le32 des3;
 };
 
 /* Extended descriptor structure (e.g. >= databook 3.50a) */
 struct dma_extended_desc {
 	struct dma_desc basic;	/* Basic descriptors */
-	unsigned int des4;	/* Extended Status */
-	unsigned int des5;	/* Reserved */
-	unsigned int des6;	/* Tx/Rx Timestamp Low */
-	unsigned int des7;	/* Tx/Rx Timestamp High */
+	__le32 des4;	/* Extended Status */
+	__le32 des5;	/* Reserved */
+	__le32 des6;	/* Tx/Rx Timestamp Low */
+	__le32 des7;	/* Tx/Rx Timestamp High */
 };
 
 /* Transmit checksum insertion control */
diff --git a/drivers/net/ethernet/stmicro/stmmac/descs_com.h b/drivers/net/ethernet/stmicro/stmmac/descs_com.h
index 7635a46..1d181e2 100644
--- a/drivers/net/ethernet/stmicro/stmmac/descs_com.h
+++ b/drivers/net/ethernet/stmicro/stmmac/descs_com.h
@@ -35,47 +35,50 @@
 /* Enhanced descriptors */
 static inline void ehn_desc_rx_set_on_ring(struct dma_desc *p, int end)
 {
-	p->des1 |= ((BUF_SIZE_8KiB - 1) << ERDES1_BUFFER2_SIZE_SHIFT)
-		   & ERDES1_BUFFER2_SIZE_MASK;
+	p->des1 |= cpu_to_le32(((BUF_SIZE_8KiB - 1)
+			<< ERDES1_BUFFER2_SIZE_SHIFT)
+		   & ERDES1_BUFFER2_SIZE_MASK);
 
 	if (end)
-		p->des1 |= ERDES1_END_RING;
+		p->des1 |= cpu_to_le32(ERDES1_END_RING);
 }
 
 static inline void enh_desc_end_tx_desc_on_ring(struct dma_desc *p, int end)
 {
 	if (end)
-		p->des0 |= ETDES0_END_RING;
+		p->des0 |= cpu_to_le32(ETDES0_END_RING);
 	else
-		p->des0 &= ~ETDES0_END_RING;
+		p->des0 &= cpu_to_le32(~ETDES0_END_RING);
 }
 
 static inline void enh_set_tx_desc_len_on_ring(struct dma_desc *p, int len)
 {
 	if (unlikely(len > BUF_SIZE_4KiB)) {
-		p->des1 |= (((len - BUF_SIZE_4KiB) << ETDES1_BUFFER2_SIZE_SHIFT)
+		p->des1 |= cpu_to_le32((((len - BUF_SIZE_4KiB)
+					<< ETDES1_BUFFER2_SIZE_SHIFT)
 			    & ETDES1_BUFFER2_SIZE_MASK) | (BUF_SIZE_4KiB
-			    & ETDES1_BUFFER1_SIZE_MASK);
+			    & ETDES1_BUFFER1_SIZE_MASK));
 	} else
-		p->des1 |= (len & ETDES1_BUFFER1_SIZE_MASK);
+		p->des1 |= cpu_to_le32((len & ETDES1_BUFFER1_SIZE_MASK));
 }
 
 /* Normal descriptors */
 static inline void ndesc_rx_set_on_ring(struct dma_desc *p, int end)
 {
-	p->des1 |= ((BUF_SIZE_2KiB - 1) << RDES1_BUFFER2_SIZE_SHIFT)
-		    & RDES1_BUFFER2_SIZE_MASK;
+	p->des1 |= cpu_to_le32(((BUF_SIZE_2KiB - 1)
+				<< RDES1_BUFFER2_SIZE_SHIFT)
+		    & RDES1_BUFFER2_SIZE_MASK);
 
 	if (end)
-		p->des1 |= RDES1_END_RING;
+		p->des1 |= cpu_to_le32(RDES1_END_RING);
 }
 
 static inline void ndesc_end_tx_desc_on_ring(struct dma_desc *p, int end)
 {
 	if (end)
-		p->des1 |= TDES1_END_RING;
+		p->des1 |= cpu_to_le32(TDES1_END_RING);
 	else
-		p->des1 &= ~TDES1_END_RING;
+		p->des1 &= cpu_to_le32(~TDES1_END_RING);
 }
 
 static inline void norm_set_tx_desc_len_on_ring(struct dma_desc *p, int len)
@@ -83,10 +86,11 @@ static inline void norm_set_tx_desc_len_on_ring(struct dma_desc *p, int len)
 	if (unlikely(len > BUF_SIZE_2KiB)) {
 		unsigned int buffer1 = (BUF_SIZE_2KiB - 1)
 					& TDES1_BUFFER1_SIZE_MASK;
-		p->des1 |= ((((len - buffer1) << TDES1_BUFFER2_SIZE_SHIFT)
-			    & TDES1_BUFFER2_SIZE_MASK) | buffer1);
+		p->des1 |= cpu_to_le32((((len - buffer1)
+					<< TDES1_BUFFER2_SIZE_SHIFT)
+				& TDES1_BUFFER2_SIZE_MASK) | buffer1);
 	} else
-		p->des1 |= (len & TDES1_BUFFER1_SIZE_MASK);
+		p->des1 |= cpu_to_le32((len & TDES1_BUFFER1_SIZE_MASK));
 }
 
 /* Specific functions used for Chain mode */
@@ -94,32 +98,32 @@ static inline void norm_set_tx_desc_len_on_ring(struct dma_desc *p, int len)
 /* Enhanced descriptors */
 static inline void ehn_desc_rx_set_on_chain(struct dma_desc *p)
 {
-	p->des1 |= ERDES1_SECOND_ADDRESS_CHAINED;
+	p->des1 |= cpu_to_le32(ERDES1_SECOND_ADDRESS_CHAINED);
 }
 
 static inline void enh_desc_end_tx_desc_on_chain(struct dma_desc *p)
 {
-	p->des0 |= ETDES0_SECOND_ADDRESS_CHAINED;
+	p->des0 |= cpu_to_le32(ETDES0_SECOND_ADDRESS_CHAINED);
 }
 
 static inline void enh_set_tx_desc_len_on_chain(struct dma_desc *p, int len)
 {
-	p->des1 |= (len & ETDES1_BUFFER1_SIZE_MASK);
+	p->des1 |= cpu_to_le32(len & ETDES1_BUFFER1_SIZE_MASK);
 }
 
 /* Normal descriptors */
 static inline void ndesc_rx_set_on_chain(struct dma_desc *p, int end)
 {
-	p->des1 |= RDES1_SECOND_ADDRESS_CHAINED;
+	p->des1 |= cpu_to_le32(RDES1_SECOND_ADDRESS_CHAINED);
 }
 
 static inline void ndesc_tx_set_on_chain(struct dma_desc *p)
 {
-	p->des1 |= TDES1_SECOND_ADDRESS_CHAINED;
+	p->des1 |= cpu_to_le32(TDES1_SECOND_ADDRESS_CHAINED);
 }
 
 static inline void norm_set_tx_desc_len_on_chain(struct dma_desc *p, int len)
 {
-	p->des1 |= len & TDES1_BUFFER1_SIZE_MASK;
+	p->des1 |= cpu_to_le32(len & TDES1_BUFFER1_SIZE_MASK);
 }
 #endif /* __DESC_COM_H__ */
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac4_descs.c b/drivers/net/ethernet/stmicro/stmmac/dwmac4_descs.c
index a1b17cd..bec72d3 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac4_descs.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac4_descs.c
@@ -23,7 +23,7 @@ static int dwmac4_wrback_get_tx_status(void *data, struct stmmac_extra_stats *x,
 	unsigned int tdes3;
 	int ret = tx_done;
 
-	tdes3 = p->des3;
+	tdes3 = le32_to_cpu(p->des3);
 
 	/* Get tx owner first */
 	if (unlikely(tdes3 & TDES3_OWN))
@@ -77,9 +77,9 @@ static int dwmac4_wrback_get_rx_status(void *data, struct stmmac_extra_stats *x,
 				       struct dma_desc *p)
 {
 	struct net_device_stats *stats = (struct net_device_stats *)data;
-	unsigned int rdes1 = p->des1;
-	unsigned int rdes2 = p->des2;
-	unsigned int rdes3 = p->des3;
+	unsigned int rdes1 = le32_to_cpu(p->des1);
+	unsigned int rdes2 = le32_to_cpu(p->des2);
+	unsigned int rdes3 = le32_to_cpu(p->des3);
 	int message_type;
 	int ret = good_frame;
 
@@ -169,42 +169,43 @@ static int dwmac4_wrback_get_rx_status(void *data, struct stmmac_extra_stats *x,
 
 static int dwmac4_rd_get_tx_len(struct dma_desc *p)
 {
-	return (p->des2 & TDES2_BUFFER1_SIZE_MASK);
+	return (le32_to_cpu(p->des2) & TDES2_BUFFER1_SIZE_MASK);
 }
 
 static int dwmac4_get_tx_owner(struct dma_desc *p)
 {
-	return (p->des3 & TDES3_OWN) >> TDES3_OWN_SHIFT;
+	return (le32_to_cpu(p->des3) & TDES3_OWN) >> TDES3_OWN_SHIFT;
 }
 
 static void dwmac4_set_tx_owner(struct dma_desc *p)
 {
-	p->des3 |= TDES3_OWN;
+	p->des3 |= cpu_to_le32(TDES3_OWN);
 }
 
 static void dwmac4_set_rx_owner(struct dma_desc *p)
 {
-	p->des3 |= RDES3_OWN;
+	p->des3 |= cpu_to_le32(RDES3_OWN);
 }
 
 static int dwmac4_get_tx_ls(struct dma_desc *p)
 {
-	return (p->des3 & TDES3_LAST_DESCRIPTOR) >> TDES3_LAST_DESCRIPTOR_SHIFT;
+	return (le32_to_cpu(p->des3) & TDES3_LAST_DESCRIPTOR)
+		>> TDES3_LAST_DESCRIPTOR_SHIFT;
 }
 
 static int dwmac4_wrback_get_rx_frame_len(struct dma_desc *p, int rx_coe)
 {
-	return (p->des3 & RDES3_PACKET_SIZE_MASK);
+	return (le32_to_cpu(p->des3) & RDES3_PACKET_SIZE_MASK);
 }
 
 static void dwmac4_rd_enable_tx_timestamp(struct dma_desc *p)
 {
-	p->des2 |= TDES2_TIMESTAMP_ENABLE;
+	p->des2 |= cpu_to_le32(TDES2_TIMESTAMP_ENABLE);
 }
 
 static int dwmac4_wrback_get_tx_timestamp_status(struct dma_desc *p)
 {
-	return (p->des3 & TDES3_TIMESTAMP_STATUS)
+	return (le32_to_cpu(p->des3) & TDES3_TIMESTAMP_STATUS)
 		>> TDES3_TIMESTAMP_STATUS_SHIFT;
 }
 
@@ -216,9 +217,9 @@ static u64 dwmac4_wrback_get_timestamp(void *desc, u32 ats)
 	struct dma_desc *p = (struct dma_desc *)desc;
 	u64 ns;
 
-	ns = p->des0;
+	ns = le32_to_cpu(p->des0);
 	/* convert high/sec time stamp value to nanosecond */
-	ns += p->des1 * 1000000000ULL;
+	ns += le32_to_cpu(p->des1) * 1000000000ULL;
 
 	return ns;
 }
@@ -227,17 +228,17 @@ static int dwmac4_context_get_rx_timestamp_status(void *desc, u32 ats)
 {
 	struct dma_desc *p = (struct dma_desc *)desc;
 
-	return (p->des1 & RDES1_TIMESTAMP_AVAILABLE)
+	return (le32_to_cpu(p->des1) & RDES1_TIMESTAMP_AVAILABLE)
 		>> RDES1_TIMESTAMP_AVAILABLE_SHIFT;
 }
 
 static void dwmac4_rd_init_rx_desc(struct dma_desc *p, int disable_rx_ic,
 				   int mode, int end)
 {
-	p->des3 = RDES3_OWN | RDES3_BUFFER1_VALID_ADDR;
+	p->des3 = cpu_to_le32(RDES3_OWN | RDES3_BUFFER1_VALID_ADDR);
 
 	if (!disable_rx_ic)
-		p->des3 |= RDES3_INT_ON_COMPLETION_EN;
+		p->des3 |= cpu_to_le32(RDES3_INT_ON_COMPLETION_EN);
 }
 
 static void dwmac4_rd_init_tx_desc(struct dma_desc *p, int mode, int end)
@@ -252,9 +253,9 @@ static void dwmac4_rd_prepare_tx_desc(struct dma_desc *p, int is_fs, int len,
 				      bool csum_flag, int mode, bool tx_own,
 				      bool ls)
 {
-	unsigned int tdes3 = p->des3;
+	unsigned int tdes3 = le32_to_cpu(p->des3);
 
-	p->des2 |= (len & TDES2_BUFFER1_SIZE_MASK);
+	p->des2 |= cpu_to_le32(len & TDES2_BUFFER1_SIZE_MASK);
 
 	if (is_fs)
 		tdes3 |= TDES3_FIRST_DESCRIPTOR;
@@ -282,7 +283,7 @@ static void dwmac4_rd_prepare_tx_desc(struct dma_desc *p, int is_fs, int len,
 		 */
 		wmb();
 
-	p->des3 = tdes3;
+	p->des3 = cpu_to_le32(tdes3);
 }
 
 static void dwmac4_rd_prepare_tso_tx_desc(struct dma_desc *p, int is_fs,
@@ -290,14 +291,14 @@ static void dwmac4_rd_prepare_tso_tx_desc(struct dma_desc *p, int is_fs,
 					  bool ls, unsigned int tcphdrlen,
 					  unsigned int tcppayloadlen)
 {
-	unsigned int tdes3 = p->des3;
+	unsigned int tdes3 = le32_to_cpu(p->des3);
 
 	if (len1)
-		p->des2 |= (len1 & TDES2_BUFFER1_SIZE_MASK);
+		p->des2 |= cpu_to_le32((len1 & TDES2_BUFFER1_SIZE_MASK));
 
 	if (len2)
-		p->des2 |= (len2 << TDES2_BUFFER2_SIZE_MASK_SHIFT)
-			    & TDES2_BUFFER2_SIZE_MASK;
+		p->des2 |= cpu_to_le32((len2 << TDES2_BUFFER2_SIZE_MASK_SHIFT)
+			    & TDES2_BUFFER2_SIZE_MASK);
 
 	if (is_fs) {
 		tdes3 |= TDES3_FIRST_DESCRIPTOR |
@@ -325,7 +326,7 @@ static void dwmac4_rd_prepare_tso_tx_desc(struct dma_desc *p, int is_fs,
 		 */
 		wmb();
 
-	p->des3 = tdes3;
+	p->des3 = cpu_to_le32(tdes3);
 }
 
 static void dwmac4_release_tx_desc(struct dma_desc *p, int mode)
@@ -336,7 +337,7 @@ static void dwmac4_release_tx_desc(struct dma_desc *p, int mode)
 
 static void dwmac4_rd_set_tx_ic(struct dma_desc *p)
 {
-	p->des2 |= TDES2_INTERRUPT_ON_COMPLETION;
+	p->des2 |= cpu_to_le32(TDES2_INTERRUPT_ON_COMPLETION);
 }
 
 static void dwmac4_display_ring(void *head, unsigned int size, bool rx)
@@ -349,7 +350,8 @@ static void dwmac4_display_ring(void *head, unsigned int size, bool rx)
 	for (i = 0; i < size; i++) {
 		pr_info("%d [0x%x]: 0x%x 0x%x 0x%x 0x%x\n",
 			i, (unsigned int)virt_to_phys(p),
-			p->des0, p->des1, p->des2, p->des3);
+			le32_to_cpu(p->des0), le32_to_cpu(p->des1),
+			le32_to_cpu(p->des2), le32_to_cpu(p->des3));
 		p++;
 	}
 }
@@ -358,8 +360,8 @@ static void dwmac4_set_mss_ctxt(struct dma_desc *p, unsigned int mss)
 {
 	p->des0 = 0;
 	p->des1 = 0;
-	p->des2 = mss;
-	p->des3 = TDES3_CONTEXT_TYPE | TDES3_CTXT_TCMSSV;
+	p->des2 = cpu_to_le32(mss);
+	p->des3 = cpu_to_le32(TDES3_CONTEXT_TYPE | TDES3_CTXT_TCMSSV);
 }
 
 const struct stmmac_desc_ops dwmac4_desc_ops = {
diff --git a/drivers/net/ethernet/stmicro/stmmac/enh_desc.c b/drivers/net/ethernet/stmicro/stmmac/enh_desc.c
index 38f19c9..8295aa9 100644
--- a/drivers/net/ethernet/stmicro/stmmac/enh_desc.c
+++ b/drivers/net/ethernet/stmicro/stmmac/enh_desc.c
@@ -30,7 +30,7 @@ static int enh_desc_get_tx_status(void *data, struct stmmac_extra_stats *x,
 				  struct dma_desc *p, void __iomem *ioaddr)
 {
 	struct net_device_stats *stats = (struct net_device_stats *)data;
-	unsigned int tdes0 = p->des0;
+	unsigned int tdes0 = le32_to_cpu(p->des0);
 	int ret = tx_done;
 
 	/* Get tx owner first */
@@ -95,7 +95,7 @@ static int enh_desc_get_tx_status(void *data, struct stmmac_extra_stats *x,
 
 static int enh_desc_get_tx_len(struct dma_desc *p)
 {
-	return (p->des1 & ETDES1_BUFFER1_SIZE_MASK);
+	return (le32_to_cpu(p->des1) & ETDES1_BUFFER1_SIZE_MASK);
 }
 
 static int enh_desc_coe_rdes0(int ipc_err, int type, int payload_err)
@@ -134,8 +134,8 @@ static int enh_desc_coe_rdes0(int ipc_err, int type, int payload_err)
 static void enh_desc_get_ext_status(void *data, struct stmmac_extra_stats *x,
 				    struct dma_extended_desc *p)
 {
-	unsigned int rdes0 = p->basic.des0;
-	unsigned int rdes4 = p->des4;
+	unsigned int rdes0 = le32_to_cpu(p->basic.des0);
+	unsigned int rdes4 = le32_to_cpu(p->des4);
 
 	if (unlikely(rdes0 & ERDES0_RX_MAC_ADDR)) {
 		int message_type = (rdes4 & ERDES4_MSG_TYPE_MASK) >> 8;
@@ -191,7 +191,7 @@ static int enh_desc_get_rx_status(void *data, struct stmmac_extra_stats *x,
 				  struct dma_desc *p)
 {
 	struct net_device_stats *stats = (struct net_device_stats *)data;
-	unsigned int rdes0 = p->des0;
+	unsigned int rdes0 = le32_to_cpu(p->des0);
 	int ret = good_frame;
 
 	if (unlikely(rdes0 & RDES0_OWN))
@@ -257,8 +257,8 @@ static int enh_desc_get_rx_status(void *data, struct stmmac_extra_stats *x,
 static void enh_desc_init_rx_desc(struct dma_desc *p, int disable_rx_ic,
 				  int mode, int end)
 {
-	p->des0 |= RDES0_OWN;
-	p->des1 |= ((BUF_SIZE_8KiB - 1) & ERDES1_BUFFER1_SIZE_MASK);
+	p->des0 |= cpu_to_le32(RDES0_OWN);
+	p->des1 |= cpu_to_le32((BUF_SIZE_8KiB - 1) & ERDES1_BUFFER1_SIZE_MASK);
 
 	if (mode == STMMAC_CHAIN_MODE)
 		ehn_desc_rx_set_on_chain(p);
@@ -266,12 +266,12 @@ static void enh_desc_init_rx_desc(struct dma_desc *p, int disable_rx_ic,
 		ehn_desc_rx_set_on_ring(p, end);
 
 	if (disable_rx_ic)
-		p->des1 |= ERDES1_DISABLE_IC;
+		p->des1 |= cpu_to_le32(ERDES1_DISABLE_IC);
 }
 
 static void enh_desc_init_tx_desc(struct dma_desc *p, int mode, int end)
 {
-	p->des0 &= ~ETDES0_OWN;
+	p->des0 &= cpu_to_le32(~ETDES0_OWN);
 	if (mode == STMMAC_CHAIN_MODE)
 		enh_desc_end_tx_desc_on_chain(p);
 	else
@@ -280,27 +280,27 @@ static void enh_desc_init_tx_desc(struct dma_desc *p, int mode, int end)
 
 static int enh_desc_get_tx_owner(struct dma_desc *p)
 {
-	return (p->des0 & ETDES0_OWN) >> 31;
+	return (le32_to_cpu(p->des0) & ETDES0_OWN) >> 31;
 }
 
 static void enh_desc_set_tx_owner(struct dma_desc *p)
 {
-	p->des0 |= ETDES0_OWN;
+	p->des0 |= cpu_to_le32(ETDES0_OWN);
 }
 
 static void enh_desc_set_rx_owner(struct dma_desc *p)
 {
-	p->des0 |= RDES0_OWN;
+	p->des0 |= cpu_to_le32(RDES0_OWN);
 }
 
 static int enh_desc_get_tx_ls(struct dma_desc *p)
 {
-	return (p->des0 & ETDES0_LAST_SEGMENT) >> 29;
+	return (le32_to_cpu(p->des0) & ETDES0_LAST_SEGMENT) >> 29;
 }
 
 static void enh_desc_release_tx_desc(struct dma_desc *p, int mode)
 {
-	int ter = (p->des0 & ETDES0_END_RING) >> 21;
+	int ter = (le32_to_cpu(p->des0) & ETDES0_END_RING) >> 21;
 
 	memset(p, 0, offsetof(struct dma_desc, des2));
 	if (mode == STMMAC_CHAIN_MODE)
@@ -313,7 +313,7 @@ static void enh_desc_prepare_tx_desc(struct dma_desc *p, int is_fs, int len,
 				     bool csum_flag, int mode, bool tx_own,
 				     bool ls)
 {
-	unsigned int tdes0 = p->des0;
+	unsigned int tdes0 = le32_to_cpu(p->des0);
 
 	if (mode == STMMAC_CHAIN_MODE)
 		enh_set_tx_desc_len_on_chain(p, len);
@@ -344,12 +344,12 @@ static void enh_desc_prepare_tx_desc(struct dma_desc *p, int is_fs, int len,
 		 */
 		wmb();
 
-	p->des0 = tdes0;
+	p->des0 = cpu_to_le32(tdes0);
 }
 
 static void enh_desc_set_tx_ic(struct dma_desc *p)
 {
-	p->des0 |= ETDES0_INTERRUPT;
+	p->des0 |= cpu_to_le32(ETDES0_INTERRUPT);
 }
 
 static int enh_desc_get_rx_frame_len(struct dma_desc *p, int rx_coe_type)
@@ -364,18 +364,18 @@ static int enh_desc_get_rx_frame_len(struct dma_desc *p, int rx_coe_type)
 	if (rx_coe_type == STMMAC_RX_COE_TYPE1)
 		csum = 2;
 
-	return (((p->des0 & RDES0_FRAME_LEN_MASK) >> RDES0_FRAME_LEN_SHIFT) -
-		csum);
+	return (((le32_to_cpu(p->des0) & RDES0_FRAME_LEN_MASK)
+				>> RDES0_FRAME_LEN_SHIFT) - csum);
 }
 
 static void enh_desc_enable_tx_timestamp(struct dma_desc *p)
 {
-	p->des0 |= ETDES0_TIME_STAMP_ENABLE;
+	p->des0 |= cpu_to_le32(ETDES0_TIME_STAMP_ENABLE);
 }
 
 static int enh_desc_get_tx_timestamp_status(struct dma_desc *p)
 {
-	return (p->des0 & ETDES0_TIME_STAMP_STATUS) >> 17;
+	return (le32_to_cpu(p->des0) & ETDES0_TIME_STAMP_STATUS) >> 17;
 }
 
 static u64 enh_desc_get_timestamp(void *desc, u32 ats)
@@ -384,13 +384,13 @@ static u64 enh_desc_get_timestamp(void *desc, u32 ats)
 
 	if (ats) {
 		struct dma_extended_desc *p = (struct dma_extended_desc *)desc;
-		ns = p->des6;
+		ns = le32_to_cpu(p->des6);
 		/* convert high/sec time stamp value to nanosecond */
-		ns += p->des7 * 1000000000ULL;
+		ns += le32_to_cpu(p->des7) * 1000000000ULL;
 	} else {
 		struct dma_desc *p = (struct dma_desc *)desc;
-		ns = p->des2;
-		ns += p->des3 * 1000000000ULL;
+		ns = le32_to_cpu(p->des2);
+		ns += le32_to_cpu(p->des3) * 1000000000ULL;
 	}
 
 	return ns;
@@ -400,10 +400,11 @@ static int enh_desc_get_rx_timestamp_status(void *desc, u32 ats)
 {
 	if (ats) {
 		struct dma_extended_desc *p = (struct dma_extended_desc *)desc;
-		return (p->basic.des0 & RDES0_IPC_CSUM_ERROR) >> 7;
+		return (le32_to_cpu(p->basic.des0) & RDES0_IPC_CSUM_ERROR) >> 7;
 	} else {
 		struct dma_desc *p = (struct dma_desc *)desc;
-		if ((p->des2 == 0xffffffff) && (p->des3 == 0xffffffff))
+		if ((le32_to_cpu(p->des2) == 0xffffffff) &&
+		    (le32_to_cpu(p->des3) == 0xffffffff))
 			/* timestamp is corrupted, hence don't store it */
 			return 0;
 		else
diff --git a/drivers/net/ethernet/stmicro/stmmac/norm_desc.c b/drivers/net/ethernet/stmicro/stmmac/norm_desc.c
index 2beacd0..fd78406 100644
--- a/drivers/net/ethernet/stmicro/stmmac/norm_desc.c
+++ b/drivers/net/ethernet/stmicro/stmmac/norm_desc.c
@@ -30,8 +30,8 @@ static int ndesc_get_tx_status(void *data, struct stmmac_extra_stats *x,
 			       struct dma_desc *p, void __iomem *ioaddr)
 {
 	struct net_device_stats *stats = (struct net_device_stats *)data;
-	unsigned int tdes0 = p->des0;
-	unsigned int tdes1 = p->des1;
+	unsigned int tdes0 = le32_to_cpu(p->des0);
+	unsigned int tdes1 = le32_to_cpu(p->des1);
 	int ret = tx_done;
 
 	/* Get tx owner first */
@@ -77,7 +77,7 @@ static int ndesc_get_tx_status(void *data, struct stmmac_extra_stats *x,
 
 static int ndesc_get_tx_len(struct dma_desc *p)
 {
-	return (p->des1 & RDES1_BUFFER1_SIZE_MASK);
+	return (le32_to_cpu(p->des1) & RDES1_BUFFER1_SIZE_MASK);
 }
 
 /* This function verifies if each incoming frame has some errors
@@ -88,7 +88,7 @@ static int ndesc_get_rx_status(void *data, struct stmmac_extra_stats *x,
 			       struct dma_desc *p)
 {
 	int ret = good_frame;
-	unsigned int rdes0 = p->des0;
+	unsigned int rdes0 = le32_to_cpu(p->des0);
 	struct net_device_stats *stats = (struct net_device_stats *)data;
 
 	if (unlikely(rdes0 & RDES0_OWN))
@@ -141,8 +141,8 @@ static int ndesc_get_rx_status(void *data, struct stmmac_extra_stats *x,
 static void ndesc_init_rx_desc(struct dma_desc *p, int disable_rx_ic, int mode,
 			       int end)
 {
-	p->des0 |= RDES0_OWN;
-	p->des1 |= (BUF_SIZE_2KiB - 1) & RDES1_BUFFER1_SIZE_MASK;
+	p->des0 |= cpu_to_le32(RDES0_OWN);
+	p->des1 |= cpu_to_le32((BUF_SIZE_2KiB - 1) & RDES1_BUFFER1_SIZE_MASK);
 
 	if (mode == STMMAC_CHAIN_MODE)
 		ndesc_rx_set_on_chain(p, end);
@@ -150,12 +150,12 @@ static void ndesc_init_rx_desc(struct dma_desc *p, int disable_rx_ic, int mode,
 		ndesc_rx_set_on_ring(p, end);
 
 	if (disable_rx_ic)
-		p->des1 |= RDES1_DISABLE_IC;
+		p->des1 |= cpu_to_le32(RDES1_DISABLE_IC);
 }
 
 static void ndesc_init_tx_desc(struct dma_desc *p, int mode, int end)
 {
-	p->des0 &= ~TDES0_OWN;
+	p->des0 &= cpu_to_le32(~TDES0_OWN);
 	if (mode == STMMAC_CHAIN_MODE)
 		ndesc_tx_set_on_chain(p);
 	else
@@ -164,27 +164,27 @@ static void ndesc_init_tx_desc(struct dma_desc *p, int mode, int end)
 
 static int ndesc_get_tx_owner(struct dma_desc *p)
 {
-	return (p->des0 & TDES0_OWN) >> 31;
+	return (le32_to_cpu(p->des0) & TDES0_OWN) >> 31;
 }
 
 static void ndesc_set_tx_owner(struct dma_desc *p)
 {
-	p->des0 |= TDES0_OWN;
+	p->des0 |= cpu_to_le32(TDES0_OWN);
 }
 
 static void ndesc_set_rx_owner(struct dma_desc *p)
 {
-	p->des0 |= RDES0_OWN;
+	p->des0 |= cpu_to_le32(RDES0_OWN);
 }
 
 static int ndesc_get_tx_ls(struct dma_desc *p)
 {
-	return (p->des1 & TDES1_LAST_SEGMENT) >> 30;
+	return (le32_to_cpu(p->des1) & TDES1_LAST_SEGMENT) >> 30;
 }
 
 static void ndesc_release_tx_desc(struct dma_desc *p, int mode)
 {
-	int ter = (p->des1 & TDES1_END_RING) >> 25;
+	int ter = (le32_to_cpu(p->des1) & TDES1_END_RING) >> 25;
 
 	memset(p, 0, offsetof(struct dma_desc, des2));
 	if (mode == STMMAC_CHAIN_MODE)
@@ -197,7 +197,7 @@ static void ndesc_prepare_tx_desc(struct dma_desc *p, int is_fs, int len,
 				  bool csum_flag, int mode, bool tx_own,
 				  bool ls)
 {
-	unsigned int tdes1 = p->des1;
+	unsigned int tdes1 = le32_to_cpu(p->des1);
 
 	if (is_fs)
 		tdes1 |= TDES1_FIRST_SEGMENT;
@@ -212,7 +212,7 @@ static void ndesc_prepare_tx_desc(struct dma_desc *p, int is_fs, int len,
 	if (ls)
 		tdes1 |= TDES1_LAST_SEGMENT;
 
-	p->des1 = tdes1;
+	p->des1 = cpu_to_le32(tdes1);
 
 	if (mode == STMMAC_CHAIN_MODE)
 		norm_set_tx_desc_len_on_chain(p, len);
@@ -220,12 +220,12 @@ static void ndesc_prepare_tx_desc(struct dma_desc *p, int is_fs, int len,
 		norm_set_tx_desc_len_on_ring(p, len);
 
 	if (tx_own)
-		p->des0 |= TDES0_OWN;
+		p->des0 |= cpu_to_le32(TDES0_OWN);
 }
 
 static void ndesc_set_tx_ic(struct dma_desc *p)
 {
-	p->des1 |= TDES1_INTERRUPT;
+	p->des1 |= cpu_to_le32(TDES1_INTERRUPT);
 }
 
 static int ndesc_get_rx_frame_len(struct dma_desc *p, int rx_coe_type)
@@ -241,19 +241,20 @@ static int ndesc_get_rx_frame_len(struct dma_desc *p, int rx_coe_type)
 	if (rx_coe_type == STMMAC_RX_COE_TYPE1)
 		csum = 2;
 
-	return (((p->des0 & RDES0_FRAME_LEN_MASK) >> RDES0_FRAME_LEN_SHIFT) -
+	return (((le32_to_cpu(p->des0) & RDES0_FRAME_LEN_MASK)
+				>> RDES0_FRAME_LEN_SHIFT) -
 		csum);
 
 }
 
 static void ndesc_enable_tx_timestamp(struct dma_desc *p)
 {
-	p->des1 |= TDES1_TIME_STAMP_ENABLE;
+	p->des1 |= cpu_to_le32(TDES1_TIME_STAMP_ENABLE);
 }
 
 static int ndesc_get_tx_timestamp_status(struct dma_desc *p)
 {
-	return (p->des0 & TDES0_TIME_STAMP_STATUS) >> 17;
+	return (le32_to_cpu(p->des0) & TDES0_TIME_STAMP_STATUS) >> 17;
 }
 
 static u64 ndesc_get_timestamp(void *desc, u32 ats)
@@ -261,9 +262,9 @@ static u64 ndesc_get_timestamp(void *desc, u32 ats)
 	struct dma_desc *p = (struct dma_desc *)desc;
 	u64 ns;
 
-	ns = p->des2;
+	ns = le32_to_cpu(p->des2);
 	/* convert high/sec time stamp value to nanosecond */
-	ns += p->des3 * 1000000000ULL;
+	ns += le32_to_cpu(p->des3) * 1000000000ULL;
 
 	return ns;
 }
@@ -272,7 +273,8 @@ static int ndesc_get_rx_timestamp_status(void *desc, u32 ats)
 {
 	struct dma_desc *p = (struct dma_desc *)desc;
 
-	if ((p->des2 == 0xffffffff) && (p->des3 == 0xffffffff))
+	if ((le32_to_cpu(p->des2) == 0xffffffff) &&
+	    (le32_to_cpu(p->des3) == 0xffffffff))
 		/* timestamp is corrupted, hence don't store it */
 		return 0;
 	else
diff --git a/drivers/net/ethernet/stmicro/stmmac/ring_mode.c b/drivers/net/ethernet/stmicro/stmmac/ring_mode.c
index 7723b5d..9983ce9 100644
--- a/drivers/net/ethernet/stmicro/stmmac/ring_mode.c
+++ b/drivers/net/ethernet/stmicro/stmmac/ring_mode.c
@@ -34,7 +34,7 @@ static int stmmac_jumbo_frm(void *p, struct sk_buff *skb, int csum)
 	unsigned int entry = priv->cur_tx;
 	struct dma_desc *desc;
 	unsigned int nopaged_len = skb_headlen(skb);
-	unsigned int bmax, len;
+	unsigned int bmax, len, des2;
 
 	if (priv->extend_desc)
 		desc = (struct dma_desc *)(priv->dma_etx + entry);
@@ -50,16 +50,17 @@ static int stmmac_jumbo_frm(void *p, struct sk_buff *skb, int csum)
 
 	if (nopaged_len > BUF_SIZE_8KiB) {
 
-		desc->des2 = dma_map_single(priv->device, skb->data,
-					    bmax, DMA_TO_DEVICE);
-		if (dma_mapping_error(priv->device, desc->des2))
+		des2 = dma_map_single(priv->device, skb->data, bmax,
+				      DMA_TO_DEVICE);
+		desc->des2 = cpu_to_le32(des2);
+		if (dma_mapping_error(priv->device, des2))
 			return -1;
 
-		priv->tx_skbuff_dma[entry].buf = desc->des2;
+		priv->tx_skbuff_dma[entry].buf = des2;
 		priv->tx_skbuff_dma[entry].len = bmax;
 		priv->tx_skbuff_dma[entry].is_jumbo = true;
 
-		desc->des3 = desc->des2 + BUF_SIZE_4KiB;
+		desc->des3 = cpu_to_le32(des2 + BUF_SIZE_4KiB);
 		priv->hw->desc->prepare_tx_desc(desc, 1, bmax, csum,
 						STMMAC_RING_MODE, 0, false);
 		priv->tx_skbuff[entry] = NULL;
@@ -70,26 +71,28 @@ static int stmmac_jumbo_frm(void *p, struct sk_buff *skb, int csum)
 		else
 			desc = priv->dma_tx + entry;
 
-		desc->des2 = dma_map_single(priv->device, skb->data + bmax,
-					    len, DMA_TO_DEVICE);
-		if (dma_mapping_error(priv->device, desc->des2))
+		des2 = dma_map_single(priv->device, skb->data + bmax, len,
+				      DMA_TO_DEVICE);
+		desc->des2 = cpu_to_le32(des2);
+		if (dma_mapping_error(priv->device, des2))
 			return -1;
-		priv->tx_skbuff_dma[entry].buf = desc->des2;
+		priv->tx_skbuff_dma[entry].buf = des2;
 		priv->tx_skbuff_dma[entry].len = len;
 		priv->tx_skbuff_dma[entry].is_jumbo = true;
 
-		desc->des3 = desc->des2 + BUF_SIZE_4KiB;
+		desc->des3 = cpu_to_le32(des2 + BUF_SIZE_4KiB);
 		priv->hw->desc->prepare_tx_desc(desc, 0, len, csum,
 						STMMAC_RING_MODE, 1, true);
 	} else {
-		desc->des2 = dma_map_single(priv->device, skb->data,
-					    nopaged_len, DMA_TO_DEVICE);
-		if (dma_mapping_error(priv->device, desc->des2))
+		des2 = dma_map_single(priv->device, skb->data,
+				      nopaged_len, DMA_TO_DEVICE);
+		desc->des2 = cpu_to_le32(des2);
+		if (dma_mapping_error(priv->device, des2))
 			return -1;
-		priv->tx_skbuff_dma[entry].buf = desc->des2;
+		priv->tx_skbuff_dma[entry].buf = des2;
 		priv->tx_skbuff_dma[entry].len = nopaged_len;
 		priv->tx_skbuff_dma[entry].is_jumbo = true;
-		desc->des3 = desc->des2 + BUF_SIZE_4KiB;
+		desc->des3 = cpu_to_le32(des2 + BUF_SIZE_4KiB);
 		priv->hw->desc->prepare_tx_desc(desc, 1, nopaged_len, csum,
 						STMMAC_RING_MODE, 0, true);
 	}
@@ -115,13 +118,13 @@ static void stmmac_refill_desc3(void *priv_ptr, struct dma_desc *p)
 
 	/* Fill DES3 in case of RING mode */
 	if (priv->dma_buf_sz >= BUF_SIZE_8KiB)
-		p->des3 = p->des2 + BUF_SIZE_8KiB;
+		p->des3 = cpu_to_le32(le32_to_cpu(p->des2) + BUF_SIZE_8KiB);
 }
 
 /* In ring mode we need to fill the desc3 because it is used as buffer */
 static void stmmac_init_desc3(struct dma_desc *p)
 {
-	p->des3 = p->des2 + BUF_SIZE_8KiB;
+	p->des3 = cpu_to_le32(le32_to_cpu(p->des2) + BUF_SIZE_8KiB);
 }
 
 static void stmmac_clean_desc3(void *priv_ptr, struct dma_desc *p)
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 48e71fa..2187c96 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -983,9 +983,9 @@ static int stmmac_init_rx_buffers(struct stmmac_priv *priv, struct dma_desc *p,
 	}
 
 	if (priv->synopsys_id >= DWMAC_CORE_4_00)
-		p->des0 = priv->rx_skbuff_dma[i];
+		p->des0 = cpu_to_le32(priv->rx_skbuff_dma[i]);
 	else
-		p->des2 = priv->rx_skbuff_dma[i];
+		p->des2 = cpu_to_le32(priv->rx_skbuff_dma[i]);
 
 	if ((priv->hw->mode->init_desc3) &&
 	    (priv->dma_buf_sz == BUF_SIZE_16KiB))
@@ -1940,7 +1940,7 @@ static void stmmac_tso_allocator(struct stmmac_priv *priv, unsigned int des,
 		priv->cur_tx = STMMAC_GET_ENTRY(priv->cur_tx, DMA_TX_SIZE);
 		desc = priv->dma_tx + priv->cur_tx;
 
-		desc->des0 = des + (total_len - tmp_len);
+		desc->des0 = cpu_to_le32(des + (total_len - tmp_len));
 		buff_size = tmp_len >= TSO_MAX_BUFF_SIZE ?
 			    TSO_MAX_BUFF_SIZE : tmp_len;
 
@@ -2042,11 +2042,11 @@ static netdev_tx_t stmmac_tso_xmit(struct sk_buff *skb, struct net_device *dev)
 	priv->tx_skbuff_dma[first_entry].len = skb_headlen(skb);
 	priv->tx_skbuff[first_entry] = skb;
 
-	first->des0 = des;
+	first->des0 = cpu_to_le32(des);
 
 	/* Fill start of payload in buff2 of first descriptor */
 	if (pay_len)
-		first->des1 =  des + proto_hdr_len;
+		first->des1 = cpu_to_le32(des + proto_hdr_len);
 
 	/* If needed take extra descriptors to fill the remaining payload */
 	tmp_pay_len = pay_len - TSO_MAX_BUFF_SIZE;
@@ -2235,13 +2235,11 @@ static netdev_tx_t stmmac_xmit(struct sk_buff *skb, struct net_device *dev)
 
 		priv->tx_skbuff[entry] = NULL;
 
-		if (unlikely(priv->synopsys_id >= DWMAC_CORE_4_00)) {
-			desc->des0 = des;
-			priv->tx_skbuff_dma[entry].buf = desc->des0;
-		} else {
-			desc->des2 = des;
-			priv->tx_skbuff_dma[entry].buf = desc->des2;
-		}
+		priv->tx_skbuff_dma[entry].buf = des;
+		if (unlikely(priv->synopsys_id >= DWMAC_CORE_4_00))
+			desc->des0 = cpu_to_le32(des);
+		else
+			desc->des2 = cpu_to_le32(des);
 
 		priv->tx_skbuff_dma[entry].map_as_page = true;
 		priv->tx_skbuff_dma[entry].len = len;
@@ -2312,13 +2310,11 @@ static netdev_tx_t stmmac_xmit(struct sk_buff *skb, struct net_device *dev)
 		if (dma_mapping_error(priv->device, des))
 			goto dma_map_err;
 
-		if (unlikely(priv->synopsys_id >= DWMAC_CORE_4_00)) {
-			first->des0 = des;
-			priv->tx_skbuff_dma[first_entry].buf = first->des0;
-		} else {
-			first->des2 = des;
-			priv->tx_skbuff_dma[first_entry].buf = first->des2;
-		}
+		priv->tx_skbuff_dma[first_entry].buf = des;
+		if (unlikely(priv->synopsys_id >= DWMAC_CORE_4_00))
+			first->des0 = cpu_to_le32(des);
+		else
+			first->des2 = cpu_to_le32(des);
 
 		priv->tx_skbuff_dma[first_entry].len = nopaged_len;
 		priv->tx_skbuff_dma[first_entry].last_segment = last_segment;
@@ -2432,10 +2428,10 @@ static inline void stmmac_rx_refill(struct stmmac_priv *priv)
 			}
 
 			if (unlikely(priv->synopsys_id >= DWMAC_CORE_4_00)) {
-				p->des0 = priv->rx_skbuff_dma[entry];
+				p->des0 = cpu_to_le32(priv->rx_skbuff_dma[entry]);
 				p->des1 = 0;
 			} else {
-				p->des2 = priv->rx_skbuff_dma[entry];
+				p->des2 = cpu_to_le32(priv->rx_skbuff_dma[entry]);
 			}
 			if (priv->hw->mode->refill_desc3)
 				priv->hw->mode->refill_desc3(priv, p);
@@ -2536,9 +2532,9 @@ static int stmmac_rx(struct stmmac_priv *priv, int limit)
 			unsigned int des;
 
 			if (unlikely(priv->synopsys_id >= DWMAC_CORE_4_00))
-				des = p->des0;
+				des = le32_to_cpu(p->des0);
 			else
-				des = p->des2;
+				des = le32_to_cpu(p->des2);
 
 			frame_len = priv->hw->desc->get_rx_frame_len(p, coe);
 
@@ -2912,14 +2908,17 @@ static void sysfs_display_ring(void *head, int size, int extend_desc,
 			x = *(u64 *) ep;
 			seq_printf(seq, "%d [0x%x]: 0x%x 0x%x 0x%x 0x%x\n",
 				   i, (unsigned int)virt_to_phys(ep),
-				   ep->basic.des0, ep->basic.des1,
-				   ep->basic.des2, ep->basic.des3);
+				   le32_to_cpu(ep->basic.des0),
+				   le32_to_cpu(ep->basic.des1),
+				   le32_to_cpu(ep->basic.des2),
+				   le32_to_cpu(ep->basic.des3));
 			ep++;
 		} else {
 			x = *(u64 *) p;
 			seq_printf(seq, "%d [0x%x]: 0x%x 0x%x 0x%x 0x%x\n",
 				   i, (unsigned int)virt_to_phys(ep),
-				   p->des0, p->des1, p->des2, p->des3);
+				   le32_to_cpu(p->des0), le32_to_cpu(p->des1),
+				   le32_to_cpu(p->des2), le32_to_cpu(p->des3));
 			p++;
 		}
 		seq_printf(seq, "\n");
-- 
2.9.3 (Apple Git-75)

^ permalink raw reply related

* [PATCH 2/3] net: ethernet: sun4i-emac: Allow to enable netif messages
From: Michael Weiser @ 2016-11-14 17:58 UTC (permalink / raw)
  To: netdev; +Cc: Michael Weiser, Maxime Ripard
In-Reply-To: <20161114175807.4747-1-michael.weiser@gmx.de>

sun4i-emac has the ability to print a number of diagnostic messages using
dev_dbg depending on message level settings implemented using netif_msg_*
macros. But there's no way to actually enable them.

Add the ability to switch diagnostic messages on using either a module
parameter debug or ethtool -s <netif> msglvl <flags>.

Signed-off-by: Michael Weiser <michael.weiser@gmx.de>
Cc: Maxime Ripard <maxime.ripard@free-electrons.com>
Cc: netdev@vger.kernel.org
---
 drivers/net/ethernet/allwinner/sun4i-emac.c | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/drivers/net/ethernet/allwinner/sun4i-emac.c b/drivers/net/ethernet/allwinner/sun4i-emac.c
index 6ffdff6..cd08885 100644
--- a/drivers/net/ethernet/allwinner/sun4i-emac.c
+++ b/drivers/net/ethernet/allwinner/sun4i-emac.c
@@ -37,6 +37,11 @@
 
 #define EMAC_MAX_FRAME_LEN	0x0600
 
+#define EMAC_DEFAULT_MSG_ENABLE 0x0000
+static int debug = -1;     /* defaults above */;
+module_param(debug, int, 0);
+MODULE_PARM_DESC(debug, "debug message flags");
+
 /* Transmit timeout, default 5 seconds. */
 static int watchdog = 5000;
 module_param(watchdog, int, 0400);
@@ -225,11 +230,27 @@ static void emac_get_drvinfo(struct net_device *dev,
 	strlcpy(info->bus_info, dev_name(&dev->dev), sizeof(info->bus_info));
 }
 
+static u32 emac_get_msglevel(struct net_device *dev)
+{
+	struct emac_board_info *db = netdev_priv(dev);
+
+	return db->msg_enable;
+}
+
+static void emac_set_msglevel(struct net_device *dev, u32 value)
+{
+	struct emac_board_info *db = netdev_priv(dev);
+
+	db->msg_enable = value;
+}
+
 static const struct ethtool_ops emac_ethtool_ops = {
 	.get_drvinfo	= emac_get_drvinfo,
 	.get_link	= ethtool_op_get_link,
 	.get_link_ksettings = phy_ethtool_get_link_ksettings,
 	.set_link_ksettings = phy_ethtool_set_link_ksettings,
+	.get_msglevel	= emac_get_msglevel,
+	.set_msglevel	= emac_set_msglevel,
 };
 
 static unsigned int emac_setup(struct net_device *ndev)
@@ -805,6 +826,7 @@ static int emac_probe(struct platform_device *pdev)
 	db->dev = &pdev->dev;
 	db->ndev = ndev;
 	db->pdev = pdev;
+	db->msg_enable = netif_msg_init(debug, EMAC_DEFAULT_MSG_ENABLE);
 
 	spin_lock_init(&db->lock);
 
-- 
2.9.3 (Apple Git-75)

^ permalink raw reply related

* [PATCH 3/3] net: ethernet: sun4i-emac: Read rxhdr in CPU byte-order
From: Michael Weiser @ 2016-11-14 17:58 UTC (permalink / raw)
  To: netdev; +Cc: Michael Weiser, Maxime Ripard
In-Reply-To: <20161114175807.4747-1-michael.weiser@gmx.de>

The EMAC EMAC_RX_IO_DATA_REG data register is dual-purpose: On one hand
it is used to move actual packet data off the wire. This will be in
wire-format and accepted as such by higher layers such as IP. Therefore
it is correctly read as-is (i.e. raw) using readsl.

On the other hand it provides metadata about incoming transfers to the
driver such as length and checksum validation status. This data is
little-endian, always and it is interpreted by the driver. Therefore it
needs to be swapped to CPU endianness to make sense to the driver. This
is already done for the "receive header" but not rxhdr.

Read rxhdr using readl in order for sun4i-emac to work correctly when
running a big-endian kernel.

Signed-off-by: Michael Weiser <michael.weiser@gmx.de>
Cc: Maxime Ripard <maxime.ripard@free-electrons.com>
Cc: netdev@vger.kernel.org
---
 drivers/net/ethernet/allwinner/sun4i-emac.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/allwinner/sun4i-emac.c b/drivers/net/ethernet/allwinner/sun4i-emac.c
index cd08885..87d0b87 100644
--- a/drivers/net/ethernet/allwinner/sun4i-emac.c
+++ b/drivers/net/ethernet/allwinner/sun4i-emac.c
@@ -592,8 +592,7 @@ static void emac_rx(struct net_device *dev)
 		/* A packet ready now  & Get status/length */
 		good_packet = true;
 
-		emac_inblk_32bit(db->membase + EMAC_RX_IO_DATA_REG,
-				&rxhdr, sizeof(rxhdr));
+		rxhdr = readl(db->membase + EMAC_RX_IO_DATA_REG);
 
 		if (netif_msg_rx_status(db))
 			dev_dbg(db->dev, "rxhdr: %x\n", *((int *)(&rxhdr)));
-- 
2.9.3 (Apple Git-75)

^ permalink raw reply related

* Re: [PATCH 2/3] net: ethernet: sun4i-emac: Allow to enable netif messages
From: Maxime Ripard @ 2016-11-14 19:00 UTC (permalink / raw)
  To: Michael Weiser; +Cc: netdev
In-Reply-To: <20161114175807.4747-3-michael.weiser@gmx.de>

[-- Attachment #1: Type: text/plain, Size: 741 bytes --]

On Mon, Nov 14, 2016 at 06:58:06PM +0100, Michael Weiser wrote:
> sun4i-emac has the ability to print a number of diagnostic messages using
> dev_dbg depending on message level settings implemented using netif_msg_*
> macros. But there's no way to actually enable them.
> 
> Add the ability to switch diagnostic messages on using either a module
> parameter debug or ethtool -s <netif> msglvl <flags>.
> 
> Signed-off-by: Michael Weiser <michael.weiser@gmx.de>
> Cc: Maxime Ripard <maxime.ripard@free-electrons.com>
> Cc: netdev@vger.kernel.org

Acked-by: Maxime Ripard <maxime.ripard@free-electrons.com>

Thanks!
Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

^ permalink raw reply

* [PATCH 0/3 v5] Fixes for running a big-endian kernel on Cubieboard2
From: Michael Weiser @ 2016-11-14 17:58 UTC (permalink / raw)
  To: netdev; +Cc: Michael Weiser

the following patches are what remains to be fixed in order to allow running a
big-endian kernel on the Cubieboard2.

The first patch fixes up endianness problems with DMA descriptors in
the stmmac driver preventing it from working correctly when runnning a
big-endian kernel.

The second patch adds the ability to enable diagnostic messages in the
sun4i-emac driver which were instrumental in finding the problem fixed
by patch number three: Endianness confusion caused by dual-purpose I/O
register usage in sun4i-emac.

All of these have been tested successfully on a Cubieboard2 DualCard.

Changes since v4:
- Rebased to current master
- Removed already applied patches to sunxi-mmc and sunxi-Kconfig

Changes since v3:
- Rebased sunxi-mmc patch against Ulf's mmc.git/next
- Changed Kconfig change to enable big-endian support only for sun7i
  devices

Changes since v2:
- Fixed typo in stmmac patch causing a build failure
- Added sun4i-emac patches

Changes since v1:
- Fixed checkpatch niggles
- Added respective Cc:s

Regards,
Michael

Michael Weiser (3):
  net: ethernet: stmmac: change dma descriptors to __le32
  net: ethernet: sun4i-emac: Allow to enable netif messages
  net: ethernet: sun4i-emac: Read rxhdr in CPU byte-order

 drivers/net/ethernet/allwinner/sun4i-emac.c        | 25 ++++++++-
 drivers/net/ethernet/stmicro/stmmac/chain_mode.c   | 55 ++++++++++----------
 drivers/net/ethernet/stmicro/stmmac/descs.h        | 20 ++++----
 drivers/net/ethernet/stmicro/stmmac/descs_com.h    | 48 +++++++++--------
 drivers/net/ethernet/stmicro/stmmac/dwmac4_descs.c | 60 +++++++++++-----------
 drivers/net/ethernet/stmicro/stmmac/enh_desc.c     | 55 ++++++++++----------
 drivers/net/ethernet/stmicro/stmmac/norm_desc.c    | 48 ++++++++---------
 drivers/net/ethernet/stmicro/stmmac/ring_mode.c    | 39 +++++++-------
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c  | 51 +++++++++---------
 9 files changed, 218 insertions(+), 183 deletions(-)

-- 
2.9.3 (Apple Git-75)

^ permalink raw reply

* Re: [PATCH 3/3] net: ethernet: sun4i-emac: Read rxhdr in CPU byte-order
From: Maxime Ripard @ 2016-11-14 18:59 UTC (permalink / raw)
  To: Michael Weiser; +Cc: netdev
In-Reply-To: <20161114175807.4747-4-michael.weiser@gmx.de>

[-- Attachment #1: Type: text/plain, Size: 1139 bytes --]

On Mon, Nov 14, 2016 at 06:58:07PM +0100, Michael Weiser wrote:
> The EMAC EMAC_RX_IO_DATA_REG data register is dual-purpose: On one hand
> it is used to move actual packet data off the wire. This will be in
> wire-format and accepted as such by higher layers such as IP. Therefore
> it is correctly read as-is (i.e. raw) using readsl.
> 
> On the other hand it provides metadata about incoming transfers to the
> driver such as length and checksum validation status. This data is
> little-endian, always and it is interpreted by the driver. Therefore it
> needs to be swapped to CPU endianness to make sense to the driver. This
> is already done for the "receive header" but not rxhdr.
> 
> Read rxhdr using readl in order for sun4i-emac to work correctly when
> running a big-endian kernel.
> 
> Signed-off-by: Michael Weiser <michael.weiser@gmx.de>
> Cc: Maxime Ripard <maxime.ripard@free-electrons.com>
> Cc: netdev@vger.kernel.org

Acked-by: Maxime Ripard <maxime.ripard@free-electrons.com>

Thanks!
Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

^ permalink raw reply

* Re: [PATCH net-next] bpf: fix range arithmetic for bpf map access
From: Josef Bacik @ 2016-11-14 18:52 UTC (permalink / raw)
  To: Alexei Starovoitov; +Cc: jannh, ast, daniel, davem, netdev
In-Reply-To: <20161112031338.GA86010@ast-mbp.thefacebook.com>

On 11/11/2016 10:13 PM, Alexei Starovoitov wrote:
> On Fri, Nov 11, 2016 at 04:47:39PM -0500, Josef Bacik wrote:
>> I made some invalid assumptions with BPF_AND and BPF_MOD that could result in
>> invalid accesses to bpf map entries.  Fix this up by doing a few things
>>
>> 1) Kill BPF_MOD support.  This doesn't actually get used by the compiler in real
>> life and just adds extra complexity.
>>
>> 2) Fix the logic for BPF_AND, don't allow AND of negative numbers and set the
>> minimum value to 0 for positive AND's.
>>
>> 3) Don't do operations on the ranges if they are set to the limits, as they are
>> by definition undefined, and allowing arithmetic operations on those values
>> could make them appear valid when they really aren't.
>>
>> This fixes the testcase provided by Jann as well as a few other theoretical
>> problems.
>>
>> Reported-by: Jann Horn <jannh@google.com>
>> Signed-off-by: Josef Bacik <jbacik@fb.com>
>> ---
>>  include/linux/bpf_verifier.h |  3 +-
>>  kernel/bpf/verifier.c        | 70 +++++++++++++++++++++++++++++---------------
>>  2 files changed, 49 insertions(+), 24 deletions(-)
>>
>> diff --git a/include/linux/bpf_verifier.h b/include/linux/bpf_verifier.h
>> index ac5b393..15ceb7f 100644
>> --- a/include/linux/bpf_verifier.h
>> +++ b/include/linux/bpf_verifier.h
>> @@ -22,7 +22,8 @@ struct bpf_reg_state {
>>  	 * Used to determine if any memory access using this register will
>>  	 * result in a bad access.
>>  	 */
>> -	u64 min_value, max_value;
>> +	s64 min_value;
>> +	u64 max_value;
>>  	u32 id;
>>  	union {
>>  		/* valid when type == CONST_IMM | PTR_TO_STACK | UNKNOWN_VALUE */
>> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
>> index 89f787c..709fe0e 100644
>> --- a/kernel/bpf/verifier.c
>> +++ b/kernel/bpf/verifier.c
>> @@ -234,8 +234,8 @@ static void print_verifier_state(struct bpf_verifier_state *state)
>>  				reg->map_ptr->value_size,
>>  				reg->id);
>>  		if (reg->min_value != BPF_REGISTER_MIN_RANGE)
>> -			verbose(",min_value=%llu",
>> -				(unsigned long long)reg->min_value);
>> +			verbose(",min_value=%lld",
>> +				(long long)reg->min_value);
>>  		if (reg->max_value != BPF_REGISTER_MAX_RANGE)
>>  			verbose(",max_value=%llu",
>>  				(unsigned long long)reg->max_value);
>> @@ -778,7 +778,7 @@ static int check_mem_access(struct bpf_verifier_env *env, u32 regno, int off,
>>  			 * index'es we need to make sure that whatever we use
>>  			 * will have a set floor within our range.
>>  			 */
>> -			if ((s64)reg->min_value < 0) {
>> +			if (reg->min_value < 0) {
>>  				verbose("R%d min value is negative, either use unsigned index or do a if (index >=0) check.\n",
>>  					regno);
>>  				return -EACCES;
>> @@ -1490,7 +1490,8 @@ static void check_reg_overflow(struct bpf_reg_state *reg)
>>  {
>>  	if (reg->max_value > BPF_REGISTER_MAX_RANGE)
>>  		reg->max_value = BPF_REGISTER_MAX_RANGE;
>> -	if ((s64)reg->min_value < BPF_REGISTER_MIN_RANGE)
>> +	if (reg->min_value < BPF_REGISTER_MIN_RANGE ||
>> +	    reg->min_value > BPF_REGISTER_MAX_RANGE)
>>  		reg->min_value = BPF_REGISTER_MIN_RANGE;
>>  }
>>
>> @@ -1498,7 +1499,8 @@ static void adjust_reg_min_max_vals(struct bpf_verifier_env *env,
>>  				    struct bpf_insn *insn)
>>  {
>>  	struct bpf_reg_state *regs = env->cur_state.regs, *dst_reg;
>> -	u64 min_val = BPF_REGISTER_MIN_RANGE, max_val = BPF_REGISTER_MAX_RANGE;
>> +	s64 min_val = BPF_REGISTER_MIN_RANGE;
>> +	u64 max_val = BPF_REGISTER_MAX_RANGE;
>>  	u8 opcode = BPF_OP(insn->code);
>>
>>  	dst_reg = &regs[insn->dst_reg];
>> @@ -1532,22 +1534,43 @@ static void adjust_reg_min_max_vals(struct bpf_verifier_env *env,
>>  		return;
>>  	}
>>
>> +	/* If one of our values was at the end of our ranges then we can't just
>> +	 * do our normal operations to the register, we need to set the values
>> +	 * to the min/max since they are undefined.
>> +	 */
>> +	if (min_val == BPF_REGISTER_MIN_RANGE)
>> +		dst_reg->min_value = BPF_REGISTER_MIN_RANGE;
>> +	if (max_val == BPF_REGISTER_MAX_RANGE)
>> +		dst_reg->max_value = BPF_REGISTER_MAX_RANGE;
>> +
>>  	switch (opcode) {
>>  	case BPF_ADD:
>> -		dst_reg->min_value += min_val;
>> -		dst_reg->max_value += max_val;
>> +		if (dst_reg->min_value != BPF_REGISTER_MIN_RANGE)
>> +			dst_reg->min_value += min_val;
>> +		if (dst_reg->max_value != BPF_REGISTER_MAX_RANGE)
>> +			dst_reg->max_value += max_val;
>>  		break;
>>  	case BPF_SUB:
>> -		dst_reg->min_value -= min_val;
>> -		dst_reg->max_value -= max_val;
>> +		if (dst_reg->min_value != BPF_REGISTER_MIN_RANGE)
>> +			dst_reg->min_value -= min_val;
>> +		if (dst_reg->max_value != BPF_REGISTER_MAX_RANGE)
>> +			dst_reg->max_value -= max_val;
>>  		break;
>>  	case BPF_MUL:
>> -		dst_reg->min_value *= min_val;
>> -		dst_reg->max_value *= max_val;
>> +		if (dst_reg->min_value != BPF_REGISTER_MIN_RANGE)
>> +			dst_reg->min_value *= min_val;
>
> looks to be few issues here with negative values as well.
> If dst_reg range [-2, 5] and right hand side range is [-2, 10],
> then above will be computed as -2 * -2 == 4
> but even if we do -1 * abs(dst_reg->min) * abs(min), it's still
> incorrect, since dst_reg could be 5 and multiplied by -2 (== -10),
> it will be less than above simple math on min values...
> so I'd suggest to disable negative values everywhere.
>
>> +		if (dst_reg->max_value != BPF_REGISTER_MAX_RANGE)
>> +			dst_reg->max_value *= max_val;
>>  		break;
>>  	case BPF_AND:
>> -		/* & is special since it could end up with 0 bits set. */
>> -		dst_reg->min_value &= min_val;
>> +		/* Disallow AND'ing of negative numbers, ain't nobody got time
>> +		 * for that.  Otherwise the minimum is 0 and the max is the max
>> +		 * value we could AND against.
>> +		 */
>> +		if (min_val < 0)
>> +			dst_reg->min_value = BPF_REGISTER_MIN_RANGE;
>> +		else
>> +			dst_reg->min_value = 0;
>>  		dst_reg->max_value = max_val;
>>  		break;
>>  	case BPF_LSH:
>> @@ -1557,24 +1580,25 @@ static void adjust_reg_min_max_vals(struct bpf_verifier_env *env,
>>  		 */
>>  		if (min_val > ilog2(BPF_REGISTER_MAX_RANGE))
>>  			dst_reg->min_value = BPF_REGISTER_MIN_RANGE;
>> -		else
>> +		else if (dst_reg->min_value != BPF_REGISTER_MIN_RANGE)
>>  			dst_reg->min_value <<= min_val;
>>
>>  		if (max_val > ilog2(BPF_REGISTER_MAX_RANGE))
>>  			dst_reg->max_value = BPF_REGISTER_MAX_RANGE;
>> -		else
>> +		else if (dst_reg->max_value != BPF_REGISTER_MAX_RANGE)
>>  			dst_reg->max_value <<= max_val;
>>  		break;
>>  	case BPF_RSH:
>> -		dst_reg->min_value >>= min_val;
>> -		dst_reg->max_value >>= max_val;
>> -		break;
>> -	case BPF_MOD:
>> -		/* % is special since it is an unsigned modulus, so the floor
>> -		 * will always be 0.
>> +		/* RSH by a negative number is undefined, and the BPF_RSH is an
>> +		 * unsigned shift, so make the appropriate casts.
>>  		 */
>> -		dst_reg->min_value = 0;
>> -		dst_reg->max_value = max_val - 1;
>> +		if (min_val < 0)
>> +			dst_reg->min_value = BPF_REGISTER_MIN_RANGE;
>> +		else if (dst_reg->min_value != BPF_REGISTER_MIN_RANGE)
>> +			dst_reg->min_value =
>> +				(u64)(dst_reg->min_value) >> min_val;
>
> when min_val is negative both >> and << are undefined,
> so we need to avoid negative values for these cases as well.
>
>> +		if (dst_reg->max_value != BPF_REGISTER_MAX_RANGE)
>> +			dst_reg->max_value >>= max_val;
>
> and for max_val too we need to make sure that max_val >= 0.

Well it's unsigned, so if somebody sets it to a negative value it'll be > 
BPF_REGISTER_MAX_RANGE and that'll get caught by the overflow logic above.

>
> To address all of it I'm thinking it will be easier to set
> BPF_REGISTER_MIN_RANGE to -1.
> I don't think we can kill tracking of min_val completely
> and assume valid min starts at zero, since we need either min
> tracking or boolean flag that indicates negative overflow and
> min tracking is imo cleaner (though valid min will always be >=0
> and invalid min is -1)
>
> Also this patch has to go to 'net' tree, so rebasing with net-next
> wasn't necessary.
>

Yeah I'm fine with killing negative values altogether, it does seem a bit silly 
to support it and isn't likely to be used in any sort of normal scenario.  Thanks,

Josef

^ permalink raw reply

* Re: [PATCH net 2/3] bpf, mlx5: fix various refcount/prog issues in mlx5e_xdp_set
From: Saeed Mahameed @ 2016-11-14 18:27 UTC (permalink / raw)
  To: Daniel Borkmann, davem
  Cc: alexei.starovoitov, bblanco, tariqt, zhiyisun, ranas, netdev
In-Reply-To: <03741f7075af64e83d23add379bdab41204396b0.1479080215.git.daniel@iogearbox.net>



On 11/14/2016 02:43 AM, Daniel Borkmann wrote:
> There are multiple issues in mlx5e_xdp_set():
> 
> 1) prog can be NULL, so calling unconditionally into bpf_prog_add(prog,
>    priv->params.num_channels) can end badly.

not correct, if prog is null we will never get to bpf_prog_add:

        reset = (!priv->xdp_prog || !prog);
        [...]
	if (!test_bit(MLX5E_STATE_OPENED, &priv->state) || reset)
		goto unlock;
        bpf_prog_add...
         

> 
> 2) The batched bpf_prog_add() should be done at an earlier point in
>    time. This makes sure that we cannot fail anymore at the time we
>    want to set the program for each channel. This only means that we
>    have to undo the bpf_prog_add() in case we return early due to
>    reset or device not in MLX5E_STATE_OPENED yet. Note, err is 0 here.
> 

It is delayed for a reason, we do delayed batched bpf_prog_add() 
only when reset is not required (exchanging prog/old_prg) when both prog and old_prog are not null,
which means the only thing that could fail in this case is bpf_prog_add.

so i don't see any reason for changing the logic, checking for  bpf_prog_add return value would be sufficient.

Sorry I need to go for now, I will continue reviewing this patch later.  but this patch looks a little bit exaggerated.

> 3) When swapping the priv->xdp_prog, then no extra reference count must
>    be taken since we got that from call path via dev_change_xdp_fd()
>    already. Otherwise, we'd never be able to free the program. Also,
>    bpf_prog_add() without checking the return code could fail.
> 
> Fixes: 86994156c736 ("net/mlx5e: XDP fast RX drop bpf programs support")
> Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
> ---
>  drivers/net/ethernet/mellanox/mlx5/core/en_main.c | 25 ++++++++++++++++++-----
>  include/linux/bpf.h                               |  5 +++++
>  kernel/bpf/syscall.c                              | 11 ++++++++++
>  3 files changed, 36 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
> index 2b83667..c90610a 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
> @@ -3125,6 +3125,17 @@ static int mlx5e_xdp_set(struct net_device *netdev, struct bpf_prog *prog)
>  		goto unlock;
>  	}
>  
> +	if (prog) {
> +		/* num_channels is invariant here, so we can take the
> +		 * batched reference right upfront.
> +		 */
> +		prog = bpf_prog_add(prog, priv->params.num_channels);
> +		if (IS_ERR(prog)) {
> +			err = PTR_ERR(prog);
> +			goto unlock;
> +		}
> +	}
> +
>  	was_opened = test_bit(MLX5E_STATE_OPENED, &priv->state);
>  	/* no need for full reset when exchanging programs */
>  	reset = (!priv->xdp_prog || !prog);
> @@ -3132,10 +3143,10 @@ static int mlx5e_xdp_set(struct net_device *netdev, struct bpf_prog *prog)
>  	if (was_opened && reset)
>  		mlx5e_close_locked(netdev);
>  
> -	/* exchange programs */
> +	/* exchange programs, extra prog reference we got from caller
> +	 * as long as we don't fail from this point onwards.
> +	 */
>  	old_prog = xchg(&priv->xdp_prog, prog);
> -	if (prog)
> -		bpf_prog_add(prog, 1);
>  	if (old_prog)
>  		bpf_prog_put(old_prog);
>  
> @@ -3146,12 +3157,11 @@ static int mlx5e_xdp_set(struct net_device *netdev, struct bpf_prog *prog)
>  		mlx5e_open_locked(netdev);
>  
>  	if (!test_bit(MLX5E_STATE_OPENED, &priv->state) || reset)
> -		goto unlock;
> +		goto unlock_put;
>  
>  	/* exchanging programs w/o reset, we update ref counts on behalf
>  	 * of the channels RQs here.
>  	 */
> -	bpf_prog_add(prog, priv->params.num_channels);
>  	for (i = 0; i < priv->params.num_channels; i++) {
>  		struct mlx5e_channel *c = priv->channel[i];
>  
> @@ -3173,6 +3183,11 @@ static int mlx5e_xdp_set(struct net_device *netdev, struct bpf_prog *prog)
>  unlock:
>  	mutex_unlock(&priv->state_lock);
>  	return err;
> +unlock_put:
> +	/* reference on priv->xdp_prog is still held at this point */
> +	if (prog)
> +		bpf_prog_sub(prog, priv->params.num_channels);
> +	goto unlock;
>  }
>  
>  static bool mlx5e_xdp_attached(struct net_device *dev)
> diff --git a/include/linux/bpf.h b/include/linux/bpf.h
> index c201017..ca495fd 100644
> --- a/include/linux/bpf.h
> +++ b/include/linux/bpf.h
> @@ -234,6 +234,7 @@ u64 bpf_event_output(struct bpf_map *map, u64 flags, void *meta, u64 meta_size,
>  struct bpf_prog *bpf_prog_get(u32 ufd);
>  struct bpf_prog *bpf_prog_get_type(u32 ufd, enum bpf_prog_type type);
>  struct bpf_prog *bpf_prog_add(struct bpf_prog *prog, int i);
> +void bpf_prog_sub(struct bpf_prog *prog, int i);
>  struct bpf_prog *bpf_prog_inc(struct bpf_prog *prog);
>  void bpf_prog_put(struct bpf_prog *prog);
>  
> @@ -303,6 +304,10 @@ static inline struct bpf_prog *bpf_prog_add(struct bpf_prog *prog, int i)
>  	return ERR_PTR(-EOPNOTSUPP);
>  }
>  
> +static inline void bpf_prog_sub(struct bpf_prog *prog, int i)
> +{
> +}
> +
>  static inline void bpf_prog_put(struct bpf_prog *prog)
>  {
>  }
> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
> index 751e806..a0fca9f 100644
> --- a/kernel/bpf/syscall.c
> +++ b/kernel/bpf/syscall.c
> @@ -682,6 +682,17 @@ struct bpf_prog *bpf_prog_add(struct bpf_prog *prog, int i)
>  }
>  EXPORT_SYMBOL_GPL(bpf_prog_add);
>  
> +void bpf_prog_sub(struct bpf_prog *prog, int i)
> +{
> +	/* Only to be used for undoing previous bpf_prog_add() in some
> +	 * error path. We still know that another entity in our call
> +	 * path holds a reference to the program, thus atomic_sub() can
> +	 * be safely used in such cases!
> +	 */
> +	WARN_ON(atomic_sub_return(i, &prog->aux->refcnt) == 0);
> +}
> +EXPORT_SYMBOL_GPL(bpf_prog_sub);
> +
>  struct bpf_prog *bpf_prog_inc(struct bpf_prog *prog)
>  {
>  	return bpf_prog_add(prog, 1);
> 

^ permalink raw reply

* Re: [PATCH v2 net-next 3/6] bpf: Refactor codes handling percpu map
From: Alexei Starovoitov @ 2016-11-14 18:43 UTC (permalink / raw)
  To: Martin KaFai Lau
  Cc: netdev, David Miller, Alexei Starovoitov, Daniel Borkmann,
	Kernel Team
In-Reply-To: <1478890511-1346984-4-git-send-email-kafai@fb.com>

On Fri, Nov 11, 2016 at 10:55:08AM -0800, Martin KaFai Lau wrote:
> Refactor the codes that populate the value
> of a htab_elem in a BPF_MAP_TYPE_PERCPU_HASH
> typed bpf_map.
> 
> Signed-off-by: Martin KaFai Lau <kafai@fb.com>

Acked-by: Alexei Starovoitov <ast@kernel.org>

^ permalink raw reply

* Re: Debugging Ethernet issues
From: Florian Fainelli @ 2016-11-14 18:42 UTC (permalink / raw)
  To: Sebastian Frias, Mason, Andrew Lunn
  Cc: netdev, Mans Rullgard, Sergei Shtylyov, Tom Lendacky, Zach Brown,
	Shaohui Xie, Tim Beale, Brian Hill, Vince Bridgers,
	Balakumaran Kannan, David S. Miller, Kirill Kapranov
In-Reply-To: <2187db98-dc5a-7a3c-7965-7ccbeffc0fa1@gmail.com>

On 11/14/2016 10:20 AM, Florian Fainelli wrote:
> On 11/14/2016 09:59 AM, Sebastian Frias wrote:
>> On 11/14/2016 06:32 PM, Florian Fainelli wrote:
>>> On 11/14/2016 07:33 AM, Mason wrote:
>>>> On 14/11/2016 15:58, Mason wrote:
>>>>
>>>>> nb8800 26000.ethernet eth0: Link is Up - 100Mbps/Full - flow control rx/tx
>>>>> vs
>>>>> nb8800 26000.ethernet eth0: Link is Up - 100Mbps/Full - flow control off
>>>>>
>>>>> I'm not sure whether "flow control" is relevant...
>>>>
>>>> Based on phy_print_status()
>>>> phydev->pause ? "rx/tx" : "off"
>>>> I added the following patch.
>>>>
>>>> diff --git a/drivers/net/ethernet/aurora/nb8800.c b/drivers/net/ethernet/aurora/nb8800.c
>>>> index defc22a15f67..4e758c1cfa4e 100644
>>>> --- a/drivers/net/ethernet/aurora/nb8800.c
>>>> +++ b/drivers/net/ethernet/aurora/nb8800.c
>>>> @@ -667,6 +667,8 @@ static void nb8800_link_reconfigure(struct net_device *dev)
>>>>         struct phy_device *phydev = priv->phydev;
>>>>         int change = 0;
>>>>  
>>>> +       printk("%s from %pf\n", __func__, __builtin_return_address(0));
>>>> +
>>>>         if (phydev->link) {
>>>>                 if (phydev->speed != priv->speed) {
>>>>                         priv->speed = phydev->speed;
>>>> @@ -1274,9 +1276,9 @@ static int nb8800_hw_init(struct net_device *dev)
>>>>         nb8800_writeb(priv, NB8800_PQ2, val & 0xff);
>>>>  
>>>>         /* Auto-negotiate by default */
>>>> -       priv->pause_aneg = true;
>>>> -       priv->pause_rx = true;
>>>> -       priv->pause_tx = true;
>>>> +       priv->pause_aneg = false;
>>>> +       priv->pause_rx = false;
>>>> +       priv->pause_tx = false;
>>>>  
>>>>         nb8800_mc_init(dev, 0);
>>>>  
>>>>
>>>> Connected to 1000 Mbps switch:
>>>>
>>>> # time udhcpc | while read LINE; do date; echo $LINE; done
>>>> Thu Jan  1 00:00:22 UTC 1970
>>>> udhcpc (v1.22.1) started
>>>> Thu Jan  1 00:00:22 UTC 1970
>>>> Sending discover...
>>>> [   24.565346] nb8800_link_reconfigure from phy_state_machine
>>>> Thu Jan  1 00:00:25 UTC 1970
>>>> Sending discover...
>>>> [   26.575402] nb8800_link_reconfigure from phy_state_machine
>>>> [   26.580972] nb8800 26000.ethernet eth0: Link is Up - 1Gbps/Full - flow control rx/tx
>>>> Thu Jan  1 00:00:28 UTC 1970
>>>> Sending discover...
>>>> Thu Jan  1 00:00:29 UTC 1970
>>>> Sending select for 172.27.64.58...
>>>> Thu Jan  1 00:00:29 UTC 1970
>>>> Lease of 172.27.64.58 obtained, lease time 604800
>>>> Thu Jan  1 00:00:29 UTC 1970
>>>> deleting routers
>>>> Thu Jan  1 00:00:29 UTC 1970
>>>> adding dns 172.27.0.17
>>>>
>>>> real    0m7.388s
>>>> user    0m0.040s
>>>> sys     0m0.090s
>>>>
>>>>
>>>>
>>>> Connected to 100 Mbps switch:
>>>>
>>>> # time udhcpc | while read LINE; do date; echo $LINE; done
>>>> Thu Jan  1 00:00:14 UTC 1970
>>>> udhcpc (v1.22.1) started
>>>> Thu Jan  1 00:00:15 UTC 1970
>>>> Sending discover...
>>>> [   16.968621] nb8800_link_reconfigure from phy_state_machine
>>>> [   17.975359] nb8800_link_reconfigure from phy_state_machine
>>>> [   17.980923] nb8800 26000.ethernet eth0: Link is Up - 100Mbps/Full - flow control rx/tx
>>>> Thu Jan  1 00:00:18 UTC 1970
>>>> Sending discover...
>>>> Thu Jan  1 00:00:19 UTC 1970
>>>> Sending select for 172.27.64.58...
>>>> Thu Jan  1 00:00:19 UTC 1970
>>>> Lease of 172.27.64.58 obtained, lease time 604800
>>>> Thu Jan  1 00:00:19 UTC 1970
>>>> deleting routers
>>>> Thu Jan  1 00:00:19 UTC 1970
>>>> adding dns 172.27.0.17
>>>>
>>>> real    0m4.355s
>>>> user    0m0.043s
>>>> sys     0m0.083s
>>>>
>>>
>>> And the time difference is clearly accounted for auto-negotiation time
>>> here, as you can see it takes about 3 seconds for Gigabit Ethernet to
>>> auto-negotiate and that seems completely acceptable and normal to me
>>> since it is a more involved process than lower speeds.
>>>
>>>>
>>>>
>>>> OK, so now it works (by accident?) even on 100 Mbps switch, but it still
>>>> prints "flow control rx/tx"...
>>>
>>> Because your link partner advertises flow control, and that's what
>>> phydev->pause and phydev->asym_pause report (I know it's confusing, but
>>> that's what it is at the moment).
>>
>> Thanks.
>> Could you confirm that Mason's patch is correct and/or that it does not
>> has negative side-effects?
> 
> The patch is not correct nor incorrect per-se, it changes the default
> policy of having pause frames advertised by default to not having them
> advertised by default. This influences both your Ethernet MAC and the
> link partner in that the result is either flow control is enabled
> (before) or it is not (with the patch). There must be something amiss if
> you see packet loss or some kind of problem like that with an early
> exchange such as DHCP. Flow control tend to kick in under higher packet
> rates (at least, that's what you expect).
> 
> 
>>
>> Right now we know that Mason's patch makes this work, but we do not understand
>> why nor its implications.
> 
> You need to understand why, right now, the way this problem is
> presented, you came up with a workaround, not with the root cause or the
> solution. What does your link partner (switch?) reports, that is, what
> is the ethtool output when you have a link up from  your nb8800 adapter?

Actually, nb8800_pause_config() seems to be doing a complete MAC/DMA
reconfiguration when pause frames get auto-negotiated while the link is
UP, and it does not differentiate being called from
ethtool::set_pauseparam or the PHYLIB adjust_link callback (which it
probably should), wondering if there is a not a remote chance you can
get the reply to arrive right when you just got signaled a link UP?
-- 
Florian

^ permalink raw reply

* Re: [PATCH] ps3_gelic: fix spelling mistake in debug message
From: David Miller @ 2016-11-14 18:39 UTC (permalink / raw)
  To: colin.king
  Cc: benh, paulus, mpe, falakreyaz, christophe.jaillet, jarod, netdev,
	linuxppc-dev, linux-kernel
In-Reply-To: <20161112172030.7583-1-colin.king@canonical.com>

From: Colin King <colin.king@canonical.com>
Date: Sat, 12 Nov 2016 17:20:30 +0000

> From: Colin Ian King <colin.king@canonical.com>
> 
> Trivial fix to spelling mistake "unmached" to "unmatched" in
> debug message.
> 
> Signed-off-by: Colin Ian King <colin.king@canonical.com>

Applied.

^ permalink raw reply

* Re: [PATCH] net: atheros: atl2: use new api ethtool_{get|set}_link_ksettings
From: David Miller @ 2016-11-14 18:38 UTC (permalink / raw)
  To: tremyfr; +Cc: jcliburn, chris.snook, jarod, ben, netdev, linux-kernel
In-Reply-To: <1479059608-32456-1-git-send-email-tremyfr@gmail.com>

From: Philippe Reynes <tremyfr@gmail.com>
Date: Sun, 13 Nov 2016 18:53:28 +0100

> The ethtool api {get|set}_settings is deprecated.
> We move this driver to new api {get|set}_link_ksettings.
> 
> The previous implementation of set_settings was modifying
> the value of advertising, but with the new API, it's not
> possible. The structure ethtool_link_ksettings is defined
> as const.
> 
> Signed-off-by: Philippe Reynes <tremyfr@gmail.com>

Applied.

^ permalink raw reply

* Re: [PATCH] net: atheros: atl1: use new api ethtool_{get|set}_link_ksettings
From: David Miller @ 2016-11-14 18:38 UTC (permalink / raw)
  To: tremyfr; +Cc: jcliburn, chris.snook, jarod, netdev, linux-kernel
In-Reply-To: <1479058514-22438-1-git-send-email-tremyfr@gmail.com>

From: Philippe Reynes <tremyfr@gmail.com>
Date: Sun, 13 Nov 2016 18:35:14 +0100

> The ethtool api {get|set}_settings is deprecated.
> We move this driver to new api {get|set}_link_ksettings.
> 
> The previous implementation of set_settings was modifying
> the value of advertising, but with the new API, it's not
> possible. The structure ethtool_link_ksettings is defined
> as const.
> 
> Signed-off-by: Philippe Reynes <tremyfr@gmail.com>

Applied.

^ permalink raw reply

* Re: [PATCH] net: atheros: atl1c: use new api ethtool_{get|set}_link_ksettings
From: David Miller @ 2016-11-14 18:38 UTC (permalink / raw)
  To: tremyfr; +Cc: jcliburn, chris.snook, netdev, linux-kernel
In-Reply-To: <1478968369-25034-1-git-send-email-tremyfr@gmail.com>

From: Philippe Reynes <tremyfr@gmail.com>
Date: Sat, 12 Nov 2016 17:32:49 +0100

> The ethtool api {get|set}_settings is deprecated.
> We move this driver to new api {get|set}_link_ksettings.
> 
> Signed-off-by: Philippe Reynes <tremyfr@gmail.com>

Applied.

^ permalink raw reply

* Re: [PATCH] net: alx: use new api ethtool_{get|set}_link_ksettings
From: David Miller @ 2016-11-14 18:38 UTC (permalink / raw)
  To: tremyfr; +Cc: jcliburn, chris.snook, netdev, linux-kernel
In-Reply-To: <1478903437-9049-1-git-send-email-tremyfr@gmail.com>

From: Philippe Reynes <tremyfr@gmail.com>
Date: Fri, 11 Nov 2016 23:30:37 +0100

> The ethtool api {get|set}_settings is deprecated.
> We move this driver to new api {get|set}_link_ksettings.
> 
> Signed-off-by: Philippe Reynes <tremyfr@gmail.com>

Applied.

^ permalink raw reply

* Re: [PATCH] icmp: Restore resistence to abnormal messages
From: David Miller @ 2016-11-14 18:36 UTC (permalink / raw)
  To: googuy; +Cc: kuznet, jmorris, yoshfuji, kaber, netdev, linux-kernel
In-Reply-To: <20161111202018.13795-1-googuy@gmail.com>

From: Vicente Jimenez Aguilar <googuy@gmail.com>
Date: Fri, 11 Nov 2016 21:20:18 +0100

> @@ -819,6 +820,12 @@ static bool icmp_unreach(struct sk_buff *skb)
>  				/* fall through */
>  			case 0:
>  				info = ntohs(icmph->un.frag.mtu);
> +				/* Handle weird case where next hop MTU is
> +				 * equal to or exceeding dropped packet size
> +				 */
> +				old_mtu = ntohs(iph->tot_len);
> +				if (info >= old_mtu)
> +					info = old_mtu - 2;

This isn't something the old code did.

The old code behaved much differently.

In the case where the new mtu was smaller than 68 or larger than
the iph->tot_len value, it would do several things:

1) First it would check for a BSD 4.2 anomaly and subtract old_mtu
   by the IP header length.

2) Second, it would try to guess the intended MTU using the
   mtu_plateau table.

I don't see any code where a subtraction by a fixed constant of 2
occurred.

Nor can I figure out what that might accomplish.  If you really
want to do this, you have to docuement what this 2 means, what
it is accomplishing, and why you have choosen to accomplish it
this way.

Thanks.

^ permalink raw reply

* Re: [PATCH] [v2] net: phy: phy drivers should not set SUPPORTED_[Asym_]Pause
From: Florian Fainelli @ 2016-11-14 18:35 UTC (permalink / raw)
  To: Timur Tabi, David Miller, jon.mason, netdev, Andrew Lunn
In-Reply-To: <1478821561-26498-1-git-send-email-timur@codeaurora.org>

On 11/10/2016 03:46 PM, Timur Tabi wrote:
> Instead of having individual PHY drivers set the SUPPORTED_Pause and
> SUPPORTED_Asym_Pause flags, phylib itself should set those flags.
> During autonegotiation, the PHYs will determine whether to enable
> pause frame support.
> 
> Pause frames are a feature that is supported by the MAC.  It is the MAC
> that generates the frames and that processes them.  The PHY can only be
> configured to allow them to pass through.
> 
> So the new process is:
> 
> 1) Phylib sets the SUPPORTED_Pause and SUPPORTED_AsymPause bits in
> phydev->supported.  This indicates that the PHY supports pause frames.
> 
> 2) The MAC driver checks phydev->supported before it calls phy_start().
> If (SUPPORTED_Pause | SUPPORTED_AsymPause) is set, then the MAC driver
> sets those bits in phydev->advertising, if it wants to enable pause
> frame support.
> 
> 3) When the link state changes, the MAC driver checks phydev->pause and
> phydev->asym_pause,  If the bits are set, then it enables the corresponding
> features in the MAC.  The algorithm is:
> 
> 	if (phydev->pause)
> 		The MAC should be programmed to receive and honor
>                 pause frames it receives, i.e. enable receive flow control.
> 
> 	if (phydev->pause != phydev->asym_pause)
> 		The MAC should be programmed to transmit pause
> 		frames when needed, i.e. enable transmit flow control.
> 
> Signed-off-by: Timur Tabi <timur@codeaurora.org>
> ---

> diff --git a/drivers/net/phy/bcm63xx.c b/drivers/net/phy/bcm63xx.c
> index e741bf6..5e9922e 100644
> --- a/drivers/net/phy/bcm63xx.c
> +++ b/drivers/net/phy/bcm63xx.c
> @@ -48,8 +48,7 @@ static int bcm63xx_config_init(struct phy_device *phydev)
>  	.phy_id		= 0x00406000,
>  	.phy_id_mask	= 0xfffffc00,
>  	.name		= "Broadcom BCM63XX (1)",
> -	/* ASYM_PAUSE bit is marked RO in datasheet, so don't cheat */
> -	.features	= (PHY_BASIC_FEATURES | SUPPORTED_Pause),
> +	.features	= PHY_BASIC_FEATURES,

Humm that's actually a pretty important piece of information here that
we are going to lose if we unconditionally move the setting of the
SUPPORTED_Pause/Asym_Pause settings into the core. I don't have the HW
in a state where I could try a mainline kernel, but I suspect that the
following could happen though:

- we would try to set the SUPPORTED_AsymPause bit, and it would not be
taken into account, since the bit is RO
- the auto-negotiation results should still show up as symmetric pause
being supported only
- the driver would properly react to that

NB: this also applies to drivers/net/phy/ste10Xp.c.

So maybe, for theses drivers specifically, what we can do, is preserve
the entry as-is, to convey that only symmetric Pause frames can be
advertised, and have the logic in PHYLIB do something like this
(pseudo-code):

if (!(drv->features & (SUPPORTED_Pause | SUPPORTED_AsymPause))
	phydev->supported |= SUPPORTED_Pause | SUPPORTED_AsymPause;
else if ((drv->features & (SUPPORTED_Pause) && (!(drv->features &
(SUPPORTED_AsymPause)))
	phydev->supported |= SUPPORTED_Pause;

(there may be more efficient ways to do this of course).
-- 
Florian

^ permalink raw reply

* Re: [PATCH v3] ip6_output: ensure flow saddr actually belongs to device
From: Hannes Frederic Sowa @ 2016-11-14 18:33 UTC (permalink / raw)
  To: David Ahern, Jason A. Donenfeld, Netdev, WireGuard mailing list,
	LKML, YOSHIFUJI Hideaki
In-Reply-To: <0214eaf8-70c6-5a37-cddd-faa1c4268871@cumulusnetworks.com>

On Mon, Nov 14, 2016, at 18:48, David Ahern wrote:
> On 11/14/16 10:33 AM, Hannes Frederic Sowa wrote:
> >>>>> I just also quickly read up on the history (sorry was travelling last
> >>>>> week) and wonder if you ever saw a user space facing bug or if this is
> >>>>> basically some difference you saw while writing out of tree code?
> >>>>
> >>>> I checked the userspace API this morning. bind and cmsg for example check that the address is valid with calls to ipv6_chk_addr.
> >>>
> >>> Hmm, so it fixes no real bug.
> >>>
> >>> Because of translations of flowi6_oif we actually can't do a correct
> >>> check of source address for cases like the one I outlined above? Hmm,
> >>> maybe we should simply depend on user space checks.
> >>
> >> I believe Jason's case is forwarding path and the ipv6_stub->ipv6_dst_lookup API.
> > 
> > It is not a kernel API, because we don't support something like that for
> > external kernel modules. We basically exported ipv6_dst_lookup to allow
> > some IPv4 code to do ipv6 stunts when the IPv6 module is loaded. ;)
> 
> ???
> 
> ipv6_stub is exported for modules (EXPORT_SYMBOL_GPL(ipv6_stub)).
> 
> ipv6_stub->ipv6_dst_lookup is used by several modules -- geneve, tipc,
> vxlan, mpls -- for IPv6 lookups, not IPv4 code do IPv6 stunts.
> 
> So how do you say that is not an exported kernel API?

Sorry, yes, I noticed I wrote it in a confusing way.

I meant to say, we don't require the IPv6 "API" to behave in a similar
way like the IPv4 one. We do this function pointer trick to allow
_in-kernel_ tree modules to use the function dynamically, even the
kernel ipv6 module would be available but is not loaded but don't
guarante any "API like IPv4" to outside tree modules.

I tried to make the point, that it is still something internal to the
kernel if compared to out-of-tree function users. And that different
behavior by itself doesn't count as a bug.

We could as well require the users of this function to check for the
source address before or require to check the source address after the
ipv6_dst_lookup call.

vxlan currently seems wrong and would impacted by this patch in a better
way, so I am all in for such a change, but I think we need to check if
we are also correct scope-wise and not just match for the address on its
own.

Thanks,
Hannes

^ permalink raw reply

* Re: [PATCH net 1/3] bpf, mlx5: fix mlx5e_create_rq taking reference on prog
From: Daniel Borkmann @ 2016-11-14 18:26 UTC (permalink / raw)
  To: Saeed Mahameed, davem
  Cc: alexei.starovoitov, bblanco, tariqt, zhiyisun, ranas, netdev
In-Reply-To: <918902f3-1852-ae68-b12d-eaa1c45bf641@mellanox.com>

Hi Saeed,

On 11/14/2016 07:15 PM, Saeed Mahameed wrote:
> On 11/14/2016 02:43 AM, Daniel Borkmann wrote:
>> In mlx5e_create_rq(), when creating a new queue, we call bpf_prog_add() but
>> without checking the return value. bpf_prog_add() can fail, so we really
>
> Didn't know this, thanks for noticing, I wonder why taking a reference for an object would fail ?
> especially when someone is requesting from the driver to take a reference to it ndo_xdp_set ?! sounds like a bad design.
>
> Anyway I will check that later.

See 92117d8443bc ("bpf: fix refcnt overflow").

>> must check it. Take the reference right when we assign it to the rq from
>> priv->xdp_prog, and just drop the reference on error path. Destruction in
>> mlx5e_destroy_rq() looks good, though.
>>
>> Fixes: 86994156c736 ("net/mlx5e: XDP fast RX drop bpf programs support")
>> Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
>> ---
>>   drivers/net/ethernet/mellanox/mlx5/core/en_main.c | 14 +++++++++++---
>>   kernel/bpf/syscall.c                              |  1 +
>>   2 files changed, 12 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
>> index 84e8b25..2b83667 100644
>> --- a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
>> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
>> @@ -489,7 +489,16 @@ static int mlx5e_create_rq(struct mlx5e_channel *c,
>>   	rq->channel = c;
>>   	rq->ix      = c->ix;
>>   	rq->priv    = c->priv;
>> +
>>   	rq->xdp_prog = priv->xdp_prog;
>
> Why keeping this assignment ? just test priv->xdp_prog.
>
>> +	if (rq->xdp_prog) {
>> +		rq->xdp_prog = bpf_prog_inc(rq->xdp_prog);
>> +		if (IS_ERR(rq->xdp_prog)) {
>> +			err = PTR_ERR(rq->xdp_prog);
>> +			rq->xdp_prog = NULL;
>> +			goto err_rq_wq_destroy;
>> +		}
>> +	}
>
> Try this, simpler and less indentations:
>
> rq->xdp_prog = priv->xdp_prog ? bpf_prog_inc(priv->xdp_prog) : NULL;
> if (IS_ERR(rq->xdp_prog)) {
> 	err = PTR_ERR(rq->xdp_prog);
> 	rq->xdp_prog = NULL;
> 	goto err_rq_wq_destroy;
> }

Sure, I don't mind. Will do.

Thanks,
Daniel

^ permalink raw reply

* Re: [PATCH net 2/3] bpf, mlx5: fix various refcount/prog issues in mlx5e_xdp_set
From: Daniel Borkmann @ 2016-11-14 18:23 UTC (permalink / raw)
  To: Alexei Starovoitov; +Cc: davem, bblanco, tariqt, zhiyisun, ranas, netdev
In-Reply-To: <20161114173525.GA98186@ast-mbp.thefacebook.com>

On 11/14/2016 06:35 PM, Alexei Starovoitov wrote:
> On Mon, Nov 14, 2016 at 09:49:49AM +0100, Daniel Borkmann wrote:
>> On 11/14/2016 03:49 AM, Alexei Starovoitov wrote:
>>> On Mon, Nov 14, 2016 at 01:43:41AM +0100, Daniel Borkmann wrote:
>> [...]
>>>> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
>>>> index 751e806..a0fca9f 100644
>>>> --- a/kernel/bpf/syscall.c
>>>> +++ b/kernel/bpf/syscall.c
>>>> @@ -682,6 +682,17 @@ struct bpf_prog *bpf_prog_add(struct bpf_prog *prog, int i)
>>>>   }
>>>>   EXPORT_SYMBOL_GPL(bpf_prog_add);
>>>>
>>>> +void bpf_prog_sub(struct bpf_prog *prog, int i)
>>>> +{
>>>> +	/* Only to be used for undoing previous bpf_prog_add() in some
>>>> +	 * error path. We still know that another entity in our call
>>>> +	 * path holds a reference to the program, thus atomic_sub() can
>>>> +	 * be safely used in such cases!
>>>> +	 */
>>>> +	WARN_ON(atomic_sub_return(i, &prog->aux->refcnt) == 0);
>>>> +}
>>>> +EXPORT_SYMBOL_GPL(bpf_prog_sub);
>>>
>>> the patches look good. I'm only worried about net/net-next merge
>>> conflict here. (I would have to deal with it as well).
>>> So instead of copying the above helper can we apply net-next's
>>> 'bpf, mlx4: fix prog refcount in mlx4_en_try_alloc_resources error path'
>>> patch to net without mlx4_xdp_set hunk and then apply
>>> the rest of this patch?
>>> Even better is to send this patch 2/3 to net-next?
>>> yes, it's an issue, but very small one. There is no security
>>> concern here, so I would prefer to avoid merge conflict.
>>> Did you do a test merge of net/net-next by any chance?
>>
>> Yes, I did a test merge and git resolved the above just fine w/o
>> any conflicts. I have no strong opinion whether net or net-next.
>> If preferred, I can just resend this series in the evening against
>> net-next instead, perhaps that's a bit better.
>
> I have slight preference to go via net-next, but since it merges fine,
> I don't mind net route too.

Ok, I'll rebase for net-next then.

^ 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