Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH net-next 04/12] net: sched: flower: track filter deletion with flag
From: Stefano Brivio @ 2019-02-14 20:49 UTC (permalink / raw)
  To: Vlad Buslov; +Cc: netdev, jhs, xiyou.wangcong, jiri, davem
In-Reply-To: <20190214074712.17846-5-vladbu@mellanox.com>

On Thu, 14 Feb 2019 09:47:04 +0200
Vlad Buslov <vladbu@mellanox.com> wrote:

> +static int __fl_delete(struct tcf_proto *tp, struct cls_fl_filter *f,
> +		       bool *last, struct netlink_ext_ack *extack)

This would be easier to follow (at least for me):

>  {
>  	struct cls_fl_head *head = fl_head_dereference(tp);
>  	bool async = tcf_exts_get_net(&f->exts);
> -	bool last;
> -
> -	idr_remove(&head->handle_idr, f->handle);
> -	list_del_rcu(&f->list);
> -	last = fl_mask_put(head, f->mask, async);
> -	if (!tc_skip_hw(f->flags))
> -		fl_hw_destroy_filter(tp, f, extack);
> -	tcf_unbind_filter(tp, &f->res);
> -	__fl_put(f);
> +	int err = 0;

without this

> +
> +	(*last) = false;

with *last = false;

> +
> +	if (!f->deleted) {

with:
	if (f->deleted)
		return -ENOENT;

> +		f->deleted = true;
> +		rhashtable_remove_fast(&f->mask->ht, &f->ht_node,
> +				       f->mask->filter_ht_params);
> +		idr_remove(&head->handle_idr, f->handle);
> +		list_del_rcu(&f->list);
> +		(*last) = fl_mask_put(head, f->mask, async);

with:
	*last = fl_mask_put(head, f->mask, async);

> +		if (!tc_skip_hw(f->flags))
> +			fl_hw_destroy_filter(tp, f, extack);
> +		tcf_unbind_filter(tp, &f->res);
> +		__fl_put(f);

and a return 0; here

> +	} else {
> +		err = -ENOENT;
> +	}
>  
> -	return last;
> +	return err;
>  }
>  
> [...]
>
> @@ -1520,14 +1541,14 @@ static int fl_delete(struct tcf_proto *tp, void *arg, bool *last,
>  {
>  	struct cls_fl_head *head = fl_head_dereference(tp);
>  	struct cls_fl_filter *f = arg;
> +	bool last_on_mask;

This is unused in this series, maybe change __fl_delete() to optionally
take NULL as 'bool *last' argument?

> +	int err = 0;

Nit: no need to initialise this.
 
> -	rhashtable_remove_fast(&f->mask->ht, &f->ht_node,
> -			       f->mask->filter_ht_params);
> -	__fl_delete(tp, f, extack);
> +	err = __fl_delete(tp, f, &last_on_mask, extack);
>  	*last = list_empty(&head->masks);
>  	__fl_put(f);
>  
> -	return 0;
> +	return err;
>  }
>  
>  static void fl_walk(struct tcf_proto *tp, struct tcf_walker *arg,

-- 
Stefano

^ permalink raw reply

* Have needs for?
From: Cindy @ 2019-02-11  8:59 UTC (permalink / raw)
  To: netdev

Do you have needs for retouching your photos? Or do deep etching and
masking for your photos,

We are the image service provider who can do this for you.

Please send photos to start testing, then you cam judge the quality of our
service.

Thanks,
Cindy
















Fuldad


Haldberstadt


^ permalink raw reply

* [PATCH net-next 0/2] net: phy: improve and use phy_resolve_aneg_linkmode
From: Heiner Kallweit @ 2019-02-14 21:12 UTC (permalink / raw)
  To: Andrew Lunn, Florian Fainelli, David Miller; +Cc: netdev@vger.kernel.org

Improve phy_resolve_aneg_linkmode and use it in genphy_read_status.

Heiner Kallweit (2):
  net: phy: improve phy_resolve_aneg_linkmode
  net: phy: use phy_resolve_aneg_linkmode in genphy_read_status

 drivers/net/phy/phy-core.c   | 43 ++++++------------------------------
 drivers/net/phy/phy_device.c | 24 +-------------------
 2 files changed, 8 insertions(+), 59 deletions(-)

-- 
2.20.1


^ permalink raw reply

* [PATCH net-next 1/2] net: phy: improve phy_resolve_aneg_linkmode
From: Heiner Kallweit @ 2019-02-14 21:15 UTC (permalink / raw)
  To: Andrew Lunn, Florian Fainelli, David Miller; +Cc: netdev@vger.kernel.org
In-Reply-To: <cc50884b-6271-6b93-605c-f451abeb0d7e@gmail.com>

We have the settings array of modes which is sorted based on aneg
priority. Instead of checking each mode manually let's simply iterate
over the sorted settings.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/net/phy/phy-core.c | 43 +++++++-------------------------------
 1 file changed, 7 insertions(+), 36 deletions(-)

diff --git a/drivers/net/phy/phy-core.c b/drivers/net/phy/phy-core.c
index cdea028d1..5d43106fe 100644
--- a/drivers/net/phy/phy-core.c
+++ b/drivers/net/phy/phy-core.c
@@ -349,45 +349,16 @@ size_t phy_speeds(unsigned int *speeds, size_t size,
 void phy_resolve_aneg_linkmode(struct phy_device *phydev)
 {
 	__ETHTOOL_DECLARE_LINK_MODE_MASK(common);
+	int i;
 
 	linkmode_and(common, phydev->lp_advertising, phydev->advertising);
 
-	if (linkmode_test_bit(ETHTOOL_LINK_MODE_10000baseT_Full_BIT, common)) {
-		phydev->speed = SPEED_10000;
-		phydev->duplex = DUPLEX_FULL;
-	} else if (linkmode_test_bit(ETHTOOL_LINK_MODE_5000baseT_Full_BIT,
-				     common)) {
-		phydev->speed = SPEED_5000;
-		phydev->duplex = DUPLEX_FULL;
-	} else if (linkmode_test_bit(ETHTOOL_LINK_MODE_2500baseT_Full_BIT,
-				     common)) {
-		phydev->speed = SPEED_2500;
-		phydev->duplex = DUPLEX_FULL;
-	} else if (linkmode_test_bit(ETHTOOL_LINK_MODE_1000baseT_Full_BIT,
-				     common)) {
-		phydev->speed = SPEED_1000;
-		phydev->duplex = DUPLEX_FULL;
-	} else if (linkmode_test_bit(ETHTOOL_LINK_MODE_1000baseT_Half_BIT,
-				     common)) {
-		phydev->speed = SPEED_1000;
-		phydev->duplex = DUPLEX_HALF;
-	} else if (linkmode_test_bit(ETHTOOL_LINK_MODE_100baseT_Full_BIT,
-				     common)) {
-		phydev->speed = SPEED_100;
-		phydev->duplex = DUPLEX_FULL;
-	} else if (linkmode_test_bit(ETHTOOL_LINK_MODE_100baseT_Half_BIT,
-				     common)) {
-		phydev->speed = SPEED_100;
-		phydev->duplex = DUPLEX_HALF;
-	} else if (linkmode_test_bit(ETHTOOL_LINK_MODE_10baseT_Full_BIT,
-				     common)) {
-		phydev->speed = SPEED_10;
-		phydev->duplex = DUPLEX_FULL;
-	} else if (linkmode_test_bit(ETHTOOL_LINK_MODE_10baseT_Half_BIT,
-				     common)) {
-		phydev->speed = SPEED_10;
-		phydev->duplex = DUPLEX_HALF;
-	}
+	for (i = 0; i < ARRAY_SIZE(settings); i++)
+		if (test_bit(settings[i].bit, common)) {
+			phydev->speed = settings[i].speed;
+			phydev->duplex = settings[i].duplex;
+			break;
+		}
 
 	if (phydev->duplex == DUPLEX_FULL) {
 		phydev->pause = linkmode_test_bit(ETHTOOL_LINK_MODE_Pause_BIT,
-- 
2.20.1



^ permalink raw reply related

* [PATCH net-next 2/2] net: phy: use phy_resolve_aneg_linkmode in genphy_read_status
From: Heiner Kallweit @ 2019-02-14 21:16 UTC (permalink / raw)
  To: Andrew Lunn, Florian Fainelli, David Miller; +Cc: netdev@vger.kernel.org
In-Reply-To: <cc50884b-6271-6b93-605c-f451abeb0d7e@gmail.com>

Now that we have phy_resolve_aneg_linkmode() we can make
genphy_read_status() much simpler.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/net/phy/phy_device.c | 24 +-----------------------
 1 file changed, 1 insertion(+), 23 deletions(-)

diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index 2c61282a2..d7ccbd94f 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -1747,8 +1747,6 @@ int genphy_read_status(struct phy_device *phydev)
 	int err;
 	int lpa;
 	int lpagb = 0;
-	int common_adv;
-	int common_adv_gb = 0;
 
 	/* Update the link, but return if there was an error */
 	err = genphy_update_link(phydev);
@@ -1780,7 +1778,6 @@ int genphy_read_status(struct phy_device *phydev)
 
 			mii_stat1000_mod_linkmode_lpa_t(phydev->lp_advertising,
 							lpagb);
-			common_adv_gb = lpagb & adv << 2;
 		}
 
 		lpa = phy_read(phydev, MII_LPA);
@@ -1793,31 +1790,12 @@ int genphy_read_status(struct phy_device *phydev)
 		if (adv < 0)
 			return adv;
 
-		common_adv = lpa & adv;
-
 		phydev->speed = SPEED_10;
 		phydev->duplex = DUPLEX_HALF;
 		phydev->pause = 0;
 		phydev->asym_pause = 0;
 
-		if (common_adv_gb & (LPA_1000FULL | LPA_1000HALF)) {
-			phydev->speed = SPEED_1000;
-
-			if (common_adv_gb & LPA_1000FULL)
-				phydev->duplex = DUPLEX_FULL;
-		} else if (common_adv & (LPA_100FULL | LPA_100HALF)) {
-			phydev->speed = SPEED_100;
-
-			if (common_adv & LPA_100FULL)
-				phydev->duplex = DUPLEX_FULL;
-		} else
-			if (common_adv & LPA_10FULL)
-				phydev->duplex = DUPLEX_FULL;
-
-		if (phydev->duplex == DUPLEX_FULL) {
-			phydev->pause = lpa & LPA_PAUSE_CAP ? 1 : 0;
-			phydev->asym_pause = lpa & LPA_PAUSE_ASYM ? 1 : 0;
-		}
+		phy_resolve_aneg_linkmode(phydev);
 	} else {
 		int bmcr = phy_read(phydev, MII_BMCR);
 
-- 
2.20.1



^ permalink raw reply related

* Re: [RESEND PATCH net] mm: page_alloc: fix ref bias in page_frag_alloc() for 1-byte allocs
From: Jann Horn @ 2019-02-14 21:26 UTC (permalink / raw)
  To: David Miller, Alexander Duyck; +Cc: Network Development, kernel list
In-Reply-To: <20190214.091328.1687361207100252890.davem@davemloft.net>

On Thu, Feb 14, 2019 at 6:13 PM David Miller <davem@davemloft.net> wrote:
>
> From: Jann Horn <jannh@google.com>
> Date: Wed, 13 Feb 2019 22:45:59 +0100
>
> > The basic idea behind ->pagecnt_bias is: If we pre-allocate the maximum
> > number of references that we might need to create in the fastpath later,
> > the bump-allocation fastpath only has to modify the non-atomic bias value
> > that tracks the number of extra references we hold instead of the atomic
> > refcount. The maximum number of allocations we can serve (under the
> > assumption that no allocation is made with size 0) is nc->size, so that's
> > the bias used.
> >
> > However, even when all memory in the allocation has been given away, a
> > reference to the page is still held; and in the `offset < 0` slowpath, the
> > page may be reused if everyone else has dropped their references.
> > This means that the necessary number of references is actually
> > `nc->size+1`.
> >
> > Luckily, from a quick grep, it looks like the only path that can call
> > page_frag_alloc(fragsz=1) is TAP with the IFF_NAPI_FRAGS flag, which
> > requires CAP_NET_ADMIN in the init namespace and is only intended to be
> > used for kernel testing and fuzzing.
> >
> > To test for this issue, put a `WARN_ON(page_ref_count(page) == 0)` in the
> > `offset < 0` path, below the virt_to_page() call, and then repeatedly call
> > writev() on a TAP device with IFF_TAP|IFF_NO_PI|IFF_NAPI_FRAGS|IFF_NAPI,
> > with a vector consisting of 15 elements containing 1 byte each.
> >
> > Signed-off-by: Jann Horn <jannh@google.com>
>
> Applied and queued up for -stable.

I had sent a v2 at Alexander Duyck's request an hour before you
applied the patch (with a minor difference that, in Alexander's
opinion, might be slightly more efficient). I guess the net tree
doesn't work like the mm tree, where patches can get removed and
replaced with newer versions? So if Alexander wants that change
(s/size/PAGE_FRAG_CACHE_MAX_SIZE/ in the refcount), someone has to
send that as a separate patch?

^ permalink raw reply

* Re: [PATCH][next] can: at91_can: mark expected switch fall-throughs
From: Gustavo A. R. Silva @ 2019-02-14 21:33 UTC (permalink / raw)
  To: Nicolas.Ferre, wg, mkl, davem, alexandre.belloni,
	Ludovic.Desroches
  Cc: linux-can, netdev, linux-arm-kernel, linux-kernel
In-Reply-To: <8119fbe8-5f4b-e45c-6687-ec42aceb3428@microchip.com>



On 2/11/19 3:03 AM, Nicolas.Ferre@microchip.com wrote:
> On 08/02/2019 at 19:44, Gustavo A. R. Silva wrote:
>> In preparation to enabling -Wimplicit-fallthrough, mark switch
>> cases where we are expecting to fall through.
>>
>> Notice that, in this particular case, the /* fall through */
>> comments are placed at the bottom of the case statement, which
>> is what GCC is expecting to find.
>>
>> Warning level 3 was used: -Wimplicit-fallthrough=3
>>
>> This patch is part of the ongoing efforts to enabling
>> -Wimplicit-fallthrough.
>>
>> Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
> 
> Looks good to me:
> Acked-by: Nicolas Ferre <nicolas.ferre@microchip.com>
> 

Thanks, Nicolas.

--
Gustavo

>> ---
>>   drivers/net/can/at91_can.c | 6 ++++--
>>   1 file changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/net/can/at91_can.c b/drivers/net/can/at91_can.c
>> index d98c69045b17..1718c20f9c99 100644
>> --- a/drivers/net/can/at91_can.c
>> +++ b/drivers/net/can/at91_can.c
>> @@ -902,7 +902,8 @@ static void at91_irq_err_state(struct net_device *dev,
>>   				CAN_ERR_CRTL_TX_WARNING :
>>   				CAN_ERR_CRTL_RX_WARNING;
>>   		}
>> -	case CAN_STATE_ERROR_WARNING:	/* fallthrough */
>> +		/* fall through */
>> +	case CAN_STATE_ERROR_WARNING:
>>   		/*
>>   		 * from: ERROR_ACTIVE, ERROR_WARNING
>>   		 * to  : ERROR_PASSIVE, BUS_OFF
>> @@ -951,7 +952,8 @@ static void at91_irq_err_state(struct net_device *dev,
>>   		netdev_dbg(dev, "Error Active\n");
>>   		cf->can_id |= CAN_ERR_PROT;
>>   		cf->data[2] = CAN_ERR_PROT_ACTIVE;
>> -	case CAN_STATE_ERROR_WARNING:	/* fallthrough */
>> +		/* fall through */
>> +	case CAN_STATE_ERROR_WARNING:
>>   		reg_idr = AT91_IRQ_ERRA | AT91_IRQ_WARN | AT91_IRQ_BOFF;
>>   		reg_ier = AT91_IRQ_ERRP;
>>   		break;
>>
> 
> 

^ permalink raw reply

* [PATCH net-next] net: caif: use skb helpers instead of open-coding them
From: Jann Horn @ 2019-02-14 21:35 UTC (permalink / raw)
  To: David S. Miller, Dmitry Tarnyagin, jannh; +Cc: netdev

Use existing skb_put_data() and skb_trim() instead of open-coding them,
with the skb_put_data() first so that logically, `skb` still contains the
data to be copied in its data..tail area when skb_put_data() reads it.
This change on its own is a cleanup, and it is also necessary for potential
future integration of skbuffs with things like KASAN.

Signed-off-by: Jann Horn <jannh@google.com>
---
 net/caif/cfpkt_skbuff.c | 16 +++++-----------
 1 file changed, 5 insertions(+), 11 deletions(-)

diff --git a/net/caif/cfpkt_skbuff.c b/net/caif/cfpkt_skbuff.c
index 38c2b7a890dd..37ac5ca0ffdf 100644
--- a/net/caif/cfpkt_skbuff.c
+++ b/net/caif/cfpkt_skbuff.c
@@ -319,16 +319,12 @@ struct cfpkt *cfpkt_append(struct cfpkt *dstpkt,
 		if (tmppkt == NULL)
 			return NULL;
 		tmp = pkt_to_skb(tmppkt);
-		skb_set_tail_pointer(tmp, dstlen);
-		tmp->len = dstlen;
-		memcpy(tmp->data, dst->data, dstlen);
+		skb_put_data(tmp, dst->data, dstlen);
 		cfpkt_destroy(dstpkt);
 		dst = tmp;
 	}
-	memcpy(skb_tail_pointer(dst), add->data, skb_headlen(add));
+	skb_put_data(dst, add->data, skb_headlen(add));
 	cfpkt_destroy(addpkt);
-	dst->tail += addlen;
-	dst->len += addlen;
 	return skb_to_pkt(dst);
 }
 
@@ -359,13 +355,11 @@ struct cfpkt *cfpkt_split(struct cfpkt *pkt, u16 pos)
 	if (skb2 == NULL)
 		return NULL;
 
+	skb_put_data(skb2, split, len2nd);
+
 	/* Reduce the length of the original packet */
-	skb_set_tail_pointer(skb, pos);
-	skb->len = pos;
+	skb_trim(skb, pos);
 
-	memcpy(skb2->data, split, len2nd);
-	skb2->tail += len2nd;
-	skb2->len += len2nd;
 	skb2->priority = skb->priority;
 	return skb_to_pkt(skb2);
 }
-- 
2.21.0.rc0.258.g878e2cd30e-goog


^ permalink raw reply related

* Re: [PATCH] can: mark expected switch fall-throughs
From: Gustavo A. R. Silva @ 2019-02-14 21:37 UTC (permalink / raw)
  To: Nicolas.Ferre, wg, mkl, davem, alexandre.belloni,
	Ludovic.Desroches
  Cc: linux-can, netdev, linux-arm-kernel, linux-kernel
In-Reply-To: <432a9399-95f4-e988-5cd2-93340f155fa1@microchip.com>



On 1/30/19 2:11 AM, Nicolas.Ferre@microchip.com wrote:
> On 29/01/2019 at 19:06, Gustavo A. R. Silva wrote:
>> In preparation to enabling -Wimplicit-fallthrough, mark switch cases
>> where we are expecting to fall through.
>>
>> This patch fixes the following warnings:
>>
>> drivers/net/can/peak_canfd/peak_pciefd_main.c:668:3: warning: this statement may fall through [-Wimplicit-fallthrough=]
>> drivers/net/can/spi/mcp251x.c:875:7: warning: this statement may fall through [-Wimplicit-fallthrough=]
>> drivers/net/can/usb/peak_usb/pcan_usb.c:422:6: warning: this statement may fall through [-Wimplicit-fallthrough=]
>> drivers/net/can/at91_can.c:895:6: warning: this statement may fall through [-Wimplicit-fallthrough=]
>> drivers/net/can/at91_can.c:953:15: warning: this statement may fall through [-Wimplicit-fallthrough=]
>> drivers/net/can/usb/peak_usb/pcan_usb.c: In function ‘pcan_usb_decode_error’:
>> drivers/net/can/usb/peak_usb/pcan_usb.c:422:6: warning: this statement may fall through [-Wimplicit-fallthrough=]
>>     if (n & PCAN_USB_ERROR_BUS_LIGHT) {
>>        ^
>> drivers/net/can/usb/peak_usb/pcan_usb.c:428:2: note: here
>>    case CAN_STATE_ERROR_WARNING:
>>    ^~~~
>>
>> Warning level 3 was used: -Wimplicit-fallthrough=3
>>
>> This patch is part of the ongoing efforts to enabling
>> -Wimplicit-fallthrough.
>>
>> Notice that in some cases spelling mistakes were fixed.
>> In other cases, the /* fall through */ comment is placed
>> at the bottom of the case statement, which is what GCC
>> is expecting to find.
>>
>> Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
>> ---
>>   drivers/net/can/at91_can.c                    | 6 ++++--
> 
> For this one:
> Acked-by: Nicolas Ferre <nicolas.ferre@microchip.com>
> 

Thanks, Nicolas.

Dave:

I wonder if you can take this patch.

Thanks
--
Gustavo

>>   drivers/net/can/peak_canfd/peak_pciefd_main.c | 2 +-
>>   drivers/net/can/spi/mcp251x.c                 | 3 ++-
>>   drivers/net/can/usb/peak_usb/pcan_usb.c       | 2 +-
>>   4 files changed, 8 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/net/can/at91_can.c b/drivers/net/can/at91_can.c
>> index d98c69045b17..1718c20f9c99 100644
>> --- a/drivers/net/can/at91_can.c
>> +++ b/drivers/net/can/at91_can.c
>> @@ -902,7 +902,8 @@ static void at91_irq_err_state(struct net_device *dev,
>>   				CAN_ERR_CRTL_TX_WARNING :
>>   				CAN_ERR_CRTL_RX_WARNING;
>>   		}
>> -	case CAN_STATE_ERROR_WARNING:	/* fallthrough */
>> +		/* fall through */
>> +	case CAN_STATE_ERROR_WARNING:
>>   		/*
>>   		 * from: ERROR_ACTIVE, ERROR_WARNING
>>   		 * to  : ERROR_PASSIVE, BUS_OFF
>> @@ -951,7 +952,8 @@ static void at91_irq_err_state(struct net_device *dev,
>>   		netdev_dbg(dev, "Error Active\n");
>>   		cf->can_id |= CAN_ERR_PROT;
>>   		cf->data[2] = CAN_ERR_PROT_ACTIVE;
>> -	case CAN_STATE_ERROR_WARNING:	/* fallthrough */
>> +		/* fall through */
>> +	case CAN_STATE_ERROR_WARNING:
>>   		reg_idr = AT91_IRQ_ERRA | AT91_IRQ_WARN | AT91_IRQ_BOFF;
>>   		reg_ier = AT91_IRQ_ERRP;
>>   		break;
>> diff --git a/drivers/net/can/peak_canfd/peak_pciefd_main.c b/drivers/net/can/peak_canfd/peak_pciefd_main.c
>> index c458d5fdc8d3..e4f4d65a76b4 100644
>> --- a/drivers/net/can/peak_canfd/peak_pciefd_main.c
>> +++ b/drivers/net/can/peak_canfd/peak_pciefd_main.c
>> @@ -668,7 +668,7 @@ static int pciefd_can_probe(struct pciefd_board *pciefd)
>>   		pciefd_can_writereg(priv, CANFD_CLK_SEL_80MHZ,
>>   				    PCIEFD_REG_CAN_CLK_SEL);
>>   
>> -		/* fallthough */
>> +		/* fall through */
>>   	case CANFD_CLK_SEL_80MHZ:
>>   		priv->ucan.can.clock.freq = 80 * 1000 * 1000;
>>   		break;
>> diff --git a/drivers/net/can/spi/mcp251x.c b/drivers/net/can/spi/mcp251x.c
>> index e90817608645..17257c73c302 100644
>> --- a/drivers/net/can/spi/mcp251x.c
>> +++ b/drivers/net/can/spi/mcp251x.c
>> @@ -875,7 +875,8 @@ static irqreturn_t mcp251x_can_ist(int irq, void *dev_id)
>>   			if (new_state >= CAN_STATE_ERROR_WARNING &&
>>   			    new_state <= CAN_STATE_BUS_OFF)
>>   				priv->can.can_stats.error_warning++;
>> -		case CAN_STATE_ERROR_WARNING:	/* fallthrough */
>> +			/* fall through */
>> +		case CAN_STATE_ERROR_WARNING:
>>   			if (new_state >= CAN_STATE_ERROR_PASSIVE &&
>>   			    new_state <= CAN_STATE_BUS_OFF)
>>   				priv->can.can_stats.error_passive++;
>> diff --git a/drivers/net/can/usb/peak_usb/pcan_usb.c b/drivers/net/can/usb/peak_usb/pcan_usb.c
>> index 13238a72a338..eca785532b6b 100644
>> --- a/drivers/net/can/usb/peak_usb/pcan_usb.c
>> +++ b/drivers/net/can/usb/peak_usb/pcan_usb.c
>> @@ -423,7 +423,7 @@ static int pcan_usb_decode_error(struct pcan_usb_msg_context *mc, u8 n,
>>   			new_state = CAN_STATE_ERROR_WARNING;
>>   			break;
>>   		}
>> -		/* else: fall through */
>> +		/* fall through */
>>   
>>   	case CAN_STATE_ERROR_WARNING:
>>   		if (n & PCAN_USB_ERROR_BUS_HEAVY) {
>>
> 
> 

^ permalink raw reply

* [PATCH net-next 0/3] devlink: add the ability to update device flash
From: Jakub Kicinski @ 2019-02-14 21:40 UTC (permalink / raw)
  To: davem, jiri; +Cc: netdev, oss-drivers, mkubecek, andrew, Jakub Kicinski

Hi!

This series is the second step to allow trouble shooting and recovering
devices in bad state without the use of netdevs as handles.  We can
already query FW versions over devlink, now we add the ability to update
the FW.  This will allow drivers to implement some from of "limp-mode"
where the device can't really be used for networking and hence has no
netdev, but we can interrogate it over devlink and fix the broken FW.

Small but nice advantage of devlink is that it only holds the devlink
instance lock during flashing, unlike ethtool which holds rtnl_lock().

Jakub Kicinski (3):
  devlink: add flash update command
  ethtool: add compat for flash update
  nfp: devlink: allow flashing the device via devlink

 .../net/ethernet/netronome/nfp/nfp_devlink.c  | 10 ++++
 drivers/net/ethernet/netronome/nfp/nfp_main.c | 41 +++++++++++++
 drivers/net/ethernet/netronome/nfp/nfp_main.h |  2 +
 .../ethernet/netronome/nfp/nfp_net_ethtool.c  | 35 +----------
 include/net/devlink.h                         | 10 ++++
 include/uapi/linux/devlink.h                  |  6 ++
 net/core/devlink.c                            | 60 +++++++++++++++++++
 net/core/ethtool.c                            | 12 +++-
 8 files changed, 141 insertions(+), 35 deletions(-)

-- 
2.19.2


^ permalink raw reply

* [PATCH net-next 1/3] devlink: add flash update command
From: Jakub Kicinski @ 2019-02-14 21:40 UTC (permalink / raw)
  To: davem, jiri; +Cc: netdev, oss-drivers, mkubecek, andrew, Jakub Kicinski
In-Reply-To: <20190214214046.19182-1-jakub.kicinski@netronome.com>

Add devlink flash update command. Advanced NICs have firmware
stored in flash and often cryptographically secured. Updating
that flash is handled by management firmware. Ethtool has a
flash update command which served us well, however, it has two
shortcomings:
 - it takes rtnl_lock unnecessarily - really flash update has
   nothing to do with networking, so using a networking device
   as a handle is suboptimal, which leads us to the second one:
 - it requires a functioning netdev - in case device enters an
   error state and can't spawn a netdev (e.g. communication
   with the device fails) there is no netdev to use as a handle
   for flashing.

Devlink already has the ability to report the firmware versions,
now with the ability to update the firmware/flash we will be
able to recover devices in bad state.

To enable updates of sub-components of the FW allow passing
component name.  This name should correspond to one of the
versions reported in devlink info.

v1: - replace target id with component name (Jiri).

Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
---
 include/net/devlink.h        |  3 +++
 include/uapi/linux/devlink.h |  6 ++++++
 net/core/devlink.c           | 30 ++++++++++++++++++++++++++++++
 3 files changed, 39 insertions(+)

diff --git a/include/net/devlink.h b/include/net/devlink.h
index c6d88759b7d5..18d7a051f412 100644
--- a/include/net/devlink.h
+++ b/include/net/devlink.h
@@ -521,6 +521,9 @@ struct devlink_ops {
 				      struct netlink_ext_ack *extack);
 	int (*info_get)(struct devlink *devlink, struct devlink_info_req *req,
 			struct netlink_ext_ack *extack);
+	int (*flash_update)(struct devlink *devlink, const char *file_name,
+			    const char *component,
+			    struct netlink_ext_ack *extack);
 };
 
 static inline void *devlink_priv(struct devlink *devlink)
diff --git a/include/uapi/linux/devlink.h b/include/uapi/linux/devlink.h
index 72d9f7c89190..53de8802a000 100644
--- a/include/uapi/linux/devlink.h
+++ b/include/uapi/linux/devlink.h
@@ -103,6 +103,8 @@ enum devlink_command {
 	DEVLINK_CMD_HEALTH_REPORTER_DUMP_GET,
 	DEVLINK_CMD_HEALTH_REPORTER_DUMP_CLEAR,
 
+	DEVLINK_CMD_FLASH_UPDATE,
+
 	/* add new commands above here */
 	__DEVLINK_CMD_MAX,
 	DEVLINK_CMD_MAX = __DEVLINK_CMD_MAX - 1
@@ -326,6 +328,10 @@ enum devlink_attr {
 	DEVLINK_ATTR_HEALTH_REPORTER_DUMP_TS,		/* u64 */
 	DEVLINK_ATTR_HEALTH_REPORTER_GRACEFUL_PERIOD,	/* u64 */
 	DEVLINK_ATTR_HEALTH_REPORTER_AUTO_RECOVER,	/* u8 */
+
+	DEVLINK_ATTR_FLASH_UPDATE_FILE_NAME,	/* string */
+	DEVLINK_ATTR_FLASH_UPDATE_COMPONENT,	/* string */
+
 	/* add new attributes above here, update the policy in devlink.c */
 
 	__DEVLINK_ATTR_MAX,
diff --git a/net/core/devlink.c b/net/core/devlink.c
index 283c3ed9f25e..bd507e13bb7b 100644
--- a/net/core/devlink.c
+++ b/net/core/devlink.c
@@ -2660,6 +2660,27 @@ static int devlink_nl_cmd_reload(struct sk_buff *skb, struct genl_info *info)
 	return devlink->ops->reload(devlink, info->extack);
 }
 
+static int devlink_nl_cmd_flash_update(struct sk_buff *skb,
+				       struct genl_info *info)
+{
+	struct devlink *devlink = info->user_ptr[0];
+	const char *file_name, *component;
+	struct nlattr *nla_component;
+
+	if (!devlink->ops->flash_update)
+		return -EOPNOTSUPP;
+
+	if (!info->attrs[DEVLINK_ATTR_FLASH_UPDATE_FILE_NAME])
+		return -EINVAL;
+	file_name = nla_data(info->attrs[DEVLINK_ATTR_FLASH_UPDATE_FILE_NAME]);
+
+	nla_component = info->attrs[DEVLINK_ATTR_FLASH_UPDATE_COMPONENT];
+	component = nla_component ? nla_data(nla_component) : NULL;
+
+	return devlink->ops->flash_update(devlink, file_name, component,
+					  info->extack);
+}
+
 static const struct devlink_param devlink_param_generic[] = {
 	{
 		.id = DEVLINK_PARAM_GENERIC_ID_INT_ERR_RESET,
@@ -4868,6 +4889,8 @@ static const struct nla_policy devlink_nl_policy[DEVLINK_ATTR_MAX + 1] = {
 	[DEVLINK_ATTR_HEALTH_REPORTER_NAME] = { .type = NLA_NUL_STRING },
 	[DEVLINK_ATTR_HEALTH_REPORTER_GRACEFUL_PERIOD] = { .type = NLA_U64 },
 	[DEVLINK_ATTR_HEALTH_REPORTER_AUTO_RECOVER] = { .type = NLA_U8 },
+	[DEVLINK_ATTR_FLASH_UPDATE_FILE_NAME] = { .type = NLA_NUL_STRING },
+	[DEVLINK_ATTR_FLASH_UPDATE_COMPONENT] = { .type = NLA_NUL_STRING },
 };
 
 static const struct genl_ops devlink_nl_ops[] = {
@@ -5156,6 +5179,13 @@ static const struct genl_ops devlink_nl_ops[] = {
 		.internal_flags = DEVLINK_NL_FLAG_NEED_DEVLINK |
 				  DEVLINK_NL_FLAG_NO_LOCK,
 	},
+	{
+		.cmd = DEVLINK_CMD_FLASH_UPDATE,
+		.doit = devlink_nl_cmd_flash_update,
+		.policy = devlink_nl_policy,
+		.flags = GENL_ADMIN_PERM,
+		.internal_flags = DEVLINK_NL_FLAG_NEED_DEVLINK,
+	},
 };
 
 static struct genl_family devlink_nl_family __ro_after_init = {
-- 
2.19.2


^ permalink raw reply related

* [PATCH net-next 2/3] ethtool: add compat for flash update
From: Jakub Kicinski @ 2019-02-14 21:40 UTC (permalink / raw)
  To: davem, jiri; +Cc: netdev, oss-drivers, mkubecek, andrew, Jakub Kicinski
In-Reply-To: <20190214214046.19182-1-jakub.kicinski@netronome.com>

If driver does not support ethtool flash update operation
call into devlink.

Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
---
 include/net/devlink.h |  7 +++++++
 net/core/devlink.c    | 30 ++++++++++++++++++++++++++++++
 net/core/ethtool.c    | 12 +++++++++---
 3 files changed, 46 insertions(+), 3 deletions(-)

diff --git a/include/net/devlink.h b/include/net/devlink.h
index 18d7a051f412..a2da49dd9147 100644
--- a/include/net/devlink.h
+++ b/include/net/devlink.h
@@ -1195,11 +1195,18 @@ devlink_health_report(struct devlink_health_reporter *reporter,
 #if IS_REACHABLE(CONFIG_NET_DEVLINK)
 void devlink_compat_running_version(struct net_device *dev,
 				    char *buf, size_t len);
+int devlink_compat_flash_update(struct net_device *dev, const char *file_name);
 #else
 static inline void
 devlink_compat_running_version(struct net_device *dev, char *buf, size_t len)
 {
 }
+
+static inline int
+devlink_compat_flash_update(struct net_device *dev, const char *file_name)
+{
+	return -EOPNOTSUPP;
+}
 #endif
 
 #endif /* _NET_DEVLINK_H_ */
diff --git a/net/core/devlink.c b/net/core/devlink.c
index bd507e13bb7b..d169b5426d3d 100644
--- a/net/core/devlink.c
+++ b/net/core/devlink.c
@@ -6435,6 +6435,36 @@ void devlink_compat_running_version(struct net_device *dev,
 	mutex_unlock(&devlink_mutex);
 }
 
+int devlink_compat_flash_update(struct net_device *dev, const char *file_name)
+{
+	struct devlink_port *devlink_port;
+	struct devlink *devlink;
+
+	mutex_lock(&devlink_mutex);
+	list_for_each_entry(devlink, &devlink_list, list) {
+		mutex_lock(&devlink->lock);
+		list_for_each_entry(devlink_port, &devlink->port_list, list) {
+			int ret = -EOPNOTSUPP;
+
+			if (devlink_port->type != DEVLINK_PORT_TYPE_ETH ||
+			    devlink_port->type_dev != dev)
+				continue;
+
+			mutex_unlock(&devlink_mutex);
+			if (devlink->ops->flash_update)
+				ret = devlink->ops->flash_update(devlink,
+								 file_name,
+								 NULL, NULL);
+			mutex_unlock(&devlink->lock);
+			return ret;
+		}
+		mutex_unlock(&devlink->lock);
+	}
+	mutex_unlock(&devlink_mutex);
+
+	return -EOPNOTSUPP;
+}
+
 static int __init devlink_module_init(void)
 {
 	return genl_register_family(&devlink_nl_family);
diff --git a/net/core/ethtool.c b/net/core/ethtool.c
index d2c47cdf25da..1320e8dce559 100644
--- a/net/core/ethtool.c
+++ b/net/core/ethtool.c
@@ -2038,11 +2038,17 @@ static noinline_for_stack int ethtool_flash_device(struct net_device *dev,
 
 	if (copy_from_user(&efl, useraddr, sizeof(efl)))
 		return -EFAULT;
+	efl.data[ETHTOOL_FLASH_MAX_FILENAME - 1] = 0;
 
-	if (!dev->ethtool_ops->flash_device)
-		return -EOPNOTSUPP;
+	if (!dev->ethtool_ops->flash_device) {
+		int ret;
 
-	efl.data[ETHTOOL_FLASH_MAX_FILENAME - 1] = 0;
+		rtnl_unlock();
+		ret = devlink_compat_flash_update(dev, efl.data);
+		rtnl_lock();
+
+		return ret;
+	}
 
 	return dev->ethtool_ops->flash_device(dev, &efl);
 }
-- 
2.19.2


^ permalink raw reply related

* [PATCH net-next 3/3] nfp: devlink: allow flashing the device via devlink
From: Jakub Kicinski @ 2019-02-14 21:40 UTC (permalink / raw)
  To: davem, jiri; +Cc: netdev, oss-drivers, mkubecek, andrew, Jakub Kicinski
In-Reply-To: <20190214214046.19182-1-jakub.kicinski@netronome.com>

Devlink now allows updating device flash.  Implement this
callback.

Compared to ethtool update we no longer have to release
the networking locks - devlink doesn't take them.

Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
---
 .../net/ethernet/netronome/nfp/nfp_devlink.c  | 10 +++++
 drivers/net/ethernet/netronome/nfp/nfp_main.c | 41 +++++++++++++++++++
 drivers/net/ethernet/netronome/nfp/nfp_main.h |  2 +
 .../ethernet/netronome/nfp/nfp_net_ethtool.c  | 35 ++--------------
 4 files changed, 56 insertions(+), 32 deletions(-)

diff --git a/drivers/net/ethernet/netronome/nfp/nfp_devlink.c b/drivers/net/ethernet/netronome/nfp/nfp_devlink.c
index 080a301f379e..db2da99f6aa7 100644
--- a/drivers/net/ethernet/netronome/nfp/nfp_devlink.c
+++ b/drivers/net/ethernet/netronome/nfp/nfp_devlink.c
@@ -330,6 +330,15 @@ nfp_devlink_info_get(struct devlink *devlink, struct devlink_info_req *req,
 	return err;
 }
 
+static int
+nfp_devlink_flash_update(struct devlink *devlink, const char *path,
+			 const char *component, struct netlink_ext_ack *extack)
+{
+	if (component)
+		return -EOPNOTSUPP;
+	return nfp_flash_update_common(devlink_priv(devlink), path, extack);
+}
+
 const struct devlink_ops nfp_devlink_ops = {
 	.port_split		= nfp_devlink_port_split,
 	.port_unsplit		= nfp_devlink_port_unsplit,
@@ -338,6 +347,7 @@ const struct devlink_ops nfp_devlink_ops = {
 	.eswitch_mode_get	= nfp_devlink_eswitch_mode_get,
 	.eswitch_mode_set	= nfp_devlink_eswitch_mode_set,
 	.info_get		= nfp_devlink_info_get,
+	.flash_update		= nfp_devlink_flash_update,
 };
 
 int nfp_devlink_port_register(struct nfp_app *app, struct nfp_port *port)
diff --git a/drivers/net/ethernet/netronome/nfp/nfp_main.c b/drivers/net/ethernet/netronome/nfp/nfp_main.c
index 6c10e8d119e4..f4c8776e42b6 100644
--- a/drivers/net/ethernet/netronome/nfp/nfp_main.c
+++ b/drivers/net/ethernet/netronome/nfp/nfp_main.c
@@ -300,6 +300,47 @@ static int nfp_pcie_sriov_configure(struct pci_dev *pdev, int num_vfs)
 		return nfp_pcie_sriov_enable(pdev, num_vfs);
 }
 
+int nfp_flash_update_common(struct nfp_pf *pf, const char *path,
+			    struct netlink_ext_ack *extack)
+{
+	struct device *dev = &pf->pdev->dev;
+	const struct firmware *fw;
+	struct nfp_nsp *nsp;
+	int err;
+
+	nsp = nfp_nsp_open(pf->cpp);
+	if (IS_ERR(nsp)) {
+		err = PTR_ERR(nsp);
+		if (extack)
+			NL_SET_ERR_MSG_MOD(extack, "can't access NSP");
+		else
+			dev_err(dev, "Failed to access the NSP: %d\n", err);
+		return err;
+	}
+
+	err = request_firmware_direct(&fw, path, dev);
+	if (err) {
+		NL_SET_ERR_MSG_MOD(extack,
+				   "unable to read flash file from disk");
+		goto exit_close_nsp;
+	}
+
+	dev_info(dev, "Please be patient while writing flash image: %s\n",
+		 path);
+
+	err = nfp_nsp_write_flash(nsp, fw);
+	if (err < 0)
+		goto exit_release_fw;
+	dev_info(dev, "Finished writing flash image\n");
+	err = 0;
+
+exit_release_fw:
+	release_firmware(fw);
+exit_close_nsp:
+	nfp_nsp_close(nsp);
+	return err;
+}
+
 static const struct firmware *
 nfp_net_fw_request(struct pci_dev *pdev, struct nfp_pf *pf, const char *name)
 {
diff --git a/drivers/net/ethernet/netronome/nfp/nfp_main.h b/drivers/net/ethernet/netronome/nfp/nfp_main.h
index a3613a2e0aa5..b7211f200d22 100644
--- a/drivers/net/ethernet/netronome/nfp/nfp_main.h
+++ b/drivers/net/ethernet/netronome/nfp/nfp_main.h
@@ -164,6 +164,8 @@ nfp_pf_map_rtsym(struct nfp_pf *pf, const char *name, const char *sym_fmt,
 		 unsigned int min_size, struct nfp_cpp_area **area);
 int nfp_mbox_cmd(struct nfp_pf *pf, u32 cmd, void *in_data, u64 in_length,
 		 void *out_data, u64 out_length);
+int nfp_flash_update_common(struct nfp_pf *pf, const char *path,
+			    struct netlink_ext_ack *extack);
 
 enum nfp_dump_diag {
 	NFP_DUMP_NSP_DIAG = 0,
diff --git a/drivers/net/ethernet/netronome/nfp/nfp_net_ethtool.c b/drivers/net/ethernet/netronome/nfp/nfp_net_ethtool.c
index cb9c512abc76..8f189149efc5 100644
--- a/drivers/net/ethernet/netronome/nfp/nfp_net_ethtool.c
+++ b/drivers/net/ethernet/netronome/nfp/nfp_net_ethtool.c
@@ -1237,11 +1237,8 @@ static int nfp_net_set_channels(struct net_device *netdev,
 static int
 nfp_net_flash_device(struct net_device *netdev, struct ethtool_flash *flash)
 {
-	const struct firmware *fw;
 	struct nfp_app *app;
-	struct nfp_nsp *nsp;
-	struct device *dev;
-	int err;
+	int ret;
 
 	if (flash->region != ETHTOOL_FLASH_ALL_REGIONS)
 		return -EOPNOTSUPP;
@@ -1250,39 +1247,13 @@ nfp_net_flash_device(struct net_device *netdev, struct ethtool_flash *flash)
 	if (!app)
 		return -EOPNOTSUPP;
 
-	dev = &app->pdev->dev;
-
-	nsp = nfp_nsp_open(app->cpp);
-	if (IS_ERR(nsp)) {
-		err = PTR_ERR(nsp);
-		dev_err(dev, "Failed to access the NSP: %d\n", err);
-		return err;
-	}
-
-	err = request_firmware_direct(&fw, flash->data, dev);
-	if (err)
-		goto exit_close_nsp;
-
-	dev_info(dev, "Please be patient while writing flash image: %s\n",
-		 flash->data);
 	dev_hold(netdev);
 	rtnl_unlock();
-
-	err = nfp_nsp_write_flash(nsp, fw);
-	if (err < 0) {
-		dev_err(dev, "Flash write failed: %d\n", err);
-		goto exit_rtnl_lock;
-	}
-	dev_info(dev, "Finished writing flash image\n");
-
-exit_rtnl_lock:
+	ret = nfp_flash_update_common(app->pf, flash->data, NULL);
 	rtnl_lock();
 	dev_put(netdev);
-	release_firmware(fw);
 
-exit_close_nsp:
-	nfp_nsp_close(nsp);
-	return err;
+	return ret;
 }
 
 static const struct ethtool_ops nfp_net_ethtool_ops = {
-- 
2.19.2


^ permalink raw reply related

* Re: [PATCHv4 5/6] net: stmmac: socfpga: Use shared System Manager driver
From: David Miller @ 2019-02-14 21:56 UTC (permalink / raw)
  To: thor.thayer
  Cc: lee.jones, arnd, dinguyen, linux, catalin.marinas, will.deacon,
	peppe.cavallaro, alexandre.torgue, joabreu, olof, mcoquelin.stm32,
	mchehab+samsung, mark.rutland, bjorn.andersson, devicetree,
	linux-kernel, linux-arm-kernel, netdev
In-Reply-To: <1550177058-1860-6-git-send-email-thor.thayer@linux.intel.com>

From: thor.thayer@linux.intel.com
Date: Thu, 14 Feb 2019 14:44:17 -0600

> From: Thor Thayer <thor.thayer@linux.intel.com>
> 
> The ARM64 System Manager requires a different method of reading
> the System Manager than ARM32. A new System Manager driver was
> created to steer ARM32 System Manager calls to regmap_mmio and
> ARM64 System Manager calls to the new access method.
> 
> Convert from syscon to the shared System Manager driver so that
> both ARM64 and ARM32 are supported.
> 
> Signed-off-by: Thor Thayer <thor.thayer@linux.intel.com>

Acked-by: David S. Miller <davem@davemloft.net>

^ permalink raw reply

* Re: [PATCH net-next] net: caif: use skb helpers instead of open-coding them
From: Jann Horn @ 2019-02-14 22:00 UTC (permalink / raw)
  To: David S. Miller, Jann Horn, abi.dmitryt; +Cc: Network Development
In-Reply-To: <20190214213547.41783-1-jannh@google.com>

On Thu, Feb 14, 2019 at 10:35 PM Jann Horn <jannh@google.com> wrote:
> Use existing skb_put_data() and skb_trim() instead of open-coding them,
> with the skb_put_data() first so that logically, `skb` still contains the
> data to be copied in its data..tail area when skb_put_data() reads it.
> This change on its own is a cleanup, and it is also necessary for potential
> future integration of skbuffs with things like KASAN.

The maintainer's email address (dmitry.tarnyagin@lockless.no) bounces
with a "553 5.3.0 <dmitry.tarnyagin@lockless.no>... No such user
here". His second address that shows up in the git log,
dmitry.tarnyagin@stericsson.com, also doesn't exist:

$ dig +short stericsson.com MX
10 mxb-00178001.gslb.pphosted.com.
10 mxa-00178001.gslb.pphosted.com.
$ nc -vC mxa-00178001.gslb.pphosted.com 25
Connection to mxa-00178001.gslb.pphosted.com 25 port [tcp/smtp] succeeded!
220 mx07-00178001.pphosted.com ESMTP mfa-m0046668
HELO thejh.net
250 mx07-00178001.pphosted.com Hello {...} (may be forged), pleased to meet you
MAIL FROM: <test@thejh.net>
250 2.1.0 Sender ok
RCPT TO: <dmitry.tarnyagin@stericsson.com>
550 5.1.1 User Unknown

But his third address from a commit he made in 2011,
abi.dmitryt@gmail.com, does exist, so I'm CC'ing that now.

@Dmitry Tarnyagin: Do you still consider yourself to be the maintainer
of the CAIF subsystem? MAINTAINERS currently lists it as "Supported"
by you, which is defined as "Someone is actually paid to look after
this".

> Signed-off-by: Jann Horn <jannh@google.com>
> ---
>  net/caif/cfpkt_skbuff.c | 16 +++++-----------
>  1 file changed, 5 insertions(+), 11 deletions(-)
>
> diff --git a/net/caif/cfpkt_skbuff.c b/net/caif/cfpkt_skbuff.c
> index 38c2b7a890dd..37ac5ca0ffdf 100644
> --- a/net/caif/cfpkt_skbuff.c
> +++ b/net/caif/cfpkt_skbuff.c
> @@ -319,16 +319,12 @@ struct cfpkt *cfpkt_append(struct cfpkt *dstpkt,
>                 if (tmppkt == NULL)
>                         return NULL;
>                 tmp = pkt_to_skb(tmppkt);
> -               skb_set_tail_pointer(tmp, dstlen);
> -               tmp->len = dstlen;
> -               memcpy(tmp->data, dst->data, dstlen);
> +               skb_put_data(tmp, dst->data, dstlen);
>                 cfpkt_destroy(dstpkt);
>                 dst = tmp;
>         }
> -       memcpy(skb_tail_pointer(dst), add->data, skb_headlen(add));
> +       skb_put_data(dst, add->data, skb_headlen(add));
>         cfpkt_destroy(addpkt);
> -       dst->tail += addlen;
> -       dst->len += addlen;
>         return skb_to_pkt(dst);
>  }
>
> @@ -359,13 +355,11 @@ struct cfpkt *cfpkt_split(struct cfpkt *pkt, u16 pos)
>         if (skb2 == NULL)
>                 return NULL;
>
> +       skb_put_data(skb2, split, len2nd);
> +
>         /* Reduce the length of the original packet */
> -       skb_set_tail_pointer(skb, pos);
> -       skb->len = pos;
> +       skb_trim(skb, pos);
>
> -       memcpy(skb2->data, split, len2nd);
> -       skb2->tail += len2nd;
> -       skb2->len += len2nd;
>         skb2->priority = skb->priority;
>         return skb_to_pkt(skb2);
>  }
> --
> 2.21.0.rc0.258.g878e2cd30e-goog
>

^ permalink raw reply

* Re: [RESEND PATCH net] mm: page_alloc: fix ref bias in page_frag_alloc() for 1-byte allocs
From: David Miller @ 2019-02-14 22:21 UTC (permalink / raw)
  To: jannh; +Cc: alexander.duyck, netdev, linux-kernel
In-Reply-To: <CAG48ez3p9kyyON4qDybveNhCNdTPfsvi3hUp7rDQUk-f43xMsQ@mail.gmail.com>

From: Jann Horn <jannh@google.com>
Date: Thu, 14 Feb 2019 22:26:22 +0100

> On Thu, Feb 14, 2019 at 6:13 PM David Miller <davem@davemloft.net> wrote:
>>
>> From: Jann Horn <jannh@google.com>
>> Date: Wed, 13 Feb 2019 22:45:59 +0100
>>
>> > The basic idea behind ->pagecnt_bias is: If we pre-allocate the maximum
>> > number of references that we might need to create in the fastpath later,
>> > the bump-allocation fastpath only has to modify the non-atomic bias value
>> > that tracks the number of extra references we hold instead of the atomic
>> > refcount. The maximum number of allocations we can serve (under the
>> > assumption that no allocation is made with size 0) is nc->size, so that's
>> > the bias used.
>> >
>> > However, even when all memory in the allocation has been given away, a
>> > reference to the page is still held; and in the `offset < 0` slowpath, the
>> > page may be reused if everyone else has dropped their references.
>> > This means that the necessary number of references is actually
>> > `nc->size+1`.
>> >
>> > Luckily, from a quick grep, it looks like the only path that can call
>> > page_frag_alloc(fragsz=1) is TAP with the IFF_NAPI_FRAGS flag, which
>> > requires CAP_NET_ADMIN in the init namespace and is only intended to be
>> > used for kernel testing and fuzzing.
>> >
>> > To test for this issue, put a `WARN_ON(page_ref_count(page) == 0)` in the
>> > `offset < 0` path, below the virt_to_page() call, and then repeatedly call
>> > writev() on a TAP device with IFF_TAP|IFF_NO_PI|IFF_NAPI_FRAGS|IFF_NAPI,
>> > with a vector consisting of 15 elements containing 1 byte each.
>> >
>> > Signed-off-by: Jann Horn <jannh@google.com>
>>
>> Applied and queued up for -stable.
> 
> I had sent a v2 at Alexander Duyck's request an hour before you
> applied the patch (with a minor difference that, in Alexander's
> opinion, might be slightly more efficient). I guess the net tree
> doesn't work like the mm tree, where patches can get removed and
> replaced with newer versions? So if Alexander wants that change
> (s/size/PAGE_FRAG_CACHE_MAX_SIZE/ in the refcount), someone has to
> send that as a separate patch?

Yes, please send a follow-up.  Sorry about that.

^ permalink raw reply

* Re: [PATCH] can: mark expected switch fall-throughs
From: Alexandre Belloni @ 2019-02-14 22:17 UTC (permalink / raw)
  To: Gustavo A. R. Silva
  Cc: Nicolas.Ferre, wg, mkl, davem, Ludovic.Desroches, linux-can,
	netdev, linux-arm-kernel, linux-kernel
In-Reply-To: <daae008e-35fd-7553-fcb2-7932fe53b959@embeddedor.com>

Hi,

On 14/02/2019 15:37:26-0600, Gustavo A. R. Silva wrote:
> 
> 
> On 1/30/19 2:11 AM, Nicolas.Ferre@microchip.com wrote:
> > On 29/01/2019 at 19:06, Gustavo A. R. Silva wrote:
> >> In preparation to enabling -Wimplicit-fallthrough, mark switch cases
> >> where we are expecting to fall through.
> >>
> >> This patch fixes the following warnings:
> >>
> >> drivers/net/can/peak_canfd/peak_pciefd_main.c:668:3: warning: this statement may fall through [-Wimplicit-fallthrough=]
> >> drivers/net/can/spi/mcp251x.c:875:7: warning: this statement may fall through [-Wimplicit-fallthrough=]
> >> drivers/net/can/usb/peak_usb/pcan_usb.c:422:6: warning: this statement may fall through [-Wimplicit-fallthrough=]
> >> drivers/net/can/at91_can.c:895:6: warning: this statement may fall through [-Wimplicit-fallthrough=]
> >> drivers/net/can/at91_can.c:953:15: warning: this statement may fall through [-Wimplicit-fallthrough=]
> >> drivers/net/can/usb/peak_usb/pcan_usb.c: In function ‘pcan_usb_decode_error’:
> >> drivers/net/can/usb/peak_usb/pcan_usb.c:422:6: warning: this statement may fall through [-Wimplicit-fallthrough=]
> >>     if (n & PCAN_USB_ERROR_BUS_LIGHT) {
> >>        ^
> >> drivers/net/can/usb/peak_usb/pcan_usb.c:428:2: note: here
> >>    case CAN_STATE_ERROR_WARNING:
> >>    ^~~~
> >>
> >> Warning level 3 was used: -Wimplicit-fallthrough=3
> >>
> >> This patch is part of the ongoing efforts to enabling
> >> -Wimplicit-fallthrough.
> >>
> >> Notice that in some cases spelling mistakes were fixed.
> >> In other cases, the /* fall through */ comment is placed
> >> at the bottom of the case statement, which is what GCC
> >> is expecting to find.
> >>
> >> Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
> >> ---
> >>   drivers/net/can/at91_can.c                    | 6 ++++--
> > 
> > For this one:
> > Acked-by: Nicolas Ferre <nicolas.ferre@microchip.com>
> > 
> 
> Thanks, Nicolas.
> 

I though I had a déjà vu but you actually sent the at91 part twice.

> Dave:
> 
> I wonder if you can take this patch.
> 
> Thanks
> --
> Gustavo
> 
> >>   drivers/net/can/peak_canfd/peak_pciefd_main.c | 2 +-
> >>   drivers/net/can/spi/mcp251x.c                 | 3 ++-
> >>   drivers/net/can/usb/peak_usb/pcan_usb.c       | 2 +-
> >>   4 files changed, 8 insertions(+), 5 deletions(-)
> >>
> >> diff --git a/drivers/net/can/at91_can.c b/drivers/net/can/at91_can.c
> >> index d98c69045b17..1718c20f9c99 100644
> >> --- a/drivers/net/can/at91_can.c
> >> +++ b/drivers/net/can/at91_can.c
> >> @@ -902,7 +902,8 @@ static void at91_irq_err_state(struct net_device *dev,
> >>   				CAN_ERR_CRTL_TX_WARNING :
> >>   				CAN_ERR_CRTL_RX_WARNING;
> >>   		}
> >> -	case CAN_STATE_ERROR_WARNING:	/* fallthrough */
> >> +		/* fall through */
> >> +	case CAN_STATE_ERROR_WARNING:
> >>   		/*
> >>   		 * from: ERROR_ACTIVE, ERROR_WARNING
> >>   		 * to  : ERROR_PASSIVE, BUS_OFF
> >> @@ -951,7 +952,8 @@ static void at91_irq_err_state(struct net_device *dev,
> >>   		netdev_dbg(dev, "Error Active\n");
> >>   		cf->can_id |= CAN_ERR_PROT;
> >>   		cf->data[2] = CAN_ERR_PROT_ACTIVE;
> >> -	case CAN_STATE_ERROR_WARNING:	/* fallthrough */

Seriously, for that one, you should fix the compiler. The fall through
is not implicit, it is actually quite explicit and the warning is simply
wrong.

Also, the gcc documentation says that -Wimplicit-fallthrough=3
recognizes /* fallthrough */ as a proper fall through comment (and I
tested with gcc 8.2).

The matching regex is [ \t.!]*([Ee]lse,? |[Ii]ntentional(ly)? )?
fall(s | |-)?thr(ough|u)[ \t.!]*(-[^\n\r]*)?

-- 
Alexandre Belloni, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

^ permalink raw reply

* Re: [PATCH v3] arm64: dts: lx2160aqds: Add mdio mux nodes
From: Li Yang @ 2019-02-14 22:32 UTC (permalink / raw)
  To: Pankaj Bansal, Rob Herring
  Cc: Shawn Guo, Andrew Lunn, Florian Fainelli, netdev@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org
In-Reply-To: <CADRPPNTEE7beTyR-3ezSBuaNm-9vMAiUVKWYUkr_gd8nswMSYg@mail.gmail.com>

On Tue, Feb 12, 2019 at 12:01 PM Li Yang <leoyang.li@nxp.com> wrote:
>
> On Mon, Feb 11, 2019 at 9:28 PM Pankaj Bansal <pankaj.bansal@nxp.com> wrote:
> >
> >
> >
> > > -----Original Message-----
> > > From: Leo Li
> > > Sent: Tuesday, 12 February, 2019 02:14 AM
> > > To: Shawn Guo <shawnguo@kernel.org>; Pankaj Bansal
> > > <pankaj.bansal@nxp.com>
> > > Cc: Andrew Lunn <andrew@lunn.ch>; Florian Fainelli <f.fainelli@gmail.com>;
> > > netdev@vger.kernel.org; linux-arm-kernel@lists.infradead.org
> > > Subject: RE: [PATCH v3] arm64: dts: lx2160aqds: Add mdio mux nodes
> > >
> > >
> > >
> > > > -----Original Message-----
> > > > From: Shawn Guo <shawnguo@kernel.org>
> > > > Sent: Sunday, February 10, 2019 9:00 PM
> > > > To: Pankaj Bansal <pankaj.bansal@nxp.com>
> > > > Cc: Leo Li <leoyang.li@nxp.com>; Andrew Lunn <andrew@lunn.ch>; Florian
> > > > Fainelli <f.fainelli@gmail.com>; netdev@vger.kernel.org; linux-arm-
> > > > kernel@lists.infradead.org
> > > > Subject: Re: [PATCH v3] arm64: dts: lx2160aqds: Add mdio mux nodes
> > > >
> > > > On Wed, Feb 06, 2019 at 09:40:33AM +0000, Pankaj Bansal wrote:
> > > > > The two external MDIO buses used to communicate with phy devices
> > > > > that are external to SOC are muxed in LX2160AQDS board.
> > > > >
> > > > > These buses can be routed to any one of the eight IO slots on
> > > > > LX2160AQDS board depending on value in fpga register 0x54.
> > > > >
> > > > > Additionally the external MDIO1 is used to communicate to the
> > > > > onboard RGMII phy devices.
> > > > >
> > > > > The mdio1 is controlled by bits 4-7 of fpga register and mdio2 is
> > > > > controlled by bits 0-3 of fpga register.
> > > > >
> > > > > Signed-off-by: Pankaj Bansal <pankaj.bansal@nxp.com>
> > > > > ---
> > > > >
> > > > > Notes:
> > > > >     V3:
> > > > >     - Add status = disabled in soc file and status = okay in board file
> > > > >       for external MDIO nodes
> > > > >     - Add interrupts property in external mdio nodes in soc file
> > > > >     V2:
> > > > >     - removed unnecassary TODO statements
> > > > >     - removed device_type from mdio nodes
> > > > >     - change the case of hex number to lowercase
> > > > >     - removed board specific comments from soc file
> > > > >
> > > > >  .../boot/dts/freescale/fsl-lx2160a-qds.dts   | 123 +++++++++++++++++
> > > > >  .../boot/dts/freescale/fsl-lx2160a.dtsi      |  22 +++
> > > > >  2 files changed, 145 insertions(+)
> > > > >
> > > > > diff --git a/arch/arm64/boot/dts/freescale/fsl-lx2160a-qds.dts
> > > > > b/arch/arm64/boot/dts/freescale/fsl-lx2160a-qds.dts
> > > > > index 99a22abbe725..079264b391a2 100644
> > > > > --- a/arch/arm64/boot/dts/freescale/fsl-lx2160a-qds.dts
> > > > > +++ b/arch/arm64/boot/dts/freescale/fsl-lx2160a-qds.dts
> > > > > @@ -35,6 +35,14 @@
> > > > >   status = "okay";
> > > > >  };
> > > > >
> > > > > +&emdio1 {
> > > > > + status = "okay";
> > > > > +};
> > > > > +
> > > > > +&emdio2 {
> > > > > + status = "okay";
> > > > > +};
> > > > > +
> > > > >  &esdhc0 {
> > > > >   status = "okay";
> > > > >  };
> > > > > @@ -46,6 +54,121 @@
> > > > >  &i2c0 {
> > > > >   status = "okay";
> > > > >
> > > > > + fpga@66 {
> > > > > +         compatible = "fsl,lx2160aqds-fpga", "fsl,fpga-qixis-i2c";
> > > > > +         reg = <0x66>;
> > > > > +         #address-cells = <1>;
> > > > > +         #size-cells = <0>;
> > > > > +
> > > > > +         mdio-mux-1@54 {
> > > > > +                 mdio-parent-bus = <&emdio1>;
> > > > > +                 reg = <0x54>;            /* BRDCFG4 */
> > > > > +                 mux-mask = <0xf8>;      /* EMI1_MDIO */
> > > > > +                 #address-cells=<1>;
> > > > > +                 #size-cells = <0>;
> > > > > +
> > > > > +                 mdio@0 {
> > > > > +                         reg = <0x00>;
> > > > > +                         #address-cells = <1>;
> > > > > +                         #size-cells = <0>;
> > > > > +                 };
> > > >
> > > > Please have a newline between nodes.  It doesn't deserve a respin
> > > > though.  I can fix them up when applying if Leo is fine with this version.
> > >
> > > I think there should be a compatible string defined for the binding of parent
> > > node mdio-mux, probably "mdio-mux-regmap", and be used here in the device
> > > tree.
> >
> > I have two concerns :
> > 1. The regmap is linux s/w construct, while device tree is h/w representation and is s/w agnostic. can we use regmap in device tree?
>
> Well, if we want to avoid using the regmap name, we probably can try
> "mdio-mux-reg" or "mdio-mux-syscon"?  With further search I also found
> a more generic mux binding at Documentation/devicetree/bindings/mux/,
> would be even better if we can use that to describe the mux.


To make it more clear, with the use of
Documentation/devicetree/bindings/mux/ binding, I think it would end
up with something like this:

i2c {
 fpga {
  compatible = "fsl,lx2160aqds-fpga", "syscon";
  ....
  mux: mux-controller {
   compatible = "mmio-mux"
   ...
  }
 }
...
}

mdio-mux {
 compatible = "mdio-mux"
 mux-controls = <&mux 0>;
 ....
 mdio {
  phy {
  }
  ...
 }
 mdio {
  ...
 }
 ...
}


}"

>
> > 2. By convention the device tree compatible binding is defined as "<manufacturer>,<model>" e.g. "fsl,mpc8349-uart". The mdio-mux node and it's sub nodes are a generic representation of mdio mux and it is not dependent on a particular manufacturer device. How to define the compatible in this case?
>
> The manufacturer prefix is for vendor specific bindings.  If the
> binding a suppose to be generic, we don't need the vendor prefix.
>
> >
> > >
> > > >
> > > > Shawn
> > > >
> > > > > +                 mdio@40 {
> > > > > +                         reg = <0x40>;
> > > > > +                         #address-cells = <1>;
> > > > > +                         #size-cells = <0>;
> > > > > +                 };
> > > > > +                 mdio@c0 {
> > > > > +                         reg = <0xc0>;
> > > > > +                         #address-cells = <1>;
> > > > > +                         #size-cells = <0>;
> > > > > +                 };
> > > > > +                 mdio@c8 {
> > > > > +                         reg = <0xc8>;
> > > > > +                         #address-cells = <1>;
> > > > > +                         #size-cells = <0>;
> > > > > +                 };
> > > > > +                 mdio@d0 {
> > > > > +                         reg = <0xd0>;
> > > > > +                         #address-cells = <1>;
> > > > > +                         #size-cells = <0>;
> > > > > +                 };
> > > > > +                 mdio@d8 {
> > > > > +                         reg = <0xd8>;
> > > > > +                         #address-cells = <1>;
> > > > > +                         #size-cells = <0>;
> > > > > +                 };
> > > > > +                 mdio@e0 {
> > > > > +                         reg = <0xe0>;
> > > > > +                         #address-cells = <1>;
> > > > > +                         #size-cells = <0>;
> > > > > +                 };
> > > > > +                 mdio@e8 {
> > > > > +                         reg = <0xe8>;
> > > > > +                         #address-cells = <1>;
> > > > > +                         #size-cells = <0>;
> > > > > +                 };
> > > > > +                 mdio@f0 {
> > > > > +                         reg = <0xf0>;
> > > > > +                         #address-cells = <1>;
> > > > > +                         #size-cells = <0>;
> > > > > +                 };
> > > > > +                 mdio@f8 {
> > > > > +                         reg = <0xf8>;
> > > > > +                         #address-cells = <1>;
> > > > > +                         #size-cells = <0>;
> > > > > +                 };
> > > > > +         };
> > > > > +
> > > > > +         mdio-mux-2@54 {
> > > > > +                 mdio-parent-bus = <&emdio2>;
> > > > > +                 reg = <0x54>;            /* BRDCFG4 */
> > > > > +                 mux-mask = <0x07>;      /* EMI2_MDIO */
> > > > > +                 #address-cells=<1>;
> > > > > +                 #size-cells = <0>;
> > > > > +
> > > > > +                 mdio@0 {
> > > > > +                         reg = <0x00>;
> > > > > +                         #address-cells = <1>;
> > > > > +                         #size-cells = <0>;
> > > > > +                 };
> > > > > +                 mdio@1 {
> > > > > +                         reg = <0x01>;
> > > > > +                         #address-cells = <1>;
> > > > > +                         #size-cells = <0>;
> > > > > +                 };
> > > > > +                 mdio@2 {
> > > > > +                         reg = <0x02>;
> > > > > +                         #address-cells = <1>;
> > > > > +                         #size-cells = <0>;
> > > > > +                 };
> > > > > +                 mdio@3 {
> > > > > +                         reg = <0x03>;
> > > > > +                         #address-cells = <1>;
> > > > > +                         #size-cells = <0>;
> > > > > +                 };
> > > > > +                 mdio@4 {
> > > > > +                         reg = <0x04>;
> > > > > +                         #address-cells = <1>;
> > > > > +                         #size-cells = <0>;
> > > > > +                 };
> > > > > +                 mdio@5 {
> > > > > +                         reg = <0x05>;
> > > > > +                         #address-cells = <1>;
> > > > > +                         #size-cells = <0>;
> > > > > +                 };
> > > > > +                 mdio@6 {
> > > > > +                         reg = <0x06>;
> > > > > +                         #address-cells = <1>;
> > > > > +                         #size-cells = <0>;
> > > > > +                 };
> > > > > +                 mdio@7 {
> > > > > +                         reg = <0x07>;
> > > > > +                         #address-cells = <1>;
> > > > > +                         #size-cells = <0>;
> > > > > +                 };
> > > > > +         };
> > > > > + };
> > > > > +
> > > > >   i2c-mux@77 {
> > > > >           compatible = "nxp,pca9547";
> > > > >           reg = <0x77>;
> > > > > diff --git a/arch/arm64/boot/dts/freescale/fsl-lx2160a.dtsi
> > > > > b/arch/arm64/boot/dts/freescale/fsl-lx2160a.dtsi
> > > > > index a79f5c1ea56d..7def5252ac1a 100644
> > > > > --- a/arch/arm64/boot/dts/freescale/fsl-lx2160a.dtsi
> > > > > +++ b/arch/arm64/boot/dts/freescale/fsl-lx2160a.dtsi
> > > > > @@ -762,5 +762,27 @@
> > > > >                                <GIC_SPI 209 IRQ_TYPE_LEVEL_HIGH>;
> > > > >                   dma-coherent;
> > > > >           };
> > > > > +
> > > > > +         /* WRIOP0: 0x8b8_0000, E-MDIO1: 0x1_6000 */
> > > > > +         emdio1: mdio@8b96000 {
> > > > > +                 compatible = "fsl,fman-memac-mdio";
> > > > > +                 reg = <0x0 0x8b96000 0x0 0x1000>;
> > > > > +                 interrupts = <GIC_SPI 90 IRQ_TYPE_LEVEL_HIGH>;
> > > > > +                 #address-cells = <1>;
> > > > > +                 #size-cells = <0>;
> > > > > +                 little-endian;  /* force the driver in LE mode */
> > > > > +                 status = "disabled";
> > > > > +         };
> > > > > +
> > > > > +         /* WRIOP0: 0x8b8_0000, E-MDIO2: 0x1_7000 */
> > > > > +         emdio2: mdio@8b97000 {
> > > > > +                 compatible = "fsl,fman-memac-mdio";
> > > > > +                 reg = <0x0 0x8b97000 0x0 0x1000>;
> > > > > +                 interrupts = <GIC_SPI 91 IRQ_TYPE_LEVEL_HIGH>;
> > > > > +                 #address-cells = <1>;
> > > > > +                 #size-cells = <0>;
> > > > > +                 little-endian;  /* force the driver in LE mode */
> > > > > +                 status = "disabled";
> > > > > +         };
> > > > >   };
> > > > >  };
> > > > > --
> > > > > 2.17.1
> > > > >

^ permalink raw reply

* [PATCH net-next] nfp: flower: fix masks for tcp and ip flags fields
From: Jakub Kicinski @ 2019-02-14 22:37 UTC (permalink / raw)
  To: davem; +Cc: netdev, oss-drivers, Pieter Jansen van Vuuren

From: Pieter Jansen van Vuuren <pieter.jansenvanvuuren@netronome.com>

Check mask fields of tcp and ip flags when setting the corresponding mask
flag used in hardware.

Fixes: 8f2566225ae2 ("flow_offload: add flow_rule and flow_match")
Signed-off-by: Pieter Jansen van Vuuren <pieter.jansenvanvuuren@netronome.com>
Reviewed-by: John Hurley <john.hurley@netronome.com>
Reviewed-by: Jakub Kicinski <jakub.kicinski@netronome.com>
---
 .../net/ethernet/netronome/nfp/flower/match.c | 35 +++++++++++--------
 1 file changed, 20 insertions(+), 15 deletions(-)

diff --git a/drivers/net/ethernet/netronome/nfp/flower/match.c b/drivers/net/ethernet/netronome/nfp/flower/match.c
index 1279fa5da9e1..e03c8ef2c28c 100644
--- a/drivers/net/ethernet/netronome/nfp/flower/match.c
+++ b/drivers/net/ethernet/netronome/nfp/flower/match.c
@@ -172,46 +172,51 @@ nfp_flower_compile_ip_ext(struct nfp_flower_ip_ext *ext,
 	}
 
 	if (flow_rule_match_key(rule, FLOW_DISSECTOR_KEY_TCP)) {
+		u16 tcp_flags, tcp_flags_mask;
 		struct flow_match_tcp match;
-		u16 tcp_flags;
 
 		flow_rule_match_tcp(rule, &match);
 		tcp_flags = be16_to_cpu(match.key->flags);
+		tcp_flags_mask = be16_to_cpu(match.mask->flags);
 
-		if (tcp_flags & TCPHDR_FIN) {
+		if (tcp_flags & TCPHDR_FIN)
 			ext->flags |= NFP_FL_TCP_FLAG_FIN;
+		if (tcp_flags_mask & TCPHDR_FIN)
 			msk->flags |= NFP_FL_TCP_FLAG_FIN;
-		}
-		if (tcp_flags & TCPHDR_SYN) {
+
+		if (tcp_flags & TCPHDR_SYN)
 			ext->flags |= NFP_FL_TCP_FLAG_SYN;
+		if (tcp_flags_mask & TCPHDR_SYN)
 			msk->flags |= NFP_FL_TCP_FLAG_SYN;
-		}
-		if (tcp_flags & TCPHDR_RST) {
+
+		if (tcp_flags & TCPHDR_RST)
 			ext->flags |= NFP_FL_TCP_FLAG_RST;
+		if (tcp_flags_mask & TCPHDR_RST)
 			msk->flags |= NFP_FL_TCP_FLAG_RST;
-		}
-		if (tcp_flags & TCPHDR_PSH) {
+
+		if (tcp_flags & TCPHDR_PSH)
 			ext->flags |= NFP_FL_TCP_FLAG_PSH;
+		if (tcp_flags_mask & TCPHDR_PSH)
 			msk->flags |= NFP_FL_TCP_FLAG_PSH;
-		}
-		if (tcp_flags & TCPHDR_URG) {
+
+		if (tcp_flags & TCPHDR_URG)
 			ext->flags |= NFP_FL_TCP_FLAG_URG;
+		if (tcp_flags_mask & TCPHDR_URG)
 			msk->flags |= NFP_FL_TCP_FLAG_URG;
-		}
 	}
 
 	if (flow_rule_match_key(rule, FLOW_DISSECTOR_KEY_CONTROL)) {
 		struct flow_match_control match;
 
 		flow_rule_match_control(rule, &match);
-		if (match.key->flags & FLOW_DIS_IS_FRAGMENT) {
+		if (match.key->flags & FLOW_DIS_IS_FRAGMENT)
 			ext->flags |= NFP_FL_IP_FRAGMENTED;
+		if (match.mask->flags & FLOW_DIS_IS_FRAGMENT)
 			msk->flags |= NFP_FL_IP_FRAGMENTED;
-		}
-		if (match.key->flags & FLOW_DIS_FIRST_FRAG) {
+		if (match.key->flags & FLOW_DIS_FIRST_FRAG)
 			ext->flags |= NFP_FL_IP_FRAG_FIRST;
+		if (match.mask->flags & FLOW_DIS_FIRST_FRAG)
 			msk->flags |= NFP_FL_IP_FRAG_FIRST;
-		}
 	}
 }
 
-- 
2.19.2


^ permalink raw reply related

* [PATCH bpf-next 2/2] libbpf: Introduce bpf_object__btf
From: Andrey Ignatov @ 2019-02-14 23:01 UTC (permalink / raw)
  To: netdev; +Cc: Andrey Ignatov, ast, daniel, kernel-team
In-Reply-To: <cover.1550185216.git.rdna@fb.com>

Add new accessor for bpf_object to get opaque struct btf * from it.

struct btf * is needed for all operations with BTF and it's present in
bpf_object. The only thing missing is a way to get it.

Example use-case is to get BTF key_type_id an value_type_id for a map in
bpf_object. It can be done with btf__get_map_kv_tids() but that function
requires struct btf *.

Similar API can be added for struct btf_ext but no use-case for it yet.

Signed-off-by: Andrey Ignatov <rdna@fb.com>
---
 tools/lib/bpf/libbpf.c   | 5 +++++
 tools/lib/bpf/libbpf.h   | 3 +++
 tools/lib/bpf/libbpf.map | 1 +
 3 files changed, 9 insertions(+)

diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 690e7b079202..a77a327c2bb8 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -2332,6 +2332,11 @@ unsigned int bpf_object__kversion(struct bpf_object *obj)
 	return obj ? obj->kern_version : 0;
 }
 
+struct btf *bpf_object__btf(struct bpf_object *obj)
+{
+	return obj ? obj->btf : NULL;
+}
+
 int bpf_object__btf_fd(const struct bpf_object *obj)
 {
 	return obj->btf ? btf__fd(obj->btf) : -1;
diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
index 987fd92661d6..6c0168f8bba5 100644
--- a/tools/lib/bpf/libbpf.h
+++ b/tools/lib/bpf/libbpf.h
@@ -89,6 +89,9 @@ LIBBPF_API int bpf_object__load(struct bpf_object *obj);
 LIBBPF_API int bpf_object__unload(struct bpf_object *obj);
 LIBBPF_API const char *bpf_object__name(struct bpf_object *obj);
 LIBBPF_API unsigned int bpf_object__kversion(struct bpf_object *obj);
+
+struct btf;
+LIBBPF_API struct btf *bpf_object__btf(struct bpf_object *obj);
 LIBBPF_API int bpf_object__btf_fd(const struct bpf_object *obj);
 
 LIBBPF_API struct bpf_program *
diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map
index 16f342c3d4bc..99dfa710c818 100644
--- a/tools/lib/bpf/libbpf.map
+++ b/tools/lib/bpf/libbpf.map
@@ -132,6 +132,7 @@ LIBBPF_0.0.2 {
 		bpf_probe_prog_type;
 		bpf_map__resize;
 		bpf_map_lookup_elem_flags;
+		bpf_object__btf;
 		bpf_object__find_map_fd_by_name;
 		bpf_get_link_xdp_id;
 		btf__dedup;
-- 
2.17.1


^ permalink raw reply related

* [PATCH bpf-next 1/2] libbpf: Introduce bpf_map__resize
From: Andrey Ignatov @ 2019-02-14 23:01 UTC (permalink / raw)
  To: netdev; +Cc: Andrey Ignatov, ast, daniel, kernel-team
In-Reply-To: <cover.1550185216.git.rdna@fb.com>

Add bpf_map__resize() to change max_entries for a map.

Quite often necessary map size is unknown at compile time and can be
calculated only at run time.

Currently the following approach is used to do so:
* bpf_object__open_buffer() to open Elf file from a buffer;
* bpf_object__find_map_by_name() to find relevant map;
* bpf_map__def() to get map attributes and create struct
  bpf_create_map_attr from them;
* update max_entries in bpf_create_map_attr;
* bpf_create_map_xattr() to create new map with updated max_entries;
* bpf_map__reuse_fd() to replace the map in bpf_object with newly
  created one.

And after all this bpf_object can finally be loaded. The map will have
new size.

It 1) is quite a lot of steps; 2) doesn't take BTF into account.

For "2)" even more steps should be made and some of them require changes
to libbpf (e.g. to get struct btf * from bpf_object).

Instead the whole problem can be solved by introducing simple
bpf_map__resize() API that checks the map and sets new max_entries if
the map is not loaded yet.

So the new steps are:
* bpf_object__open_buffer() to open Elf file from a buffer;
* bpf_object__find_map_by_name() to find relevant map;
* bpf_map__resize() to update max_entries.

That's much simpler and works with BTF.

Signed-off-by: Andrey Ignatov <rdna@fb.com>
---
 tools/lib/bpf/libbpf.c   | 14 ++++++++++++++
 tools/lib/bpf/libbpf.h   |  1 +
 tools/lib/bpf/libbpf.map |  1 +
 3 files changed, 16 insertions(+)

diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index e3c39edfb9d3..690e7b079202 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -1114,6 +1114,20 @@ int bpf_map__reuse_fd(struct bpf_map *map, int fd)
 	return -errno;
 }
 
+int bpf_map__resize(struct bpf_map *map, __u32 max_entries)
+{
+	if (!map || !max_entries)
+		return -EINVAL;
+
+	/* If map already created, its attributes can't be changed. */
+	if (map->fd >= 0)
+		return -EBUSY;
+
+	map->def.max_entries = max_entries;
+
+	return 0;
+}
+
 static int
 bpf_object__probe_name(struct bpf_object *obj)
 {
diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
index 69a7c25eaccc..987fd92661d6 100644
--- a/tools/lib/bpf/libbpf.h
+++ b/tools/lib/bpf/libbpf.h
@@ -294,6 +294,7 @@ LIBBPF_API int bpf_map__set_priv(struct bpf_map *map, void *priv,
 				 bpf_map_clear_priv_t clear_priv);
 LIBBPF_API void *bpf_map__priv(struct bpf_map *map);
 LIBBPF_API int bpf_map__reuse_fd(struct bpf_map *map, int fd);
+LIBBPF_API int bpf_map__resize(struct bpf_map *map, __u32 max_entries);
 LIBBPF_API bool bpf_map__is_offload_neutral(struct bpf_map *map);
 LIBBPF_API void bpf_map__set_ifindex(struct bpf_map *map, __u32 ifindex);
 LIBBPF_API int bpf_map__pin(struct bpf_map *map, const char *path);
diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map
index 5fc8222209f8..16f342c3d4bc 100644
--- a/tools/lib/bpf/libbpf.map
+++ b/tools/lib/bpf/libbpf.map
@@ -130,6 +130,7 @@ LIBBPF_0.0.2 {
 		bpf_probe_helper;
 		bpf_probe_map_type;
 		bpf_probe_prog_type;
+		bpf_map__resize;
 		bpf_map_lookup_elem_flags;
 		bpf_object__find_map_fd_by_name;
 		bpf_get_link_xdp_id;
-- 
2.17.1


^ permalink raw reply related

* [PATCH bpf-next 0/2] libbpf: Add new interfaces
From: Andrey Ignatov @ 2019-02-14 23:01 UTC (permalink / raw)
  To: netdev; +Cc: Andrey Ignatov, ast, daniel, kernel-team

The patch set adds a couple of new interfaces to libbpf:

Patch 1 adds bpf_map__resize() to resize map before loading bpf_object;
Patch 2 adds bpf_object__btf() to get struct btf * from bpf_object__btf.


Andrey Ignatov (2):
  libbpf: Introduce bpf_map__resize
  libbpf: Introduce bpf_object__btf

 tools/lib/bpf/libbpf.c   | 19 +++++++++++++++++++
 tools/lib/bpf/libbpf.h   |  4 ++++
 tools/lib/bpf/libbpf.map |  2 ++
 3 files changed, 25 insertions(+)

-- 
2.17.1


^ permalink raw reply

* Re: [PATCH] can: mark expected switch fall-throughs
From: Gustavo A. R. Silva @ 2019-02-14 23:04 UTC (permalink / raw)
  To: Alexandre Belloni
  Cc: Nicolas.Ferre, wg, mkl, davem, Ludovic.Desroches, linux-can,
	netdev, linux-arm-kernel, linux-kernel, Kees Cook
In-Reply-To: <20190214221703.GQ10129@piout.net>



On 2/14/19 4:17 PM, Alexandre Belloni wrote:
> Hi,
> 
> On 14/02/2019 15:37:26-0600, Gustavo A. R. Silva wrote:
>>
>>
>> On 1/30/19 2:11 AM, Nicolas.Ferre@microchip.com wrote:
>>> On 29/01/2019 at 19:06, Gustavo A. R. Silva wrote:
>>>> In preparation to enabling -Wimplicit-fallthrough, mark switch cases
>>>> where we are expecting to fall through.
>>>>
>>>> This patch fixes the following warnings:
>>>>
>>>> drivers/net/can/peak_canfd/peak_pciefd_main.c:668:3: warning: this statement may fall through [-Wimplicit-fallthrough=]
>>>> drivers/net/can/spi/mcp251x.c:875:7: warning: this statement may fall through [-Wimplicit-fallthrough=]
>>>> drivers/net/can/usb/peak_usb/pcan_usb.c:422:6: warning: this statement may fall through [-Wimplicit-fallthrough=]
>>>> drivers/net/can/at91_can.c:895:6: warning: this statement may fall through [-Wimplicit-fallthrough=]
>>>> drivers/net/can/at91_can.c:953:15: warning: this statement may fall through [-Wimplicit-fallthrough=]
>>>> drivers/net/can/usb/peak_usb/pcan_usb.c: In function ‘pcan_usb_decode_error’:
>>>> drivers/net/can/usb/peak_usb/pcan_usb.c:422:6: warning: this statement may fall through [-Wimplicit-fallthrough=]
>>>>     if (n & PCAN_USB_ERROR_BUS_LIGHT) {
>>>>        ^
>>>> drivers/net/can/usb/peak_usb/pcan_usb.c:428:2: note: here
>>>>    case CAN_STATE_ERROR_WARNING:
>>>>    ^~~~
>>>>
>>>> Warning level 3 was used: -Wimplicit-fallthrough=3
>>>>
>>>> This patch is part of the ongoing efforts to enabling
>>>> -Wimplicit-fallthrough.
>>>>
>>>> Notice that in some cases spelling mistakes were fixed.
>>>> In other cases, the /* fall through */ comment is placed
>>>> at the bottom of the case statement, which is what GCC
>>>> is expecting to find.
>>>>
>>>> Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
>>>> ---
>>>>   drivers/net/can/at91_can.c                    | 6 ++++--
>>>
>>> For this one:
>>> Acked-by: Nicolas Ferre <nicolas.ferre@microchip.com>
>>>
>>
>> Thanks, Nicolas.
>>
> 
> I though I had a déjà vu but you actually sent the at91 part twice.
> 

It wasn't intentional.

>> Dave:
>>
>> I wonder if you can take this patch.
>>
>> Thanks
>> --
>> Gustavo
>>
>>>>   drivers/net/can/peak_canfd/peak_pciefd_main.c | 2 +-
>>>>   drivers/net/can/spi/mcp251x.c                 | 3 ++-
>>>>   drivers/net/can/usb/peak_usb/pcan_usb.c       | 2 +-
>>>>   4 files changed, 8 insertions(+), 5 deletions(-)
>>>>
>>>> diff --git a/drivers/net/can/at91_can.c b/drivers/net/can/at91_can.c
>>>> index d98c69045b17..1718c20f9c99 100644
>>>> --- a/drivers/net/can/at91_can.c
>>>> +++ b/drivers/net/can/at91_can.c
>>>> @@ -902,7 +902,8 @@ static void at91_irq_err_state(struct net_device *dev,
>>>>   				CAN_ERR_CRTL_TX_WARNING :
>>>>   				CAN_ERR_CRTL_RX_WARNING;
>>>>   		}
>>>> -	case CAN_STATE_ERROR_WARNING:	/* fallthrough */
>>>> +		/* fall through */
>>>> +	case CAN_STATE_ERROR_WARNING:
>>>>   		/*
>>>>   		 * from: ERROR_ACTIVE, ERROR_WARNING
>>>>   		 * to  : ERROR_PASSIVE, BUS_OFF
>>>> @@ -951,7 +952,8 @@ static void at91_irq_err_state(struct net_device *dev,
>>>>   		netdev_dbg(dev, "Error Active\n");
>>>>   		cf->can_id |= CAN_ERR_PROT;
>>>>   		cf->data[2] = CAN_ERR_PROT_ACTIVE;
>>>> -	case CAN_STATE_ERROR_WARNING:	/* fallthrough */
> 
> Seriously, for that one, you should fix the compiler. The fall through

I'll pass your feedback on to the GCC guys.

> is not implicit, it is actually quite explicit and the warning is simply
> wrong.
> 
> Also, the gcc documentation says that -Wimplicit-fallthrough=3
> recognizes /* fallthrough */ as a proper fall through comment (and I
> tested with gcc 8.2).
> 

Yeah. But that's not the relevant change in this case.  Notice that the
comment was moved to the very bottom of the previous case.

Thanks
--
Gustavo

^ permalink raw reply

* Re: [PATCH] can: mark expected switch fall-throughs
From: Alexandre Belloni @ 2019-02-14 23:07 UTC (permalink / raw)
  To: Gustavo A. R. Silva
  Cc: Nicolas.Ferre, wg, mkl, davem, Ludovic.Desroches, linux-can,
	netdev, linux-arm-kernel, linux-kernel, Kees Cook
In-Reply-To: <a8e53868-2b13-b050-7714-e3e24d18cf00@embeddedor.com>

On 14/02/2019 17:04:03-0600, Gustavo A. R. Silva wrote:
> 
> 
> On 2/14/19 4:17 PM, Alexandre Belloni wrote:
> > Hi,
> > 
> > On 14/02/2019 15:37:26-0600, Gustavo A. R. Silva wrote:
> >>
> >>
> >> On 1/30/19 2:11 AM, Nicolas.Ferre@microchip.com wrote:
> >>> On 29/01/2019 at 19:06, Gustavo A. R. Silva wrote:
> >>>> In preparation to enabling -Wimplicit-fallthrough, mark switch cases
> >>>> where we are expecting to fall through.
> >>>>
> >>>> This patch fixes the following warnings:
> >>>>
> >>>> drivers/net/can/peak_canfd/peak_pciefd_main.c:668:3: warning: this statement may fall through [-Wimplicit-fallthrough=]
> >>>> drivers/net/can/spi/mcp251x.c:875:7: warning: this statement may fall through [-Wimplicit-fallthrough=]
> >>>> drivers/net/can/usb/peak_usb/pcan_usb.c:422:6: warning: this statement may fall through [-Wimplicit-fallthrough=]
> >>>> drivers/net/can/at91_can.c:895:6: warning: this statement may fall through [-Wimplicit-fallthrough=]
> >>>> drivers/net/can/at91_can.c:953:15: warning: this statement may fall through [-Wimplicit-fallthrough=]
> >>>> drivers/net/can/usb/peak_usb/pcan_usb.c: In function ‘pcan_usb_decode_error’:
> >>>> drivers/net/can/usb/peak_usb/pcan_usb.c:422:6: warning: this statement may fall through [-Wimplicit-fallthrough=]
> >>>>     if (n & PCAN_USB_ERROR_BUS_LIGHT) {
> >>>>        ^
> >>>> drivers/net/can/usb/peak_usb/pcan_usb.c:428:2: note: here
> >>>>    case CAN_STATE_ERROR_WARNING:
> >>>>    ^~~~
> >>>>
> >>>> Warning level 3 was used: -Wimplicit-fallthrough=3
> >>>>
> >>>> This patch is part of the ongoing efforts to enabling
> >>>> -Wimplicit-fallthrough.
> >>>>
> >>>> Notice that in some cases spelling mistakes were fixed.
> >>>> In other cases, the /* fall through */ comment is placed
> >>>> at the bottom of the case statement, which is what GCC
> >>>> is expecting to find.
> >>>>
> >>>> Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
> >>>> ---
> >>>>   drivers/net/can/at91_can.c                    | 6 ++++--
> >>>
> >>> For this one:
> >>> Acked-by: Nicolas Ferre <nicolas.ferre@microchip.com>
> >>>
> >>
> >> Thanks, Nicolas.
> >>
> > 
> > I though I had a déjà vu but you actually sent the at91 part twice.
> > 
> 
> It wasn't intentional.
> 
> >> Dave:
> >>
> >> I wonder if you can take this patch.
> >>
> >> Thanks
> >> --
> >> Gustavo
> >>
> >>>>   drivers/net/can/peak_canfd/peak_pciefd_main.c | 2 +-
> >>>>   drivers/net/can/spi/mcp251x.c                 | 3 ++-
> >>>>   drivers/net/can/usb/peak_usb/pcan_usb.c       | 2 +-
> >>>>   4 files changed, 8 insertions(+), 5 deletions(-)
> >>>>
> >>>> diff --git a/drivers/net/can/at91_can.c b/drivers/net/can/at91_can.c
> >>>> index d98c69045b17..1718c20f9c99 100644
> >>>> --- a/drivers/net/can/at91_can.c
> >>>> +++ b/drivers/net/can/at91_can.c
> >>>> @@ -902,7 +902,8 @@ static void at91_irq_err_state(struct net_device *dev,
> >>>>   				CAN_ERR_CRTL_TX_WARNING :
> >>>>   				CAN_ERR_CRTL_RX_WARNING;
> >>>>   		}
> >>>> -	case CAN_STATE_ERROR_WARNING:	/* fallthrough */
> >>>> +		/* fall through */
> >>>> +	case CAN_STATE_ERROR_WARNING:
> >>>>   		/*
> >>>>   		 * from: ERROR_ACTIVE, ERROR_WARNING
> >>>>   		 * to  : ERROR_PASSIVE, BUS_OFF
> >>>> @@ -951,7 +952,8 @@ static void at91_irq_err_state(struct net_device *dev,
> >>>>   		netdev_dbg(dev, "Error Active\n");
> >>>>   		cf->can_id |= CAN_ERR_PROT;
> >>>>   		cf->data[2] = CAN_ERR_PROT_ACTIVE;
> >>>> -	case CAN_STATE_ERROR_WARNING:	/* fallthrough */
> > 
> > Seriously, for that one, you should fix the compiler. The fall through
> 
> I'll pass your feedback on to the GCC guys.
> 
> > is not implicit, it is actually quite explicit and the warning is simply
> > wrong.
> > 
> > Also, the gcc documentation says that -Wimplicit-fallthrough=3
> > recognizes /* fallthrough */ as a proper fall through comment (and I
> > tested with gcc 8.2).
> > 
> 
> Yeah. But that's not the relevant change in this case.  Notice that the
> comment was moved to the very bottom of the previous case.
> 

Yes and it doesn't matter for gcc, I tested with gcc 8.2.

-- 
Alexandre Belloni, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

^ permalink raw reply

* Re: [PATCH] can: mark expected switch fall-throughs
From: Gustavo A. R. Silva @ 2019-02-14 23:14 UTC (permalink / raw)
  To: Alexandre Belloni
  Cc: Nicolas.Ferre, wg, mkl, davem, Ludovic.Desroches, linux-can,
	netdev, linux-arm-kernel, linux-kernel, Kees Cook
In-Reply-To: <20190214230756.GR10129@piout.net>



On 2/14/19 5:07 PM, Alexandre Belloni wrote:
> On 14/02/2019 17:04:03-0600, Gustavo A. R. Silva wrote:
>>
>>
>> On 2/14/19 4:17 PM, Alexandre Belloni wrote:
>>> Hi,
>>>
>>> On 14/02/2019 15:37:26-0600, Gustavo A. R. Silva wrote:
>>>>
>>>>
>>>> On 1/30/19 2:11 AM, Nicolas.Ferre@microchip.com wrote:
>>>>> On 29/01/2019 at 19:06, Gustavo A. R. Silva wrote:
>>>>>> In preparation to enabling -Wimplicit-fallthrough, mark switch cases
>>>>>> where we are expecting to fall through.
>>>>>>
>>>>>> This patch fixes the following warnings:
>>>>>>
>>>>>> drivers/net/can/peak_canfd/peak_pciefd_main.c:668:3: warning: this statement may fall through [-Wimplicit-fallthrough=]
>>>>>> drivers/net/can/spi/mcp251x.c:875:7: warning: this statement may fall through [-Wimplicit-fallthrough=]
>>>>>> drivers/net/can/usb/peak_usb/pcan_usb.c:422:6: warning: this statement may fall through [-Wimplicit-fallthrough=]
>>>>>> drivers/net/can/at91_can.c:895:6: warning: this statement may fall through [-Wimplicit-fallthrough=]
>>>>>> drivers/net/can/at91_can.c:953:15: warning: this statement may fall through [-Wimplicit-fallthrough=]
>>>>>> drivers/net/can/usb/peak_usb/pcan_usb.c: In function ‘pcan_usb_decode_error’:
>>>>>> drivers/net/can/usb/peak_usb/pcan_usb.c:422:6: warning: this statement may fall through [-Wimplicit-fallthrough=]
>>>>>>     if (n & PCAN_USB_ERROR_BUS_LIGHT) {
>>>>>>        ^
>>>>>> drivers/net/can/usb/peak_usb/pcan_usb.c:428:2: note: here
>>>>>>    case CAN_STATE_ERROR_WARNING:
>>>>>>    ^~~~
>>>>>>
>>>>>> Warning level 3 was used: -Wimplicit-fallthrough=3
>>>>>>
>>>>>> This patch is part of the ongoing efforts to enabling
>>>>>> -Wimplicit-fallthrough.
>>>>>>
>>>>>> Notice that in some cases spelling mistakes were fixed.
>>>>>> In other cases, the /* fall through */ comment is placed
>>>>>> at the bottom of the case statement, which is what GCC
>>>>>> is expecting to find.
>>>>>>
>>>>>> Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
>>>>>> ---
>>>>>>   drivers/net/can/at91_can.c                    | 6 ++++--
>>>>>
>>>>> For this one:
>>>>> Acked-by: Nicolas Ferre <nicolas.ferre@microchip.com>
>>>>>
>>>>
>>>> Thanks, Nicolas.
>>>>
>>>
>>> I though I had a déjà vu but you actually sent the at91 part twice.
>>>
>>
>> It wasn't intentional.
>>
>>>> Dave:
>>>>
>>>> I wonder if you can take this patch.
>>>>
>>>> Thanks
>>>> --
>>>> Gustavo
>>>>
>>>>>>   drivers/net/can/peak_canfd/peak_pciefd_main.c | 2 +-
>>>>>>   drivers/net/can/spi/mcp251x.c                 | 3 ++-
>>>>>>   drivers/net/can/usb/peak_usb/pcan_usb.c       | 2 +-
>>>>>>   4 files changed, 8 insertions(+), 5 deletions(-)
>>>>>>
>>>>>> diff --git a/drivers/net/can/at91_can.c b/drivers/net/can/at91_can.c
>>>>>> index d98c69045b17..1718c20f9c99 100644
>>>>>> --- a/drivers/net/can/at91_can.c
>>>>>> +++ b/drivers/net/can/at91_can.c
>>>>>> @@ -902,7 +902,8 @@ static void at91_irq_err_state(struct net_device *dev,
>>>>>>   				CAN_ERR_CRTL_TX_WARNING :
>>>>>>   				CAN_ERR_CRTL_RX_WARNING;
>>>>>>   		}
>>>>>> -	case CAN_STATE_ERROR_WARNING:	/* fallthrough */
>>>>>> +		/* fall through */
>>>>>> +	case CAN_STATE_ERROR_WARNING:
>>>>>>   		/*
>>>>>>   		 * from: ERROR_ACTIVE, ERROR_WARNING
>>>>>>   		 * to  : ERROR_PASSIVE, BUS_OFF
>>>>>> @@ -951,7 +952,8 @@ static void at91_irq_err_state(struct net_device *dev,
>>>>>>   		netdev_dbg(dev, "Error Active\n");
>>>>>>   		cf->can_id |= CAN_ERR_PROT;
>>>>>>   		cf->data[2] = CAN_ERR_PROT_ACTIVE;
>>>>>> -	case CAN_STATE_ERROR_WARNING:	/* fallthrough */
>>>
>>> Seriously, for that one, you should fix the compiler. The fall through
>>
>> I'll pass your feedback on to the GCC guys.
>>
>>> is not implicit, it is actually quite explicit and the warning is simply
>>> wrong.
>>>
>>> Also, the gcc documentation says that -Wimplicit-fallthrough=3
>>> recognizes /* fallthrough */ as a proper fall through comment (and I
>>> tested with gcc 8.2).
>>>
>>
>> Yeah. But that's not the relevant change in this case.  Notice that the
>> comment was moved to the very bottom of the previous case.
>>
> 
> Yes and it doesn't matter for gcc, I tested with gcc 8.2.
> 

Yeah. But, again, you are missing the relevant part of the patch.

--
Gustavo

^ 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