Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH net-next 5/5] net: dsa: b53: Add SerDes support
From: Florian Fainelli @ 2018-09-05  0:02 UTC (permalink / raw)
  To: Andrew Lunn; +Cc: netdev, vivien.didelot, davem
In-Reply-To: <20180904231543.GI29121@lunn.ch>

On 09/04/2018 04:15 PM, Andrew Lunn wrote:
> On Tue, Sep 04, 2018 at 03:11:20PM -0700, Florian Fainelli wrote:
>> Add support for the Northstar Plus SerDes which is accessed through a
>> special page of the switch. Since this is something that most people
>> probably will not want to use, make it a configurable option.
>>
>> The SerDes supports both SGMII and 1000baseX modes, and is internally
>> looking like a seemingly standard MII PHY, except for the few bits that
>> got repurposed.
> 
> Hi Florian
> 
> The SERDES in the 6352 also look very similar to a standard MII PHYs.
> 
> Maybe at some point, we should look at the SERDES drivers we have
> embedded in different MAC drivers, and see if we can pull them out,
> maybe put them in drivers/net/phy. Any SERDES driver being used in
> combination with phylink probably has the same API.

Yes, that would sound like a good move forward. The SerDes on the
Northstar Plus does have a bunch of MII standard registers, but not a
whole lot (BMSR, BMCR, MII_PHYSID1/2, AUTONEGADV, AUTONEGLPABIL) and
then, it's all custom.

It would be good to have possibly a third vendor (Mediatek? Qualcomm?)
and see how they did it so we can define an appropriate API.
-- 
Florian

^ permalink raw reply

* Re: [PATCH net-next 5/5] net: dsa: b53: Add SerDes support
From: Andrew Lunn @ 2018-09-05  0:06 UTC (permalink / raw)
  To: Florian Fainelli; +Cc: netdev, vivien.didelot, davem
In-Reply-To: <57632a74-e3dd-893f-2573-d0450d6b290a@gmail.com>

On Tue, Sep 04, 2018 at 04:55:26PM -0700, Florian Fainelli wrote:
> On 09/04/2018 04:32 PM, Andrew Lunn wrote:
> > 
> > 
> >> +void b53_serdes_phylink_validate(struct b53_device *dev, int port,
> >> +				 unsigned long *supported,
> >> +				 struct phylink_link_state *state)
> >> +{
> >> +	u8 lane = b53_serdes_map_lane(dev, port);
> >> +
> >> +	if (lane == B53_INVALID_LANE)
> >> +		return;
> >> +
> >> +	switch (lane) {
> >> +	case 0:
> >> +		phylink_set(supported, 2500baseX_Full);
> > 
> > Hi Florian
> > 
> > Could you also use it for 2500BaseT_Full with an appropriate copper
> > PHY?
> 
> My reading of the datasheet (which only mentions 2.5G with no further
> mention) make me think that is not possible to do copper at 2.5G and
> only 2500baseX since it only talks about fiber and not copper.
> 
> Would you recommend a specific SFP that allows that? Like this one:
> 
> https://www.flexoptix.net/en/sfp-t-transceiver-2h-gigabit-cat-5e-rj-45-100m-100m-1000m-2500-base-t.html?co8829=85744

I was actually thinking of a 'plain old' copper PHY with a SERDES
interface which can do 25000Base-T. The Marvell 88x3310 or the
Aquantia 10G PHY, for example.

Russell might be able to make a recommendation. I don't have any
Copper SFP modules.

       Andrew

^ permalink raw reply

* Re: Why not use all the syn queues? in the function "tcp_conn_request", I have some questions.
From: Ttttabcd @ 2018-09-05  0:20 UTC (permalink / raw)
  To: Neal Cardwell; +Cc: Netdev
In-Reply-To: <CADVnQy=neWRer8AZBimf2c+iFzn=1iRoBwcsAYgjj=SJXQj7jg@mail.gmail.com>




Sent with ProtonMail Secure Email.

‐‐‐‐‐‐‐ Original Message ‐‐‐‐‐‐‐
On 4 September 2018 9:06 PM, Neal Cardwell <ncardwell@google.com> wrote:

> On Tue, Sep 4, 2018 at 1:48 AM Ttttabcd ttttabcd@protonmail.com wrote:
>
> > Hello everyone,recently I am looking at the source code for handling TCP three-way handshake(Linux Kernel version 4.18.5).
> > I found some strange places in the source code for handling syn messages.
> > in the function "tcp_conn_request"
> > This code will be executed when we don't enable the syn cookies.
> >
> >                 if (!net->ipv4.sysctl_tcp_syncookies &&
> >                     (net->ipv4.sysctl_max_syn_backlog - inet_csk_reqsk_queue_len(sk) <
> >                      (net->ipv4.sysctl_max_syn_backlog >> 2)) &&
> >                     !tcp_peer_is_proven(req, dst)) {
> >                         /* Without syncookies last quarter of
> >                          * backlog is filled with destinations,
> >                          * proven to be alive.
> >                          * It means that we continue to communicate
> >                          * to destinations, already remembered
> >                          * to the moment of synflood.
> >                          */
> >                         pr_drop_req(req, ntohs(tcp_hdr(skb)->source),
> >                                     rsk_ops->family);
> >                         goto drop_and_release;
> >                 }
> >
> >
> > But why don't we use all the syn queues?
>
> If tcp_peer_is_proven() returns true then we do allow ourselves to use
> the whole queue.
>
> > Why do we need to leave the size of (net->ipv4.sysctl_max_syn_backlog >> 2) in the queue?
> > Even if the system is attacked by a syn flood, there is no need to leave a part. Why do we need to leave a part?
>
> The comment describes the rationale. If syncookies are disabled, then
> the last quarter of the backlog is reserved for filling with
> destinations that were proven to be alive, according to
> tcp_peer_is_proven() (which uses RTTs measured in previous
> connections). The idea is that if there is a SYN flood, we do not want
> to use all of our queue budget on attack traffic but instead want to
> reserve some queue space for SYNs from real remote machines that we
> have actually contacted in the past.
>
> > The value of sysctl_max_syn_backlog is the maximum length of the queue only if syn cookies are enabled.
>
> Even if syncookies are disabled, sysctl_max_syn_backlog is the maximum
> length of the queue.
>
> > This is the first strange place, here is another strange place
> >
> >         __u32 isn = TCP_SKB_CB(skb)->tcp_tw_isn;
> >
> >         if ((net->ipv4.sysctl_tcp_syncookies == 2 ||
> >              inet_csk_reqsk_queue_is_full(sk)) && !isn) {
> >
> >         if (!want_cookie && !isn) {
> >
> >
> > The value of "isn" comes from TCP_SKB_CB(skb)->tcp_tw_isn, then it is judged twice whether its value is indeed 0.
> > But "tcp_tw_isn" is initialized in the function "tcp_v4_fill_cb"
> >
> >         TCP_SKB_CB(skb)->tcp_tw_isn = 0;
> >
> >
> > So it has always been 0, I used printk to test, and the result is always 0.
>
> That field is also set in tcp_timewait_state_process():
>
> TCP_SKB_CB(skb)->tcp_tw_isn = isn;
>
> So there can be cases where it is not 0.
>
> Hope that helps,
> neal

Thank you very much, I understand

^ permalink raw reply

* [RFC/PATCH] net: nixge: Add PHYLINK support
From: Moritz Fischer @ 2018-09-05  0:15 UTC (permalink / raw)
  To: netdev
  Cc: davem, f.fainelli, andrew, alex.williams, moritz.fischer,
	linux-kernel, Moritz Fischer

Add basic PHYLINK support to driver.

Suggested-by: Andrew Lunn <andrew@lunn.ch>
Signed-off-by: Moritz Fischer <mdf@kernel.org>
---

Hi all,

as Andrew suggested in order to enable SFP as
well as fixed-link support add PHYLINK support.

A couple of questions are still open (hence the RFC):

1) It seems odd to implement PHYLINK callbacks that
   are all empty? If so, should we have generic empty
   ones in drivers/net/phy/phylink.c like we have for
   genphys?

2) If this is ok, then I'll go ahead rework this with
   a DT binding update to deprecate the non-'mdio'-subnode
   case (since there are no in-tree users we might just
   change the binding)?

3) I'm again not sure about the 'select PHYLINK', wouldn't
   wanna break the build again...

Thanks again for your time!

Moritz

---
 drivers/net/ethernet/ni/Kconfig |   1 +
 drivers/net/ethernet/ni/nixge.c | 115 +++++++++++++++++++++++---------
 2 files changed, 83 insertions(+), 33 deletions(-)

diff --git a/drivers/net/ethernet/ni/Kconfig b/drivers/net/ethernet/ni/Kconfig
index c73978474c4b..80cd72948551 100644
--- a/drivers/net/ethernet/ni/Kconfig
+++ b/drivers/net/ethernet/ni/Kconfig
@@ -21,6 +21,7 @@ config NI_XGE_MANAGEMENT_ENET
 	depends on HAS_IOMEM && HAS_DMA
 	select PHYLIB
 	select OF_MDIO if OF
+	select PHYLINK
 	help
 	  Simple LAN device for debug or management purposes. Can
 	  support either 10G or 1G PHYs via SFP+ ports.
diff --git a/drivers/net/ethernet/ni/nixge.c b/drivers/net/ethernet/ni/nixge.c
index 74cf52e3fb09..a0e790d07b1c 100644
--- a/drivers/net/ethernet/ni/nixge.c
+++ b/drivers/net/ethernet/ni/nixge.c
@@ -11,6 +11,7 @@
 #include <linux/of_mdio.h>
 #include <linux/of_net.h>
 #include <linux/of_platform.h>
+#include <linux/phylink.h>
 #include <linux/of_irq.h>
 #include <linux/skbuff.h>
 #include <linux/phy.h>
@@ -165,7 +166,7 @@ struct nixge_priv {
 	struct device *dev;
 
 	/* Connection to PHY device */
-	struct device_node *phy_node;
+	struct phylink *phylink;
 	phy_interface_t		phy_mode;
 
 	int link;
@@ -416,20 +417,6 @@ static void nixge_device_reset(struct net_device *ndev)
 	netif_trans_update(ndev);
 }
 
-static void nixge_handle_link_change(struct net_device *ndev)
-{
-	struct nixge_priv *priv = netdev_priv(ndev);
-	struct phy_device *phydev = ndev->phydev;
-
-	if (phydev->link != priv->link || phydev->speed != priv->speed ||
-	    phydev->duplex != priv->duplex) {
-		priv->link = phydev->link;
-		priv->speed = phydev->speed;
-		priv->duplex = phydev->duplex;
-		phy_print_status(phydev);
-	}
-}
-
 static void nixge_tx_skb_unmap(struct nixge_priv *priv,
 			       struct nixge_tx_skb *tx_skb)
 {
@@ -859,17 +846,15 @@ static void nixge_dma_err_handler(unsigned long data)
 static int nixge_open(struct net_device *ndev)
 {
 	struct nixge_priv *priv = netdev_priv(ndev);
-	struct phy_device *phy;
 	int ret;
 
 	nixge_device_reset(ndev);
 
-	phy = of_phy_connect(ndev, priv->phy_node,
-			     &nixge_handle_link_change, 0, priv->phy_mode);
-	if (!phy)
-		return -ENODEV;
+	ret = phylink_of_phy_connect(priv->phylink, priv->dev->of_node, 0);
+	if (ret < 0)
+		return ret;
 
-	phy_start(phy);
+	phylink_start(priv->phylink);
 
 	/* Enable tasklets for Axi DMA error handling */
 	tasklet_init(&priv->dma_err_tasklet, nixge_dma_err_handler,
@@ -893,8 +878,7 @@ static int nixge_open(struct net_device *ndev)
 err_rx_irq:
 	free_irq(priv->tx_irq, ndev);
 err_tx_irq:
-	phy_stop(phy);
-	phy_disconnect(phy);
+	phylink_disconnect_phy(priv->phylink);
 	tasklet_kill(&priv->dma_err_tasklet);
 	netdev_err(ndev, "request_irq() failed\n");
 	return ret;
@@ -908,9 +892,9 @@ static int nixge_stop(struct net_device *ndev)
 	netif_stop_queue(ndev);
 	napi_disable(&priv->napi);
 
-	if (ndev->phydev) {
-		phy_stop(ndev->phydev);
-		phy_disconnect(ndev->phydev);
+	if (priv->phylink) {
+		phylink_stop(priv->phylink);
+		phylink_disconnect_phy(priv->phylink);
 	}
 
 	cr = nixge_dma_read_reg(priv, XAXIDMA_RX_CR_OFFSET);
@@ -1076,13 +1060,31 @@ static int nixge_ethtools_set_phys_id(struct net_device *ndev,
 	return 0;
 }
 
+static int
+nixge_ethtool_set_link_ksettings(struct net_device *ndev,
+				 const struct ethtool_link_ksettings *cmd)
+{
+	struct nixge_priv *priv = netdev_priv(ndev);
+
+	return phylink_ethtool_ksettings_set(priv->phylink, cmd);
+}
+
+static int
+nixge_ethtool_get_link_ksettings(struct net_device *ndev,
+				 struct ethtool_link_ksettings *cmd)
+{
+	struct nixge_priv *priv = netdev_priv(ndev);
+
+	return phylink_ethtool_ksettings_get(priv->phylink, cmd);
+}
+
 static const struct ethtool_ops nixge_ethtool_ops = {
 	.get_drvinfo    = nixge_ethtools_get_drvinfo,
 	.get_coalesce   = nixge_ethtools_get_coalesce,
 	.set_coalesce   = nixge_ethtools_set_coalesce,
 	.set_phys_id    = nixge_ethtools_set_phys_id,
-	.get_link_ksettings     = phy_ethtool_get_link_ksettings,
-	.set_link_ksettings     = phy_ethtool_set_link_ksettings,
+	.get_link_ksettings     = nixge_ethtool_get_link_ksettings,
+	.set_link_ksettings     = nixge_ethtool_set_link_ksettings,
 	.get_link		= ethtool_op_get_link,
 };
 
@@ -1225,11 +1227,52 @@ static void *nixge_get_nvmem_address(struct device *dev)
 	return mac;
 }
 
+static void nixge_validate(struct net_device *ndev, unsigned long *supported,
+			   struct phylink_link_state *state)
+{
+}
+
+static int nixge_mac_link_state(struct net_device *ndev,
+				struct phylink_link_state *state)
+{
+	return 0;
+}
+
+static void nixge_mac_config(struct net_device *ndev, unsigned int mode,
+			     const struct phylink_link_state *state)
+{
+}
+
+static void nixge_mac_an_restart(struct net_device *ndev)
+{
+}
+
+static void nixge_mac_link_down(struct net_device *ndev, unsigned int mode,
+				phy_interface_t interface)
+{
+}
+
+static void nixge_mac_link_up(struct net_device *ndev, unsigned int mode,
+			      phy_interface_t interface,
+			      struct phy_device *phy)
+{
+}
+
+static const struct phylink_mac_ops nixge_phylink_ops = {
+	.validate = nixge_validate,
+	.mac_link_state = nixge_mac_link_state,
+	.mac_an_restart = nixge_mac_an_restart,
+	.mac_config = nixge_mac_config,
+	.mac_link_down = nixge_mac_link_down,
+	.mac_link_up = nixge_mac_link_up,
+};
+
 static int nixge_probe(struct platform_device *pdev)
 {
 	struct nixge_priv *priv;
 	struct net_device *ndev;
 	struct resource *dmares;
+	struct device_node *mn;
 	const u8 *mac_addr;
 	int err;
 
@@ -1286,7 +1329,13 @@ static int nixge_probe(struct platform_device *pdev)
 	priv->coalesce_count_rx = XAXIDMA_DFT_RX_THRESHOLD;
 	priv->coalesce_count_tx = XAXIDMA_DFT_TX_THRESHOLD;
 
-	err = nixge_mdio_setup(priv, pdev->dev.of_node);
+	mn = of_get_child_by_name(pdev->dev.of_node, "mdio");
+	if (!mn) {
+		dev_warn(&pdev->dev, "No \"mdio\" subnode found, defaulting to legacy\n");
+		mn = pdev->dev.of_node;
+	}
+
+	err = nixge_mdio_setup(priv, mn);
 	if (err) {
 		netdev_err(ndev, "error registering mdio bus");
 		goto free_netdev;
@@ -1299,10 +1348,10 @@ static int nixge_probe(struct platform_device *pdev)
 		goto unregister_mdio;
 	}
 
-	priv->phy_node = of_parse_phandle(pdev->dev.of_node, "phy-handle", 0);
-	if (!priv->phy_node) {
-		netdev_err(ndev, "not find \"phy-handle\" property\n");
-		err = -EINVAL;
+	priv->phylink = phylink_create(ndev, pdev->dev.fwnode, priv->phy_mode,
+				       &nixge_phylink_ops);
+	if (IS_ERR(priv->phylink)) {
+		err = PTR_ERR(priv->phylink);
 		goto unregister_mdio;
 	}
 
-- 
2.18.0

^ permalink raw reply related

* I wait for your prompt response.
From: Aziz Dake @ 2018-09-05  0:44 UTC (permalink / raw)


Good day,

I am Mr. Aziz Dake, from Burkina Faso  a Minister confide on me to
look for foreign partner who will assist him to invest the sum of
Thirty  Million  Dollars  ($30,000,000) in your country.

He has investment interest in mining, exotic properties for commercial
resident, development properties, hotels and any other viable
investment opportunities in your country based on your recommendation
will be highly welcomed.

Hence your co -operation is highly needed to actualize this investment project

I wait for your prompt response.

Sincerely yours

Mr Aziz Dake.

^ permalink raw reply

* Planned downtime for ozlabs.org
From: Stephen Rothwell @ 2018-09-05  0:55 UTC (permalink / raw)
  To: users
  Cc: Stephen Finucane, David Miller, richard.weinberger, rdunlap,
	andrew.donnellan, jeffrey.t.kirsher, jk, patchwork, netdev,
	miquel.raynal

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

Hi all,

There will be a (hopefully) short outage of
{bilbo,patchwork,lists,irc,mail,mx}.ozlabs.org tomorrow (Thu Sep 6) at
around 11am Canberra time (+1000) to (hopefully) fix the problem with
our router port that has been causing some of the recent connectivity
problems.

I may take the opportunity to also reboot the server for useful kernel
updates.

We also currently have no IPv6 connectivity (and no ETA for recovery)
so I have removed the IPv6 addresses from most of our public facing
services.  I will put them back when we have our connectivity back.

-- 
Cheers,
Stephen Rothwell

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply

* Re: [RFC/PATCH] net: nixge: Add PHYLINK support
From: Andrew Lunn @ 2018-09-05  1:07 UTC (permalink / raw)
  To: Moritz Fischer
  Cc: netdev, davem, f.fainelli, alex.williams, moritz.fischer,
	linux-kernel
In-Reply-To: <20180905001535.19168-1-mdf@kernel.org>

> -static void nixge_handle_link_change(struct net_device *ndev)
> -{
> -	struct nixge_priv *priv = netdev_priv(ndev);
> -	struct phy_device *phydev = ndev->phydev;
> -
> -	if (phydev->link != priv->link || phydev->speed != priv->speed ||
> -	    phydev->duplex != priv->duplex) {
> -		priv->link = phydev->link;
> -		priv->speed = phydev->speed;
> -		priv->duplex = phydev->duplex;
> -		phy_print_status(phydev);
> -	}
> -}

I think this is why you are having trouble with the phylink
callbacks. You currently don't have any configuration of the MAC. You
normally need to tell the MAC what speed it should be doing. What
duplex it should be doing, if the link is up or down, if it should do
pause, or asym pause or no pause, etc.

       Andrew

^ permalink raw reply

* Re: [PATCH] net: ethernet: i40e: fix build error
From: Wang, Dongsheng @ 2018-09-05  1:32 UTC (permalink / raw)
  To: Sergei Shtylyov, jeffrey.t.kirsher@intel.com
  Cc: jacob.e.keller@intel.com, davem@davemloft.net,
	intel-wired-lan@lists.osuosl.org, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org
In-Reply-To: <84982f95-940e-d278-f6a2-13a9f76823c9@cogentembedded.com>

On 2018/9/4 18:54, Sergei Shtylyov wrote:
> On 9/4/2018 12:48 PM, Wang Dongsheng wrote:
>
>> Remove "inline" from __i40e_add_stat_strings.
>>
>> In file included from
>> drivers/net/ethernet/intel/i40e/i40e_ethtool.c:9:0:
>> drivers/net/ethernet/intel/i40e/i40e_ethtool.c: In function
>> ‘__i40e_add_stat_strings’:
>> drivers/net/ethernet/intel/i40e/i40e_ethtool_stats.h:193:20: error:
>> function ‘__i40e_add_stat_strings’ can never be inlined because it uses
>> variable argument lists
>>   static inline void __i40e_add_stat_strings(u8 **p, const struct
>> 					    i40e_stats stats[],
>>
>> Signed-off-by: Wang Dongsheng <dongsheng.wang@hxt-semitech.com>
>> ---
>>   drivers/net/ethernet/intel/i40e/i40e_ethtool_stats.h | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/ethernet/intel/i40e/i40e_ethtool_stats.h b/drivers/net/ethernet/intel/i40e/i40e_ethtool_stats.h
>> index bba1cb0..0290ade 100644
>> --- a/drivers/net/ethernet/intel/i40e/i40e_ethtool_stats.h
>> +++ b/drivers/net/ethernet/intel/i40e/i40e_ethtool_stats.h
>> @@ -190,7 +190,7 @@ struct i40e_stats {
>>    * Format and copy the strings described by stats into the buffer pointed at
>>    * by p.
>>    **/
>> -static inline void __i40e_add_stat_strings(u8 **p, const struct i40e_stats stats[],
>> +static void __i40e_add_stat_strings(u8 **p, const struct i40e_stats stats[],
>     You can't have non-inline function in a header file. Please move it, 
> leaving only prototype here.
:(. Oops, a stupid mistake...fix an error and bring in another
error...Thanks.

Cheers,
-Dongsheng
>>   				    const unsigned int size, ...)
>>   {
>>   	unsigned int i;
> MBR, Sergei
>

^ permalink raw reply

* Re: [Patch net v3] tipc: call start and done ops directly in __tipc_nl_compat_dumpit()
From: Ying Xue @ 2018-09-05  6:04 UTC (permalink / raw)
  To: Cong Wang; +Cc: Linux Kernel Network Developers, tipc-discussion
In-Reply-To: <CAM_iQpXvwy1vR9extqOYk2J6zk=xajH+5L8mabwgd1N_JkfPAQ@mail.gmail.com>

On 09/05/2018 01:49 PM, Cong Wang wrote:
> On Tue, Sep 4, 2018 at 8:25 PM Ying Xue <ying.xue@windriver.com> wrote:
>>
>> On 09/05/2018 05:54 AM, Cong Wang wrote:
>>> __tipc_nl_compat_dumpit() uses a netlink_callback on stack,
>>> so the only way to align it with other ->dumpit() call path
>>> is calling tipc_dump_start() and tipc_dump_done() directly
>>> inside it. Otherwise ->dumpit() would always get NULL from
>>> cb->args[].
>>>
>>> But tipc_dump_start() uses sock_net(cb->skb->sk) to retrieve
>>> net pointer, the cb->skb here doesn't set skb->sk, the net pointer
>>> is saved in msg->net instead, so introduce a helper function
>>> __tipc_dump_start() to pass in msg->net.
>>>
>>> Ying pointed out cb->args[0...3] are already used by other
>>> callbacks on this call path, so we can't use cb->args[0] any
>>> more, use cb->args[4] instead.
>>
>> It's a common mechanism to save rhashtable iterator pointer in cb->args
>> after tipc_dump_start() and tipc_dump_done() are introduced. Someday
>> probably we will involve new dumpit function. In order to lower the risk
>> that rhashtable iterator pointer saved is overwritten, it's better to
>> use the last slot, ie, cb->args[5].
> 
> I don't understand, currently only cb->args[0..3] are used at most,
> therefore cb->args[4] is pretty safe in current code base, there is
> no reason to be so defensive to use cb->args[5].
> 

Yes, at present cb->args[4] is safe.

> If you really worry about future, you probably want to extend cb->args
> from 6 to whatever larger, rather than just skipping cb->args[4].
> 

When we have to use the fifth slot of cb->args[] in the future, we need
to skip sb->args[4], which is a bit wried. This is the reason why I
suggested we could use the last one.

> I don't see any reason to do so.

As I said, the current version is safe. If you think it's unnecessary to
change, it's okay to me.

Acked-by: Ying Xue <ying.xue@windriver.com>

> 

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot

^ permalink raw reply

* Re: [PATCH bpf-next v2] libbpf: Remove the duplicate checking of function storage
From: Alexei Starovoitov @ 2018-09-05  6:11 UTC (permalink / raw)
  To: Jakub Kicinski
  Cc: Taeung Song, Daniel Borkmann, Alexei Starovoitov, netdev,
	linux-kernel
In-Reply-To: <20180904110745.3ab4dc3e@cakuba>

On Tue, Sep 04, 2018 at 11:07:45AM +0200, Jakub Kicinski wrote:
> On Mon,  3 Sep 2018 08:30:07 +0900, Taeung Song wrote:
> > After the commit eac7d84519a3 ("tools: libbpf: don't return '.text'
> > as a program for multi-function programs"), bpf_program__next()
> > in bpf_object__for_each_program skips the function storage such as .text,
> > so eliminate the duplicate checking.
> > 
> > Cc: Jakub Kicinski <jakub.kicinski@netronome.com>
> > Signed-off-by: Taeung Song <treeze.taeung@gmail.com>
> 
> Acked-by: Jakub Kicinski <jakub.kicinski@netronome.com>

Applied, Thanks

^ permalink raw reply

* Re: [PATCH bpf-next] bpf/verifier: properly clear union members after a ctx read
From: Alexei Starovoitov @ 2018-09-05  2:23 UTC (permalink / raw)
  To: Edward Cree; +Cc: daniel, ast, netdev
In-Reply-To: <0b724ba4-9ddf-e635-0bd5-201658355cf5@solarflare.com>

On Tue, Sep 04, 2018 at 03:19:52PM +0100, Edward Cree wrote:
> In check_mem_access(), for the PTR_TO_CTX case, after check_ctx_access()
>  has supplied a reg_type, the other members of the register state are set
>  appropriately.  Previously reg.range was set to 0, but as it is in a
>  union with reg.map_ptr, which is larger, upper bytes of the latter were
>  left in place.  This then caused the memcmp() in regsafe() to fail,
>  preventing some branches from being pruned (and occasionally causing the
>  same program to take a varying number of processed insns on repeated
>  verifier runs).
> 
> Signed-off-by: Edward Cree <ecree@solarflare.com>
> ---
> Possibly something might need adding to __mark_reg_unknown() as well to
>  clear map_ptr/range, I'm not sure (though doing so did not affect the
>  processed insn count on the cilium programs).
> 
>  kernel/bpf/verifier.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index f4ff0c569e54..49e4ea66fdd3 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -1640,9 +1640,9 @@ static int check_mem_access(struct bpf_verifier_env *env, int insn_idx, u32 regn
>  			else
>  				mark_reg_known_zero(env, regs,
>  						    value_regno);
> -			regs[value_regno].id = 0;
> -			regs[value_regno].off = 0;
> -			regs[value_regno].range = 0;
> +			/* Clear id, off, and union(map_ptr, range) */
> +			memset(regs + value_regno, 0,
> +			       offsetof(struct bpf_reg_state, var_off));
>  			regs[value_regno].type = reg_type;
>  		}

Awesome! Thanks a bunch for tracking it down.
I vaguely remember thinking about overlapping map_ptr with other fields
and not clearing map_ptr explicitly, because it was unnecessary...
Doing a bit of git-archaeology...
Looks like commit f1174f77b50c ("bpf/verifier: rework value tracking")
removed 'imm' from that union, so that __mark_reg_unknown_value()
that was clearing both 'imm' and 'map_ptr' before was no longer happening.
So old sequence:
  mark_reg_unknown_value_and_range(); // which called __mark_reg_unknown_value()
                                      //  which cleared 'imm' (and id|off|range)
  state->regs[value_regno].type = reg_type;
got replaced with
  mark_reg_known_zero();
  state->regs[value_regno].id = 0;
  state->regs[value_regno].off = 0;
  state->regs[value_regno].range = 0;
  state->regs[value_regno].type = reg_type;
which made map_ptr contain junk in upper bits.
I bet the comment "note that reg.[id|off|range] == 0" few lines before
that was deleted by that commit probably caused that bug :)
That comment I added as part of commit 969bf05eb3ce ("bpf: direct packet access")
What I was trying to express in that comment that
"mark_reg_unknown_value() that is called right before that comment
also clears id|off|range that are included as part of bigger 'imm' field
that mark_reg_unknown_value() clears, so these three fields don't need
to be cleared separately"
Sorry for confusion that that comment caused and painful debugging.

So would you agree it's fair to add
Fixes: f1174f77b50c ("bpf/verifier: rework value tracking")
?

Also I think it's better to do this memset() in both
__mark_reg_unknown() and in __mark_reg_known()
instead of open coding it here in check_mem_access().

While at it we would also need to adjust this:
static void __mark_reg_const_zero(struct bpf_reg_state *reg)
{
        __mark_reg_known(reg, 0);
        reg->off = 0;
        reg->type = SCALAR_VALUE;
}
since line reg->off = 0; wouldn't make sense after memset() is added
and few other places.

btw the 4 byte hole:
	enum bpf_reg_type          type;                 /*     0     4 */
	/* XXX 4 bytes hole, try to pack */
	union {
		u16                range;                /*           2 */
		struct bpf_map *   map_ptr;              /*           8 */
	};                                               /*     8     8 */
doesn't cause instability issues, since we kzalloc verifier reg state.

How about patch like the following:
------------
>From 422fd975ed78645ab67d2eb50ff6e1ff6fb3de32 Mon Sep 17 00:00:00 2001
From: Alexei Starovoitov <ast@kernel.org>
Date: Tue, 4 Sep 2018 19:13:44 -0700
Subject: [PATCH] bpf/verifier: fix verifier instability

Fixes: f1174f77b50c ("bpf/verifier: rework value tracking")
Debugged-by: Edward Cree <ecree@solarflare.com> 
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
---
 kernel/bpf/verifier.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index f4ff0c569e54..6ff1bac1795d 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -570,7 +570,9 @@ static void __mark_reg_not_init(struct bpf_reg_state *reg);
  */
 static void __mark_reg_known(struct bpf_reg_state *reg, u64 imm)
 {
-	reg->id = 0;
+	/* Clear id, off, and union(map_ptr, range) */
+	memset(((u8 *)reg) + sizeof(reg->type), 0,
+	       offsetof(struct bpf_reg_state, var_off) - sizeof(reg->type));
 	reg->var_off = tnum_const(imm);
 	reg->smin_value = (s64)imm;
 	reg->smax_value = (s64)imm;
@@ -589,7 +591,6 @@ static void __mark_reg_known_zero(struct bpf_reg_state *reg)
 static void __mark_reg_const_zero(struct bpf_reg_state *reg)
 {
 	__mark_reg_known(reg, 0);
-	reg->off = 0;
 	reg->type = SCALAR_VALUE;
 }
 
@@ -700,9 +701,12 @@ static void __mark_reg_unbounded(struct bpf_reg_state *reg)
 /* Mark a register as having a completely unknown (scalar) value. */
 static void __mark_reg_unknown(struct bpf_reg_state *reg)
 {
+	/*
+	 * Clear type, id, off, and union(map_ptr, range) and
+	 * padding between 'type' and union
+	 */
+	memset(reg, 0, offsetof(struct bpf_reg_state, var_off));
 	reg->type = SCALAR_VALUE;
-	reg->id = 0;
-	reg->off = 0;
 	reg->var_off = tnum_unknown;
 	reg->frameno = 0;
 	__mark_reg_unbounded(reg);
@@ -1640,9 +1644,6 @@ static int check_mem_access(struct bpf_verifier_env *env, int insn_idx, u32 regn
 			else
 				mark_reg_known_zero(env, regs,
 						    value_regno);
-			regs[value_regno].id = 0;
-			regs[value_regno].off = 0;
-			regs[value_regno].range = 0;
 			regs[value_regno].type = reg_type;
 		}
 
@@ -2495,7 +2496,6 @@ static int check_helper_call(struct bpf_verifier_env *env, int func_id, int insn
 			regs[BPF_REG_0].type = PTR_TO_MAP_VALUE_OR_NULL;
 		/* There is no offset yet applied, variable or fixed */
 		mark_reg_known_zero(env, regs, BPF_REG_0);
-		regs[BPF_REG_0].off = 0;
 		/* remember map_ptr, so that check_map_access()
 		 * can check 'value_size' boundary of memory access
 		 * to map element returned from bpf_map_lookup_elem()
-- 
2.17.1

^ permalink raw reply related

* [PATCH v2 2/2] net: ethernet: i40evf: fix potential build error
From: Wang Dongsheng @ 2018-09-05  2:27 UTC (permalink / raw)
  To: jeffrey.t.kirsher, sergei.shtylyov
  Cc: jacob.e.keller, davem, intel-wired-lan, netdev, linux-kernel,
	Wang Dongsheng
In-Reply-To: <1536114430-21356-1-git-send-email-dongsheng.wang@hxt-semitech.com>

Can't have non-inline function in a header file.
There is a risk of "Multiple definition" from cross-including.

Signed-off-by: Wang Dongsheng <dongsheng.wang@hxt-semitech.com>
---
 .../intel/i40evf/i40e_ethtool_stats.h         | 25 ++-----------------
 .../ethernet/intel/i40evf/i40evf_ethtool.c    | 24 ++++++++++++++++++
 2 files changed, 26 insertions(+), 23 deletions(-)

diff --git a/drivers/net/ethernet/intel/i40evf/i40e_ethtool_stats.h b/drivers/net/ethernet/intel/i40evf/i40e_ethtool_stats.h
index 60b595dd8c39..d70a071f065f 100644
--- a/drivers/net/ethernet/intel/i40evf/i40e_ethtool_stats.h
+++ b/drivers/net/ethernet/intel/i40evf/i40e_ethtool_stats.h
@@ -181,29 +181,8 @@ i40evf_add_queue_stats(u64 **data, struct i40e_ring *ring)
 	*data += size;
 }
 
-/**
- * __i40e_add_stat_strings - copy stat strings into ethtool buffer
- * @p: ethtool supplied buffer
- * @stats: stat definitions array
- * @size: size of the stats array
- *
- * Format and copy the strings described by stats into the buffer pointed at
- * by p.
- **/
-static void __i40e_add_stat_strings(u8 **p, const struct i40e_stats stats[],
-				    const unsigned int size, ...)
-{
-	unsigned int i;
-
-	for (i = 0; i < size; i++) {
-		va_list args;
-
-		va_start(args, size);
-		vsnprintf(*p, ETH_GSTRING_LEN, stats[i].stat_string, args);
-		*p += ETH_GSTRING_LEN;
-		va_end(args);
-	}
-}
+void __i40e_add_stat_strings(u8 **p, const struct i40e_stats stats[],
+			     const unsigned int size, ...);
 
 /**
  * 40e_add_stat_strings - copy stat strings into ethtool buffer
diff --git a/drivers/net/ethernet/intel/i40evf/i40evf_ethtool.c b/drivers/net/ethernet/intel/i40evf/i40evf_ethtool.c
index 9319971c5c92..c9a54f6de61e 100644
--- a/drivers/net/ethernet/intel/i40evf/i40evf_ethtool.c
+++ b/drivers/net/ethernet/intel/i40evf/i40evf_ethtool.c
@@ -171,6 +171,30 @@ static void i40evf_get_priv_flag_strings(struct net_device *netdev, u8 *data)
 	}
 }
 
+/**
+ * __i40e_add_stat_strings - copy stat strings into ethtool buffer
+ * @p: ethtool supplied buffer
+ * @stats: stat definitions array
+ * @size: size of the stats array
+ *
+ * Format and copy the strings described by stats into the buffer pointed at
+ * by p.
+ **/
+void __i40e_add_stat_strings(u8 **p, const struct i40e_stats stats[],
+			     const unsigned int size, ...)
+{
+	unsigned int i;
+
+	for (i = 0; i < size; i++) {
+		va_list args;
+
+		va_start(args, size);
+		vsnprintf(*p, ETH_GSTRING_LEN, stats[i].stat_string, args);
+		*p += ETH_GSTRING_LEN;
+		va_end(args);
+	}
+}
+
 /**
  * i40evf_get_stat_strings - Get stat strings
  * @netdev: network interface device structure
-- 
2.18.0

^ permalink raw reply related

* [PATCH v2 1/2] net: ethernet: i40e: fix build error
From: Wang Dongsheng @ 2018-09-05  2:27 UTC (permalink / raw)
  To: jeffrey.t.kirsher, sergei.shtylyov
  Cc: jacob.e.keller, davem, intel-wired-lan, netdev, linux-kernel,
	Wang Dongsheng

Remove "inline" from __i40e_add_stat_strings.

In file included from
drivers/net/ethernet/intel/i40e/i40e_ethtool.c:9:0:
drivers/net/ethernet/intel/i40e/i40e_ethtool.c: In function
‘__i40e_add_stat_strings’:
drivers/net/ethernet/intel/i40e/i40e_ethtool_stats.h:193:20: error:
function ‘__i40e_add_stat_strings’ can never be inlined because it uses
variable argument lists
 static inline void __i40e_add_stat_strings(u8 **p, const struct
					    i40e_stats stats[],

Signed-off-by: Wang Dongsheng <dongsheng.wang@hxt-semitech.com>
---
v2:
1. Move function.
2. Include a new patch at [2/2].

---
 .../net/ethernet/intel/i40e/i40e_ethtool.c    | 24 ++++++++++++++++++
 .../ethernet/intel/i40e/i40e_ethtool_stats.h  | 25 ++-----------------
 2 files changed, 26 insertions(+), 23 deletions(-)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_ethtool.c b/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
index d7d3974beca2..f4a70d67a80a 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
@@ -1821,6 +1821,30 @@ static void i40e_get_ethtool_stats(struct net_device *netdev,
 		  "ethtool stats count mismatch!");
 }
 
+/**
+ * __i40e_add_stat_strings - copy stat strings into ethtool buffer
+ * @p: ethtool supplied buffer
+ * @stats: stat definitions array
+ * @size: size of the stats array
+ *
+ * Format and copy the strings described by stats into the buffer pointed at
+ * by p.
+ **/
+void __i40e_add_stat_strings(u8 **p, const struct i40e_stats stats[],
+			     const unsigned int size, ...)
+{
+	unsigned int i;
+
+	for (i = 0; i < size; i++) {
+		va_list args;
+
+		va_start(args, size);
+		vsnprintf(*p, ETH_GSTRING_LEN, stats[i].stat_string, args);
+		*p += ETH_GSTRING_LEN;
+		va_end(args);
+	}
+}
+
 /**
  * i40e_get_stat_strings - copy stat strings into supplied buffer
  * @netdev: the netdev to collect strings for
diff --git a/drivers/net/ethernet/intel/i40e/i40e_ethtool_stats.h b/drivers/net/ethernet/intel/i40e/i40e_ethtool_stats.h
index bba1cb0b658f..0874c352136a 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_ethtool_stats.h
+++ b/drivers/net/ethernet/intel/i40e/i40e_ethtool_stats.h
@@ -181,29 +181,8 @@ i40e_add_queue_stats(u64 **data, struct i40e_ring *ring)
 	*data += size;
 }
 
-/**
- * __i40e_add_stat_strings - copy stat strings into ethtool buffer
- * @p: ethtool supplied buffer
- * @stats: stat definitions array
- * @size: size of the stats array
- *
- * Format and copy the strings described by stats into the buffer pointed at
- * by p.
- **/
-static inline void __i40e_add_stat_strings(u8 **p, const struct i40e_stats stats[],
-				    const unsigned int size, ...)
-{
-	unsigned int i;
-
-	for (i = 0; i < size; i++) {
-		va_list args;
-
-		va_start(args, size);
-		vsnprintf(*p, ETH_GSTRING_LEN, stats[i].stat_string, args);
-		*p += ETH_GSTRING_LEN;
-		va_end(args);
-	}
-}
+void __i40e_add_stat_strings(u8 **p, const struct i40e_stats stats[],
+			     const unsigned int size, ...);
 
 /**
  * 40e_add_stat_strings - copy stat strings into ethtool buffer
-- 
2.18.0

^ permalink raw reply related

* Re: [PATCH v7 3/4] gpiolib: Pass array info to get/set array functions
From: kbuild test robot @ 2018-09-05  7:11 UTC (permalink / raw)
  To: Janusz Krzysztofik
  Cc: kbuild-all, Linus Walleij, Jonathan Corbet, Miguel Ojeda Sandonis,
	Peter Korsgaard, Peter Rosin, Ulf Hansson, Andrew Lunn,
	Florian Fainelli, David S. Miller, Dominik Brodowski,
	Greg Kroah-Hartman, Kishon Vijay Abraham I, Lars-Peter Clausen,
	Michael Hennerich, Jonathan Cameron, Hartmut Knaack,
	Peter Meerwald-Stadler, Jiri Slaby, Willy 
In-Reply-To: <20180902120144.6855-4-jmkrzyszt@gmail.com>

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

Hi Janusz,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on gpio/for-next]
[also build test ERROR on v4.19-rc2 next-20180905]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Janusz-Krzysztofik/gpiolib-Pass-bitmaps-not-integer-arrays-to-get-set-array/20180903-172834
base:   https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-gpio.git for-next
config: x86_64-randconfig-f2-201835 (attached as .config)
compiler: gcc-6 (Debian 6.4.0-9) 6.4.0 20171026
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All errors (new ones prefixed by >>):

   drivers/phy/motorola/phy-mapphone-mdm6600.c: In function 'phy_mdm6600_cmd':
   drivers/phy/motorola/phy-mapphone-mdm6600.c:166:12: error: passing argument 3 of 'gpiod_set_array_value_cansleep' from incompatible pointer type [-Werror=incompatible-pointer-types]
               ddata->cmd_gpios->info, values);
               ^~~~~
   In file included from drivers/phy/motorola/phy-mapphone-mdm6600.c:16:0:
   include/linux/gpio/consumer.h:417:20: note: expected 'int *' but argument is of type 'struct gpio_array *'
    static inline void gpiod_set_array_value_cansleep(unsigned int array_size,
                       ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> drivers/phy/motorola/phy-mapphone-mdm6600.c:164:2: error: too many arguments to function 'gpiod_set_array_value_cansleep'
     gpiod_set_array_value_cansleep(PHY_MDM6600_NR_CMD_LINES,
     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   In file included from drivers/phy/motorola/phy-mapphone-mdm6600.c:16:0:
   include/linux/gpio/consumer.h:417:20: note: declared here
    static inline void gpiod_set_array_value_cansleep(unsigned int array_size,
                       ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/phy/motorola/phy-mapphone-mdm6600.c: In function 'phy_mdm6600_status':
   drivers/phy/motorola/phy-mapphone-mdm6600.c:185:13: error: passing argument 3 of 'gpiod_get_array_value_cansleep' from incompatible pointer type [-Werror=incompatible-pointer-types]
                ddata->status_gpios->info,
                ^~~~~
   In file included from drivers/phy/motorola/phy-mapphone-mdm6600.c:16:0:
   include/linux/gpio/consumer.h:404:19: note: expected 'int *' but argument is of type 'struct gpio_array *'
    static inline int gpiod_get_array_value_cansleep(unsigned int array_size,
                      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> drivers/phy/motorola/phy-mapphone-mdm6600.c:183:10: error: too many arguments to function 'gpiod_get_array_value_cansleep'
     error = gpiod_get_array_value_cansleep(PHY_MDM6600_NR_STATUS_LINES,
             ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   In file included from drivers/phy/motorola/phy-mapphone-mdm6600.c:16:0:
   include/linux/gpio/consumer.h:404:19: note: declared here
    static inline int gpiod_get_array_value_cansleep(unsigned int array_size,
                      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   cc1: some warnings being treated as errors

vim +/gpiod_set_array_value_cansleep +164 drivers/phy/motorola/phy-mapphone-mdm6600.c

5d1ebbda0 Tony Lindgren      2018-03-08  151  
5d1ebbda0 Tony Lindgren      2018-03-08  152  /**
5d1ebbda0 Tony Lindgren      2018-03-08  153   * phy_mdm6600_cmd() - send a command request to mdm6600
5d1ebbda0 Tony Lindgren      2018-03-08  154   * @ddata: device driver data
5d1ebbda0 Tony Lindgren      2018-03-08  155   *
5d1ebbda0 Tony Lindgren      2018-03-08  156   * Configures the three command request GPIOs to the specified value.
5d1ebbda0 Tony Lindgren      2018-03-08  157   */
5d1ebbda0 Tony Lindgren      2018-03-08  158  static void phy_mdm6600_cmd(struct phy_mdm6600 *ddata, int val)
5d1ebbda0 Tony Lindgren      2018-03-08  159  {
916010a73 Janusz Krzysztofik 2018-09-02  160  	DECLARE_BITMAP(values, PHY_MDM6600_NR_CMD_LINES);
5d1ebbda0 Tony Lindgren      2018-03-08  161  
916010a73 Janusz Krzysztofik 2018-09-02  162  	*values = val;
5d1ebbda0 Tony Lindgren      2018-03-08  163  
5d1ebbda0 Tony Lindgren      2018-03-08 @164  	gpiod_set_array_value_cansleep(PHY_MDM6600_NR_CMD_LINES,
dea6937cb Janusz Krzysztofik 2018-09-02  165  				       ddata->cmd_gpios->desc,
dea6937cb Janusz Krzysztofik 2018-09-02 @166  				       ddata->cmd_gpios->info, values);
5d1ebbda0 Tony Lindgren      2018-03-08  167  }
5d1ebbda0 Tony Lindgren      2018-03-08  168  
5d1ebbda0 Tony Lindgren      2018-03-08  169  /**
5d1ebbda0 Tony Lindgren      2018-03-08  170   * phy_mdm6600_status() - read mdm6600 status lines
5d1ebbda0 Tony Lindgren      2018-03-08  171   * @ddata: device driver data
5d1ebbda0 Tony Lindgren      2018-03-08  172   */
5d1ebbda0 Tony Lindgren      2018-03-08  173  static void phy_mdm6600_status(struct work_struct *work)
5d1ebbda0 Tony Lindgren      2018-03-08  174  {
5d1ebbda0 Tony Lindgren      2018-03-08  175  	struct phy_mdm6600 *ddata;
5d1ebbda0 Tony Lindgren      2018-03-08  176  	struct device *dev;
916010a73 Janusz Krzysztofik 2018-09-02  177  	DECLARE_BITMAP(values, PHY_MDM6600_NR_STATUS_LINES);
5d1ebbda0 Tony Lindgren      2018-03-08  178  	int error, i, val = 0;
5d1ebbda0 Tony Lindgren      2018-03-08  179  
5d1ebbda0 Tony Lindgren      2018-03-08  180  	ddata = container_of(work, struct phy_mdm6600, status_work.work);
5d1ebbda0 Tony Lindgren      2018-03-08  181  	dev = ddata->dev;
5d1ebbda0 Tony Lindgren      2018-03-08  182  
ad5003300 Tony Lindgren      2018-05-31 @183  	error = gpiod_get_array_value_cansleep(PHY_MDM6600_NR_STATUS_LINES,
5d1ebbda0 Tony Lindgren      2018-03-08  184  					       ddata->status_gpios->desc,
dea6937cb Janusz Krzysztofik 2018-09-02 @185  					       ddata->status_gpios->info,
5d1ebbda0 Tony Lindgren      2018-03-08  186  					       values);
5d1ebbda0 Tony Lindgren      2018-03-08  187  	if (error)
5d1ebbda0 Tony Lindgren      2018-03-08  188  		return;
5d1ebbda0 Tony Lindgren      2018-03-08  189  
ad5003300 Tony Lindgren      2018-05-31  190  	for (i = 0; i < PHY_MDM6600_NR_STATUS_LINES; i++) {
916010a73 Janusz Krzysztofik 2018-09-02  191  		val |= test_bit(i, values) << i;
5d1ebbda0 Tony Lindgren      2018-03-08  192  		dev_dbg(ddata->dev, "XXX %s: i: %i values[i]: %i val: %i\n",
916010a73 Janusz Krzysztofik 2018-09-02  193  			__func__, i, test_bit(i, values), val);
5d1ebbda0 Tony Lindgren      2018-03-08  194  	}
5d1ebbda0 Tony Lindgren      2018-03-08  195  	ddata->status = val;
5d1ebbda0 Tony Lindgren      2018-03-08  196  
5d1ebbda0 Tony Lindgren      2018-03-08  197  	dev_info(dev, "modem status: %i %s\n",
5d1ebbda0 Tony Lindgren      2018-03-08  198  		 ddata->status,
5d1ebbda0 Tony Lindgren      2018-03-08  199  		 phy_mdm6600_status_name[ddata->status & 7]);
5d1ebbda0 Tony Lindgren      2018-03-08  200  	complete(&ddata->ack);
5d1ebbda0 Tony Lindgren      2018-03-08  201  }
5d1ebbda0 Tony Lindgren      2018-03-08  202  

:::::: The code at line 164 was first introduced by commit
:::::: 5d1ebbda0318b1ba55eaa1fae3fd867af17b0774 phy: mapphone-mdm6600: Add USB PHY driver for MDM6600 on Droid 4

:::::: TO: Tony Lindgren <tony@atomide.com>
:::::: CC: Kishon Vijay Abraham I <kishon@ti.com>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 30605 bytes --]

^ permalink raw reply

* Re: [PATCH 1/9] PCI: sysfs: Export available PCIe bandwidth
From: Stephen Hemminger @ 2018-09-05  7:26 UTC (permalink / raw)
  To: Alexandru Gagniuc
  Cc: linux-pci, bhelgaas, keith.busch, alex_gagniuc, austin_bolen,
	shyam_iyer, Ariel Elior, everest-linux-l2, David S. Miller,
	Michael Chan, Ganesh Goudar, Jeff Kirsher, Tariq Toukan,
	Saeed Mahameed, Leon Romanovsky, Jakub Kicinski,
	Dirk van der Merwe, netdev, linux-kernel, intel-wired-lan,
	linux-rdma, oss-drivers
In-Reply-To: <20180903180242.14504-2-mr.nuke.me@gmail.com>

On Mon,  3 Sep 2018 13:02:28 -0500
Alexandru Gagniuc <mr.nuke.me@gmail.com> wrote:

> For certain bandwidth-critical devices (e.g. multi-port network cards)
> it is useful to know the available bandwidth to the root complex. This
> information is only available via the system log, which doesn't
> account for link degradation after probing.
> 
> With a sysfs attribute, we can computes the bandwidth on-demand, and
> will detect degraded links.
> 
> Signed-off-by: Alexandru Gagniuc <mr.nuke.me@gmail.com>

In other places (like /sys/class/net/eth0/speed) only the raw value is printed
without suffix.  The general convention in sysfs is that it should be one value
per file and in more raw format.  So why not just print it in bits/sec without
suffix?

^ permalink raw reply

* [PATCH] 9p: readdir: do not trust pdu content for stat item size
From: Dominique Martinet @ 2018-09-05  8:00 UTC (permalink / raw)
  To: Dominique Martinet, Eric Van Hensbergen, Latchesar Ionkov
  Cc: Gertjan Halkes, v9fs-developer, netdev, linux-kernel,
	Dominique Martinet

From: Gertjan Halkes <gertjan@google.com>

v9fs_dir_readdir() could deadloop if a struct was sent with a size set
to -2

Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=88021
Signed-off-by: Gertjan Halkes <gertjan@google.com>
Signed-off-by: Dominique Martinet <dominique.martinet@cea.fr>
---

This patch has been sitting in bugzilla for three years, I'll take it
for the next merge window after some testing.
Thanks Gertjan!

 fs/9p/vfs_dir.c   | 8 +++-----
 net/9p/protocol.c | 3 ++-
 2 files changed, 5 insertions(+), 6 deletions(-)

diff --git a/fs/9p/vfs_dir.c b/fs/9p/vfs_dir.c
index 48db9a9f13f9..cb6c4031af55 100644
--- a/fs/9p/vfs_dir.c
+++ b/fs/9p/vfs_dir.c
@@ -105,7 +105,6 @@ static int v9fs_dir_readdir(struct file *file, struct dir_context *ctx)
 	int err = 0;
 	struct p9_fid *fid;
 	int buflen;
-	int reclen = 0;
 	struct p9_rdir *rdir;
 	struct kvec kvec;
 
@@ -138,11 +137,10 @@ static int v9fs_dir_readdir(struct file *file, struct dir_context *ctx)
 		while (rdir->head < rdir->tail) {
 			err = p9stat_read(fid->clnt, rdir->buf + rdir->head,
 					  rdir->tail - rdir->head, &st);
-			if (err) {
+			if (err <= 0) {
 				p9_debug(P9_DEBUG_VFS, "returned %d\n", err);
 				return -EIO;
 			}
-			reclen = st.size+2;
 
 			over = !dir_emit(ctx, st.name, strlen(st.name),
 					 v9fs_qid2ino(&st.qid), dt_type(&st));
@@ -150,8 +148,8 @@ static int v9fs_dir_readdir(struct file *file, struct dir_context *ctx)
 			if (over)
 				return 0;
 
-			rdir->head += reclen;
-			ctx->pos += reclen;
+			rdir->head += err;
+			ctx->pos += err;
 		}
 	}
 }
diff --git a/net/9p/protocol.c b/net/9p/protocol.c
index ee32bbf12675..b4d80c533f89 100644
--- a/net/9p/protocol.c
+++ b/net/9p/protocol.c
@@ -571,9 +571,10 @@ int p9stat_read(struct p9_client *clnt, char *buf, int len, struct p9_wstat *st)
 	if (ret) {
 		p9_debug(P9_DEBUG_9P, "<<< p9stat_read failed: %d\n", ret);
 		trace_9p_protocol_dump(clnt, &fake_pdu);
+		return ret;
 	}
 
-	return ret;
+	return fake_pdu.offset;
 }
 EXPORT_SYMBOL(p9stat_read);
 
-- 
2.17.1

^ permalink raw reply related

* [PATCH iproute2] bridge/mdb: fix missing new line when show bridge mdb
From: Hangbin Liu @ 2018-09-05  3:33 UTC (permalink / raw)
  To: netdev; +Cc: Stephen Hemminger, David Ahern, Hangbin Liu

The bridge mdb show is broken on current iproute2. e.g.
]# bridge mdb show
34: br0  veth0_br  224.1.1.2  temp 34: br0  veth0_br  224.1.1.1  temp

After fix:
]# bridge mdb show
34: br0  veth0_br  224.1.1.2  temp
34: br0  veth0_br  224.1.1.1  temp

Reported-by: Ying Xu <yinxu@redhat.com>
Fixes: c7c1a1ef51aea ("bridge: colorize output and use JSON print library")
Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
---
 bridge/mdb.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/bridge/mdb.c b/bridge/mdb.c
index f38dc67..d89c065 100644
--- a/bridge/mdb.c
+++ b/bridge/mdb.c
@@ -107,6 +107,10 @@ static void br_print_router_ports(FILE *f, struct rtattr *attr,
 			fprintf(f, "%s ", port_ifname);
 		}
 	}
+
+	if (!is_json_context() && !show_stats)
+		fprintf(f, "\n");
+
 	close_json_array(PRINT_JSON, NULL);
 }
 
@@ -164,6 +168,10 @@ static void print_mdb_entry(FILE *f, int ifindex, const struct br_mdb_entry *e,
 		print_string(PRINT_ANY, "timer", " %s",
 			     format_timer(timer));
 	}
+
+	if (!is_json_context())
+		fprintf(f, "\n");
+
 	close_json_object();
 }
 
-- 
2.5.5

^ permalink raw reply related

* Re: [RFC/PATCH] net: nixge: Add PHYLINK support
From: Moritz Fischer @ 2018-09-05  4:05 UTC (permalink / raw)
  To: Florian Fainelli
  Cc: netdev, David S. Miller, Andrew Lunn, Alex Williams,
	Linux Kernel Mailing List
In-Reply-To: <fa4fca9c-5950-8ec1-5746-91e51085cecf@gmail.com>

Hi Florian,

On Tue, Sep 4, 2018 at 5:27 PM, Florian Fainelli <f.fainelli@gmail.com> wrote:
> On 09/04/2018 05:15 PM, Moritz Fischer wrote:
>> Add basic PHYLINK support to driver.
>>
>> Suggested-by: Andrew Lunn <andrew@lunn.ch>
>> Signed-off-by: Moritz Fischer <mdf@kernel.org>
>> ---
>>
>> Hi all,
>>
>> as Andrew suggested in order to enable SFP as
>> well as fixed-link support add PHYLINK support.
>>
>> A couple of questions are still open (hence the RFC):
>>
>> 1) It seems odd to implement PHYLINK callbacks that
>>    are all empty? If so, should we have generic empty
>>    ones in drivers/net/phy/phylink.c like we have for
>>    genphys?
>
> Yes it is odd, the validate callback most certainly should not be empty,
> neither should the mac_config and mac_link_{up,down}, but, with some
> luck, you can get things to just work, typically with MDIO PHYs, since a
> large amount of what they can do is discoverable.
>
> If you had an existing adjust_link callback from PHYLIB, it's really
> about breaking it down such that the MAC configuration of
> speed/duplex/pause happens in mac_config, and the link setting (if
> necessary), happens in mac_link_{up,down}, and that's pretty much it for
> MLO_AN_PHY cases.

Let me check, it seems there is a register that indicates whether the MAC can
do either 1G or 10G. I might be able to use that for some of the above, but
there is not really much in terms of writable registers there.
It's like a DMA engine with a bit of MDIO on the side. Let me see if I can make
it look less weird with that. If not I'll go with a comment explaining
that there
isn't much to do for the MLO_AN_PHY case and the MLO_FIXED cases?

Cheers and thanks for your feedback,
Moritz

^ permalink raw reply

* pull-request: mac80211-next 2018-09-05
From: Johannes Berg @ 2018-09-05  9:05 UTC (permalink / raw)
  To: David Miller
  Cc: netdev-u79uwXL29TY76Z2rM5mHXA,
	linux-wireless-u79uwXL29TY76Z2rM5mHXA

Hi Dave,

Here's a pull request for net-next. Thanks for including
net, that way I could include Stanislaw's patch to check
the regulatory WMM data.

Please pull and let me know if there's any problem.

Thanks,
johannes



The following changes since commit 36302685f59345959de96d0d70a5ad20a3a3451b:

  Merge git://git.kernel.org/pub/scm/linux/kernel/git/davem/net (2018-09-04 21:33:03 -0700)

are available in the Git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/jberg/mac80211-next.git tags/mac80211-next-for-davem-2018-09-05

for you to fetch changes up to 014f5a250fc49fa8c6cd50093e725e71f3ae52da:

  cfg80211: validate wmm rule when setting (2018-09-05 10:16:59 +0200)

----------------------------------------------------------------
This time, we have some pretty impactful work. Among
the changes:
 * changes to make PTK rekeying work better, or actually
   better/safely if drivers get updated
 * VHT extended NSS support - some APs had capabilities
   that didn't fit into the VHT (11ac) spec, so the spec
   was updated and we follow that now
 * some TXQ and A-MSDU building work - will allow iwlwifi
   to use this soon
 * more HE work, including aligning to 802.11ax Draft 3.0
 * L-SIG and 0-length-PSDU support in radiotap

----------------------------------------------------------------
Alexander Wetzel (2):
      nl80211: Add CAN_REPLACE_PTK0 API
      mac80211: Fix PTK rekey freezes and clear text leak

Gustavo A. R. Silva (1):
      mac80211: remove unnecessary NULL check

Ido Yariv (1):
      mac80211: Add he_capa debugfs entry

Johannes Berg (7):
      mac80211: remove pointless 'params' NULL checks
      mac80211: use le16_encode_bits() instead of open-coding
      mac80211: add an optional TXQ for other PS-buffered frames
      ieee80211: add new VHT capability fields/parsing
      mac80211: introduce capability flags for VHT EXT NSS support
      mac80211: add ability to parse CCFS2
      mac80211: copy VHT EXT NSS BW Support/Capable data to station

Naftali Goldstein (1):
      mac80211: fix saving a few HE values

Sara Sharon (4):
      ieee80211: remove redundant leading zeroes
      mac80211: add an option for station management TXQ
      mac80211: allow AMSDU size limitation per-TID
      mac80211: add an option for drivers to check if packets can be aggregated

Shaul Triebitz (5):
      cfg80211: add he_capabilities (ext) IE to AP settings
      mac80211: in AP mode, set bss_conf::he_supported
      mac80211: support radiotap L-SIG data
      mac80211: support reporting 0-length PSDU in radiotap
      wireless: align to draft 11ax D3.0

Stanislaw Gruszka (1):
      cfg80211: validate wmm rule when setting

Wen Gong (1):
      mac80211: Store sk_pacing_shift in ieee80211_hw

 drivers/net/wireless/intel/iwlwifi/iwl-nvm-parse.c |  51 ++-
 drivers/net/wireless/intel/iwlwifi/mvm/mac80211.c  |   4 -
 drivers/net/wireless/mac80211_hwsim.c              |  18 +-
 include/linux/ieee80211.h                          | 111 +++++--
 include/net/cfg80211.h                             |   2 +
 include/net/ieee80211_radiotap.h                   |  21 ++
 include/net/mac80211.h                             |  80 ++++-
 include/uapi/linux/nl80211.h                       |   6 +
 net/mac80211/cfg.c                                 |   9 +-
 net/mac80211/debugfs.c                             |   4 +
 net/mac80211/debugfs_sta.c                         | 364 ++++++++++++++++++++-
 net/mac80211/driver-ops.h                          |  10 +
 net/mac80211/ibss.c                                |   4 +-
 net/mac80211/ieee80211_i.h                         |   7 +-
 net/mac80211/key.c                                 | 111 +++++--
 net/mac80211/main.c                                |  65 ++++
 net/mac80211/mesh.c                                |   5 +-
 net/mac80211/mlme.c                                |  23 +-
 net/mac80211/rx.c                                  |  37 ++-
 net/mac80211/spectmgmt.c                           |   5 +-
 net/mac80211/sta_info.c                            |  21 +-
 net/mac80211/tx.c                                  |  64 +++-
 net/mac80211/util.c                                |  48 ++-
 net/mac80211/vht.c                                 |  20 ++
 net/wireless/nl80211.c                             |   3 +
 net/wireless/reg.c                                 |  64 ++--
 net/wireless/util.c                                | 109 ++++++
 27 files changed, 1088 insertions(+), 178 deletions(-)

^ permalink raw reply

* Re: [PATCH v2 00/11] mscc: ocelot: add support for SerDes muxing configuration
From: Alexandre Belloni @ 2018-09-05  9:07 UTC (permalink / raw)
  To: Paul Burton
  Cc: Quentin Schulz, David Miller, andrew, ralf, jhogan, robh+dt,
	mark.rutland, kishon, f.fainelli, allan.nielsen, linux-mips,
	devicetree, linux-kernel, netdev, thomas.petazzoni
In-Reply-To: <20180904230351.vwlq2s7joulvqw2i@pburton-laptop>

On 04/09/2018 16:03:51-0700, Paul Burton wrote:
> Well, it sounded like David is OK with this all going through the MIPS
> tree, though we'd need an ack for the PHY parts.
> 
> Alternatively I'd be happy for the DT changes to go through the net-next
> tree, which may make more sense given that the .dts changes are pretty
> trivial in comparison with the driver changes. If David wants to do that
> then for patches 1 & 8:
> 
>     Acked-by: Paul Burton <paul.burton@mips.com>
> 
> Either way there may be conflicts for ocelot.dtsi when it comes to
> merging to master, but they should be simple to resolve. It seems
> Wolfram already took your DT changes for I2C so there's probably going
> to be multiple trees updating that file this cycle already anyway.
> 

Actually, I think Wolfram meant that he took the bindings so you can
take the DT patches for i2c.

> Ideally I'd say "don't break bisection" but that's sort of a separate
> issue here since even if you restructure your series to do that it would
> still need to go through one tree. For example you could adjust
> mscc_ocelot_probe() to handle either the reg property or the syscon,
> then adjust the DT to use the syscon, then remove the code dealing with
> the reg property, and I'd consider that a good idea anyway but it would
> still probably all need to go through one tree to make sure things get
> merged in the right order & avoid breaking bisection.
> 

I don't really think bisection is important at this stage but if you
don't want to break it, then I guess it makes more sense to have the
whole series through net.


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

^ permalink raw reply

* Re: [PATCH mlx5-next v1 05/15] net/mlx5: Break encap/decap into two separated flow table creation flags
From: Leon Romanovsky @ 2018-09-05  5:10 UTC (permalink / raw)
  To: Jason Gunthorpe
  Cc: Doug Ledford, RDMA mailing list, Ariel Levkovich, Mark Bloch,
	Or Gerlitz, Saeed Mahameed, linux-netdev
In-Reply-To: <20180904220242.GA3895@ziepe.ca>

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

On Tue, Sep 04, 2018 at 04:02:42PM -0600, Jason Gunthorpe wrote:
> On Tue, Aug 28, 2018 at 02:18:44PM +0300, Leon Romanovsky wrote:
> > From: Mark Bloch <markb@mellanox.com>
> >
> > Today we are able to attach encap and decap actions only to the FDB. In
> > preparation to enable those actions on the NIC flow tables, break the
> > single flag into two. Those flags control whatever a decap or encap
> > operations can be attached to the flow table created. For FDB, if
> > encapsulation is required, we set both of them.
> >
> > Signed-off-by: Mark Bloch <markb@mellanox.com>
> > Reviewed-by: Saeed Mahameed <saeedm@mellanox.com>
> > Reviewed-by: Or Gerlitz <ogerlitz@mellanox.com>
> > Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
> >  drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads.c | 3 ++-
> >  drivers/net/ethernet/mellanox/mlx5/core/fs_cmd.c           | 7 ++++---
> >  include/linux/mlx5/fs.h                                    | 3 ++-
> >  3 files changed, 8 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads.c b/drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads.c
> > index f72b5c9dcfe9..ff21807a0c4b 100644
> > +++ b/drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads.c
> > @@ -529,7 +529,8 @@ static int esw_create_offloads_fast_fdb_table(struct mlx5_eswitch *esw)
> >  		esw_size >>= 1;
> >
> >  	if (esw->offloads.encap != DEVLINK_ESWITCH_ENCAP_MODE_NONE)
> > -		flags |= MLX5_FLOW_TABLE_TUNNEL_EN;
> > +		flags |= (MLX5_FLOW_TABLE_TUNNEL_EN_ENCAP |
> > +			  MLX5_FLOW_TABLE_TUNNEL_EN_DECAP);
> >
> >  	fdb = mlx5_create_auto_grouped_flow_table(root_ns, FDB_FAST_PATH,
> >  						  esw_size,
> > diff --git a/drivers/net/ethernet/mellanox/mlx5/core/fs_cmd.c b/drivers/net/ethernet/mellanox/mlx5/core/fs_cmd.c
> > index 9ae777e56529..1698f325a21e 100644
> > +++ b/drivers/net/ethernet/mellanox/mlx5/core/fs_cmd.c
> > @@ -152,7 +152,8 @@ static int mlx5_cmd_create_flow_table(struct mlx5_core_dev *dev,
> >  				      struct mlx5_flow_table *next_ft,
> >  				      unsigned int *table_id, u32 flags)
> >  {
> > -	int en_encap_decap = !!(flags & MLX5_FLOW_TABLE_TUNNEL_EN);
> > +	int en_encap = !!(flags & MLX5_FLOW_TABLE_TUNNEL_EN_ENCAP);
> > +	int en_decap = !!(flags & MLX5_FLOW_TABLE_TUNNEL_EN_DECAP);
>
> Yuk, please don't use !!.
>
> 	bool en_decap = flags & MLX5_FLOW_TABLE_TUNNEL_EN_DECAP;

We need to provide en_encap and en_decap as an input to MLX5_SET(...)
which is passed to FW as 0 or 1. Boolean type is declared in C as int
and treated as zero for false and any other value for true, so we
can't pass "bool en_decap" without ensuring before that it is 1.

I'm applying this patch as is.

Thanks

>
> Jason

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

^ permalink raw reply

* Re: [PATCH rdma-next v1 00/15] Flow actions to mutate packets
From: Leon Romanovsky @ 2018-09-05  5:14 UTC (permalink / raw)
  To: Jason Gunthorpe
  Cc: Doug Ledford, RDMA mailing list, Ariel Levkovich, Mark Bloch,
	Or Gerlitz, Saeed Mahameed, linux-netdev
In-Reply-To: <20180904221205.GB3895@ziepe.ca>

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

On Tue, Sep 04, 2018 at 04:12:05PM -0600, Jason Gunthorpe wrote:
> On Tue, Aug 28, 2018 at 02:18:39PM +0300, Leon Romanovsky wrote:
> > From: Leon Romanovsky <leonro@mellanox.com>
> >
> > >From Mark,
> >
> > This series exposes the ability to create flow actions which can
> > mutate packet headers. We do that by exposing two new verbs:
> >  * modify header - can change existing packet headers. packet
> >  * reformat - can encapsulate or decapsulate a packet.
> >               Once created a flow action must be attached to a steering
> >               rule for it to take effect.
> >
> > The first 10 patches refactor mlx5_core code, rename internal structures
> > to better reflect their operation and export needed functions so the
> > RDMA side can allocate the action.
> >
> > The last 5 patches expose via the IOCTL infrastructure mlx5_ib methods
> > which do the actual allocation of resources and return an handle to the
> > user. A user of this API is expected to know how to work with the
> > device's spec as the input to those function is HW depended.
> >
> > An example usage of the modify header action is routing, A user can
> > create an action which edits the L2 header and decrease the TTL.
> >
> > An example usage of the packet reformat action is VXLAN encap/decap
> > which is done by the HW.
> >
> > Changelog:
> >  v0 -> v1:
> >   * Patch 1: Addressed Saeed's comments and simplified the logic.
> >   * Patch 2: Changed due to changes in patch 1.
> >
> >  Split the 27 patch series into 3, this is the first one
> >  which just lets the user create flow action.
> >  Other than that styling fixes mainly in the RDMA patches
> >  to make sure 80 chars limit isn't exceeded.
> >
> >  RFC -> v0:
> >   * Patch 1 a new patch which refactors the logic
> >     when getting a flow namespace.
> >   * Patch 2 was split into two.
> >   * Patch 3: Fixed a typo in commit message
> >   * Patch 5: Updated commit message
> >   * Patch 7: Updated commit message
> >     Renamed:
> >       - MLX5_FLOW_CONTEXT_ACTION_PACKET_REFORMAT_ID to
> >         MLX5_FLOW_CONTEXT_ACTION_PACKET_REFORMAT
> >       - packet_reformat_id to reformat_id in struct mlx5_flow_act
> >       - packet_reformat_id to encap_id in struct mlx5_esw_flow_attr
> >       - packet_reformat_id to encap_id in struct mlx5e_encap_entry
> >       - PACKET_REFORMAT to REFORMAT when printing trace points
> >   * Patch 9: Updated commit message
> >     Updated function declaration in mlx5_core.h, could of lead
> >     to compile error on bisection.
> >   * Patch 11: Disallow egress rules insertion when in switchdev mode
> >   * Patch 12: A new patch to deal with passing enum values using
> >     the IOCTL infrastructure.
> >   * Patch 13: Use new enum value attribute when passing enum
> >     mlx5_ib_uapi_flow_table_type
> >   * Patch 15: Don't set encap flags on flow tables if in switchdev mode
> >   * Patch 17: Use new enum value attribute when passing enum
> >     mlx5_ib_uapi_flow_table_type and enum
> >     mlx5_ib_uapi_flow_action_packet_reformat_type
> >   * Patch 19: Allow creation of both
> >     MLX5_IB_UAPI_FLOW_ACTION_PACKET_REFORMAT_TYPE_L2_TO_L3_TUNNEL
> >     and MLX5_IB_UAPI_FLOW_ACTION_PACKET_REFORMAT_TYPE_L3_TUNNEL_TO_L2
> > packet
> >     reformat actions.
> >   * Patch 20: A new patch which allows attaching packet reformat
> >     actions to flow tables on NIC RX.
> >
> > Thanks
> >
> > Mark Bloch (15):
> >   net/mlx5: Cleanup flow namespace getter switch logic
> >   net/mlx5: Add proper NIC TX steering flow tables support
> >   net/mlx5: Export modify header alloc/dealloc functions
> >   net/mlx5: Add support for more namespaces when allocating modify
> >     header
> >   net/mlx5: Break encap/decap into two separated flow table creation
> >     flags
> >   net/mlx5: Move header encap type to IFC header file
> >   {net, RDMA}/mlx5: Rename encap to reformat packet
> >   net/mlx5: Expose new packet reformat capabilities
> >   net/mlx5: Pass a namespace for packet reformat ID allocation
> >   net/mlx5: Export packet reformat alloc/dealloc functions
> >   RDMA/uverbs: Add UVERBS_ATTR_CONST_IN to the specs language
> >   RDMA/mlx5: Add a new flow action verb - modify header
> >   RDMA/uverbs: Add generic function to fill in flow action object
> >   RDMA/mlx5: Add new flow action verb - packet reformat
> >   RDMA/mlx5: Extend packet reformat verbs
> >
> >  drivers/infiniband/core/uverbs_ioctl.c             |  23 ++
> >  .../infiniband/core/uverbs_std_types_flow_action.c |   7 +-
> >  drivers/infiniband/hw/mlx5/devx.c                  |   6 +-
> >  drivers/infiniband/hw/mlx5/flow.c                  | 301 +++++++++++++++++++++
> >  drivers/infiniband/hw/mlx5/main.c                  |   3 +
> >  drivers/infiniband/hw/mlx5/mlx5_ib.h               |  19 +-
> >  drivers/net/ethernet/mellanox/mlx5/core/cmd.c      |   8 +-
> >  .../mellanox/mlx5/core/diag/fs_tracepoint.h        |   2 +-
> >  drivers/net/ethernet/mellanox/mlx5/core/en_tc.c    |  51 ++--
> >  drivers/net/ethernet/mellanox/mlx5/core/eswitch.c  |   2 +-
> >  .../ethernet/mellanox/mlx5/core/eswitch_offloads.c |   9 +-
> >  drivers/net/ethernet/mellanox/mlx5/core/fs_cmd.c   |  87 +++---
> >  drivers/net/ethernet/mellanox/mlx5/core/fs_core.c  |  73 +++--
> >  .../net/ethernet/mellanox/mlx5/core/mlx5_core.h    |  12 +-
> >  include/linux/mlx5/device.h                        |   6 +
> >  include/linux/mlx5/fs.h                            |  20 +-
> >  include/linux/mlx5/mlx5_ifc.h                      |  70 +++--
> >  include/rdma/uverbs_ioctl.h                        |  40 +++
> >  include/rdma/uverbs_std_types.h                    |  12 +
> >  include/uapi/rdma/mlx5_user_ioctl_cmds.h           |  18 ++
> >  include/uapi/rdma/mlx5_user_ioctl_verbs.h          |  12 +
> >  21 files changed, 638 insertions(+), 143 deletions(-)
>
> This looks OK to me, can you make the shared commit please?

Thanks, I pushed to mlx5-next

50acec06f392 net/mlx5: Export packet reformat alloc/dealloc functions
31ca3648f01b net/mlx5: Pass a namespace for packet reformat ID allocation
bea4e1f6c6c5 net/mlx5: Expose new packet reformat capabilities
60786f0987c0 {net, RDMA}/mlx5: Rename encap to reformat packet
e0e7a3861b6c net/mlx5: Move header encap type to IFC header file
61444b458b01 net/mlx5: Break encap/decap into two separated flow table creation flags
c3c062f80665 net/mlx5: Add support for more namespaces when allocating modify header
90c1d1b8da67 net/mlx5: Export modify header alloc/dealloc functions
8ce78257965e net/mlx5: Add proper NIC TX steering flow tables support
2226dcb424bf net/mlx5: Cleanup flow namespace getter switch logic


>
> Thanks,
> Jason

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

^ permalink raw reply

* Re: [PATCH rdma-next v1 12/15] RDMA/mlx5: Add a new flow action verb - modify header
From: Leon Romanovsky @ 2018-09-05  5:20 UTC (permalink / raw)
  To: Jason Gunthorpe
  Cc: Doug Ledford, RDMA mailing list, Ariel Levkovich, Mark Bloch,
	Or Gerlitz, Saeed Mahameed, linux-netdev
In-Reply-To: <20180904215823.GA3792@ziepe.ca>

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

On Tue, Sep 04, 2018 at 03:58:23PM -0600, Jason Gunthorpe wrote:
> On Tue, Aug 28, 2018 at 02:18:51PM +0300, Leon Romanovsky wrote:
>
> > +static int UVERBS_HANDLER(MLX5_IB_METHOD_FLOW_ACTION_CREATE_MODIFY_HEADER)(
> > +	struct ib_uverbs_file *file,
> > +	struct uverbs_attr_bundle *attrs)
> > +{
> > +	struct ib_uobject *uobj = uverbs_attr_get_uobject(
> > +		attrs, MLX5_IB_ATTR_CREATE_MODIFY_HEADER_HANDLE);
> > +	struct mlx5_ib_dev *mdev = to_mdev(uobj->context->device);
> > +	enum mlx5_ib_uapi_flow_table_type ft_type;
> > +	struct ib_flow_action *action;
> > +	size_t num_actions;
> > +	void *in;
> > +	int len;
> > +	int ret;
> > +
> > +	if (!mlx5_ib_modify_header_supported(mdev))
> > +		return -EOPNOTSUPP;
> > +
> > +	in = uverbs_attr_get_alloced_ptr(attrs,
> > +		MLX5_IB_ATTR_CREATE_MODIFY_HEADER_ACTIONS_PRM);
> > +	len = uverbs_attr_get_len(attrs,
> > +		MLX5_IB_ATTR_CREATE_MODIFY_HEADER_ACTIONS_PRM);
> > +
> > +	if (len % MLX5_UN_SZ_BYTES(set_action_in_add_action_in_auto))
> > +		return -EINVAL;
> > +
> > +	ret = uverbs_get_const(&ft_type, attrs,
> > +			       MLX5_IB_ATTR_CREATE_MODIFY_HEADER_FT_TYPE);
> > +	if (ret)
> > +		return -EINVAL;
>
> This should be
>
> 	if (ret)
> 		return ret;
>
> Every call to uverbs_get_const is wrong in this same way..

Right, from technical point of view uverbs_get_const can return EINVAL
only, and it is correct for now, but need to be changed to proper
"return ret".

>
> I can probably fix it if this is the only thing though..
>

Thanks, I appreciate it.

> Jason

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

^ permalink raw reply

* [GIT] net merged into net-next
From: David Miller @ 2018-09-05  5:21 UTC (permalink / raw)
  To: netdev


Just FYI...

^ permalink raw reply

* Re: [Patch net v3] tipc: call start and done ops directly in __tipc_nl_compat_dumpit()
From: Cong Wang @ 2018-09-05  5:49 UTC (permalink / raw)
  To: Ying Xue; +Cc: Linux Kernel Network Developers, tipc-discussion, Jon Maloy
In-Reply-To: <e1e17123-0b69-291b-db04-ccc8e1be1c97@windriver.com>

On Tue, Sep 4, 2018 at 8:25 PM Ying Xue <ying.xue@windriver.com> wrote:
>
> On 09/05/2018 05:54 AM, Cong Wang wrote:
> > __tipc_nl_compat_dumpit() uses a netlink_callback on stack,
> > so the only way to align it with other ->dumpit() call path
> > is calling tipc_dump_start() and tipc_dump_done() directly
> > inside it. Otherwise ->dumpit() would always get NULL from
> > cb->args[].
> >
> > But tipc_dump_start() uses sock_net(cb->skb->sk) to retrieve
> > net pointer, the cb->skb here doesn't set skb->sk, the net pointer
> > is saved in msg->net instead, so introduce a helper function
> > __tipc_dump_start() to pass in msg->net.
> >
> > Ying pointed out cb->args[0...3] are already used by other
> > callbacks on this call path, so we can't use cb->args[0] any
> > more, use cb->args[4] instead.
>
> It's a common mechanism to save rhashtable iterator pointer in cb->args
> after tipc_dump_start() and tipc_dump_done() are introduced. Someday
> probably we will involve new dumpit function. In order to lower the risk
> that rhashtable iterator pointer saved is overwritten, it's better to
> use the last slot, ie, cb->args[5].

I don't understand, currently only cb->args[0..3] are used at most,
therefore cb->args[4] is pretty safe in current code base, there is
no reason to be so defensive to use cb->args[5].

If you really worry about future, you probably want to extend cb->args
from 6 to whatever larger, rather than just skipping cb->args[4].

I don't see any reason to do so.

^ 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