Netdev List
 help / color / mirror / Atom feed
* Re: [RFC PATCH 3/5] net: macb: Add pm runtime support
From: Claudiu Beznea @ 2018-05-03 12:59 UTC (permalink / raw)
  To: Harini Katakam
  Cc: Nicolas Ferre, David Miller, netdev, linux-kernel, michals,
	appanad, Shubhrajyoti Datta
In-Reply-To: <CAFcVECKWz6bGZpg7UF=4hObUjca12Oq5NbE0SBL+v=1grJP8YA@mail.gmail.com>



On 03.05.2018 14:13, Harini Katakam wrote:
> Hi Claudiu,
> 
> On Thu, May 3, 2018 at 3:39 PM, Claudiu Beznea
> <Claudiu.Beznea@microchip.com> wrote:
>>
>>
>> On 22.03.2018 15:51, harinikatakamlinux@gmail.com wrote:
>>> From: Harini Katakam <harinik@xilinx.com>
> <snip>
>> I would use a "goto" instruction, e.g.:
>>                 value = -ETIMEDOUT;
>>                 goto out;
>>
> 
> Will do
> 
>>
>> Below, in macb_open() you have a return err; case:
>>         err = macb_alloc_consistent(bp);
>>         if (err) {
>>                 netdev_err(dev, "Unable to allocate DMA memory (error %d)\n",
>>                            err);
>>                 return err;
>>         }
>>
>> You have to undo pm_runtime_get_sync() with pm_runtime_put_sync() or something
>> similar to decrement dev->power.usage_count.
> 
> Will do
> 
> <snip>
>>> @@ -4104,11 +4145,16 @@ static int macb_remove(struct platform_device *pdev)
>>>               mdiobus_free(bp->mii_bus);
>>>
>>>               unregister_netdev(dev);
>>> -             clk_disable_unprepare(bp->tx_clk);
>>> -             clk_disable_unprepare(bp->hclk);
>>> -             clk_disable_unprepare(bp->pclk);
>>> -             clk_disable_unprepare(bp->rx_clk);
>>> -             clk_disable_unprepare(bp->tsu_clk);
>>> +             pm_runtime_disable(&pdev->dev);
>>> +             pm_runtime_dont_use_autosuspend(&pdev->dev);
>>> +             if (!pm_runtime_suspended(&pdev->dev)) {
>>> +                     clk_disable_unprepare(bp->tx_clk);
>>> +                     clk_disable_unprepare(bp->hclk);
>>> +                     clk_disable_unprepare(bp->pclk);
>>> +                     clk_disable_unprepare(bp->rx_clk);
>>> +                     clk_disable_unprepare(bp->tsu_clk);
>>> +                     pm_runtime_set_suspended(&pdev->dev);
>>
>> This is driver remove function. Shouldn't clocks be removed?
> 
> clk_disable_unprepare IS being done here.
> The check for !pm_runtime_suspended is just to make sure the
> clocks are not already removed (in runtime_suspend).

Could this happen? Looking over code, starting with 
platform_driver_unregister() it looks to me that the runtime resume
is called just before driver remove is called.

platform_driver_unregister() ->
driver_unregister() ->
bus_remove_driver() ->
driver_detach() ->
device_release_driver_internal() ->
__device_release_driver()
{
	// ...
        pm_runtime_get_sync(dev);		// runtime resume                                       
        pm_runtime_clean_up_links(dev);                                 
 
	// ...

	pm_runtime_put_sync(dev);                                       
                                                                                
        if (dev->bus && dev->bus->remove)                               
        	dev->bus->remove(dev);                                  
        else if (drv->remove)                                           
                drv->remove(dev);                                       

	// ...
}

Thank you,
Claudiu

> 
> Regards,
> Harini
> 

^ permalink raw reply

* Re: [PATCH net-next mlxsw v2 1/2] switchdev: Add fdb.added_by_user to switchdev notifications
From: Ivan Vecera @ 2018-05-03 12:59 UTC (permalink / raw)
  To: Petr Machata, netdev, bridge
  Cc: jiri, idosch, davem, stephen, andrew, vivien.didelot, f.fainelli
In-Reply-To: <36a671d4d30995902f5c3acc68cf8317f8ce05cb.1525350809.git.petrm@mellanox.com>

On 3.5.2018 14:43, Petr Machata wrote:
> The following patch enables sending notifications also for events on FDB
> entries that weren't added by the user. Give the drivers the information
> necessary to distinguish between the two origins of FDB entries.
> 
> To maintain the current behavior, have switchdev-implementing drivers
> bail out on notifications about non-user-added FDB entries. In case of
> mlxsw driver, allow a call to mlxsw_sp_span_respin() so that SPAN over
> bridge catches up with the changed FDB.
> 
> Signed-off-by: Petr Machata <petrm@mellanox.com>
> Reviewed-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
> ---
>  drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c |  4 ++++
>  drivers/net/ethernet/rocker/rocker_main.c                |  2 ++
>  include/net/switchdev.h                                  |  1 +
>  net/bridge/br_switchdev.c                                | 10 +++++++---
>  net/dsa/slave.c                                          |  5 ++++-
>  5 files changed, 18 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c
> index 1af99fe..3973d90 100644
> --- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c
> +++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c
> @@ -2270,6 +2270,8 @@ static void mlxsw_sp_switchdev_event_work(struct work_struct *work)
>  	switch (switchdev_work->event) {
>  	case SWITCHDEV_FDB_ADD_TO_DEVICE:
>  		fdb_info = &switchdev_work->fdb_info;
> +		if (!fdb_info->added_by_user)
> +			break;
>  		err = mlxsw_sp_port_fdb_set(mlxsw_sp_port, fdb_info, true);
>  		if (err)
>  			break;
> @@ -2279,6 +2281,8 @@ static void mlxsw_sp_switchdev_event_work(struct work_struct *work)
>  		break;
>  	case SWITCHDEV_FDB_DEL_TO_DEVICE:
>  		fdb_info = &switchdev_work->fdb_info;
> +		if (!fdb_info->added_by_user)
> +			break;
>  		mlxsw_sp_port_fdb_set(mlxsw_sp_port, fdb_info, false);
>  		break;
>  	case SWITCHDEV_FDB_ADD_TO_BRIDGE: /* fall through */
> diff --git a/drivers/net/ethernet/rocker/rocker_main.c b/drivers/net/ethernet/rocker/rocker_main.c
> index 056cb60..152d694 100644
> --- a/drivers/net/ethernet/rocker/rocker_main.c
> +++ b/drivers/net/ethernet/rocker/rocker_main.c
> @@ -2783,6 +2783,8 @@ static int rocker_switchdev_event(struct notifier_block *unused,
>  	switch (event) {
>  	case SWITCHDEV_FDB_ADD_TO_DEVICE: /* fall through */
>  	case SWITCHDEV_FDB_DEL_TO_DEVICE:
> +		if (!fdb_info->added_by_user)
> +			break;
>  		memcpy(&switchdev_work->fdb_info, ptr,
>  		       sizeof(switchdev_work->fdb_info));
>  		switchdev_work->fdb_info.addr = kzalloc(ETH_ALEN, GFP_ATOMIC);
> diff --git a/include/net/switchdev.h b/include/net/switchdev.h
> index 39bc855..d574ce6 100644
> --- a/include/net/switchdev.h
> +++ b/include/net/switchdev.h
> @@ -155,6 +155,7 @@ struct switchdev_notifier_fdb_info {
>  	struct switchdev_notifier_info info; /* must be first */
>  	const unsigned char *addr;
>  	u16 vid;
> +	bool added_by_user;
>  };
>  
>  static inline struct net_device *
> diff --git a/net/bridge/br_switchdev.c b/net/bridge/br_switchdev.c
> index ee775f4..71a03c4 100644
> --- a/net/bridge/br_switchdev.c
> +++ b/net/bridge/br_switchdev.c
> @@ -102,13 +102,15 @@ int br_switchdev_set_port_flag(struct net_bridge_port *p,
>  
>  static void
>  br_switchdev_fdb_call_notifiers(bool adding, const unsigned char *mac,
> -				u16 vid, struct net_device *dev)
> +				u16 vid, struct net_device *dev,
> +				bool added_by_user)
>  {
>  	struct switchdev_notifier_fdb_info info;
>  	unsigned long notifier_type;
>  
>  	info.addr = mac;
>  	info.vid = vid;
> +	info.added_by_user = added_by_user;
>  	notifier_type = adding ? SWITCHDEV_FDB_ADD_TO_DEVICE : SWITCHDEV_FDB_DEL_TO_DEVICE;
>  	call_switchdev_notifiers(notifier_type, dev, &info.info);
>  }
> @@ -123,12 +125,14 @@ br_switchdev_fdb_notify(const struct net_bridge_fdb_entry *fdb, int type)
>  	case RTM_DELNEIGH:
>  		br_switchdev_fdb_call_notifiers(false, fdb->key.addr.addr,
>  						fdb->key.vlan_id,
> -						fdb->dst->dev);
> +						fdb->dst->dev,
> +						fdb->added_by_user);
>  		break;
>  	case RTM_NEWNEIGH:
>  		br_switchdev_fdb_call_notifiers(true, fdb->key.addr.addr,
>  						fdb->key.vlan_id,
> -						fdb->dst->dev);
> +						fdb->dst->dev,
> +						fdb->added_by_user);
>  		break;
>  	}
>  }
> diff --git a/net/dsa/slave.c b/net/dsa/slave.c
> index f3fb3a0..c287f1e 100644
> --- a/net/dsa/slave.c
> +++ b/net/dsa/slave.c
> @@ -1441,6 +1441,7 @@ static int dsa_slave_switchdev_event(struct notifier_block *unused,
>  				     unsigned long event, void *ptr)
>  {
>  	struct net_device *dev = switchdev_notifier_info_to_dev(ptr);
> +	struct switchdev_notifier_fdb_info *fdb_info = ptr;
>  	struct dsa_switchdev_event_work *switchdev_work;
>  
>  	if (!dsa_slave_dev_check(dev))
> @@ -1458,8 +1459,10 @@ static int dsa_slave_switchdev_event(struct notifier_block *unused,
>  	switch (event) {
>  	case SWITCHDEV_FDB_ADD_TO_DEVICE: /* fall through */
>  	case SWITCHDEV_FDB_DEL_TO_DEVICE:
> +		if (!fdb_info->added_by_user)
> +			break;
>  		if (dsa_slave_switchdev_fdb_work_init(switchdev_work,
> -						      ptr))
> +						      fdb_info))
>  			goto err_fdb_work_init;
>  		dev_hold(dev);
>  		break;
> 
LGTM

Acked-by: Ivan Vecera <ivecera@redhat.com>

^ permalink raw reply

* Re: [PATCH net-next mlxsw v2 2/2] net: bridge: Notify about !added_by_user FDB entries
From: Ivan Vecera @ 2018-05-03 13:00 UTC (permalink / raw)
  To: Petr Machata, netdev, bridge
  Cc: jiri, idosch, davem, stephen, andrew, vivien.didelot, f.fainelli
In-Reply-To: <33ac8ce3fcd084190c5797a409ebd7858f7d47db.1525350809.git.petrm@mellanox.com>

On 3.5.2018 14:43, Petr Machata wrote:
> Do not automatically bail out on sending notifications about activity on
> non-user-added FDB entries. Instead, notify about this activity except
> for cases where the activity itself originates in a notification, to
> avoid sending duplicate notifications.
> 
> Signed-off-by: Petr Machata <petrm@mellanox.com>
> ---
>  net/bridge/br.c           |  4 ++--
>  net/bridge/br_fdb.c       | 47 ++++++++++++++++++++++++++---------------------
>  net/bridge/br_private.h   |  6 ++++--
>  net/bridge/br_switchdev.c |  2 +-
>  4 files changed, 33 insertions(+), 26 deletions(-)
> 
> diff --git a/net/bridge/br.c b/net/bridge/br.c
> index 671d13c..c6b033e 100644
> --- a/net/bridge/br.c
> +++ b/net/bridge/br.c
> @@ -141,7 +141,7 @@ static int br_switchdev_event(struct notifier_block *unused,
>  	case SWITCHDEV_FDB_ADD_TO_BRIDGE:
>  		fdb_info = ptr;
>  		err = br_fdb_external_learn_add(br, p, fdb_info->addr,
> -						fdb_info->vid);
> +						fdb_info->vid, false);
>  		if (err) {
>  			err = notifier_from_errno(err);
>  			break;
> @@ -152,7 +152,7 @@ static int br_switchdev_event(struct notifier_block *unused,
>  	case SWITCHDEV_FDB_DEL_TO_BRIDGE:
>  		fdb_info = ptr;
>  		err = br_fdb_external_learn_del(br, p, fdb_info->addr,
> -						fdb_info->vid);
> +						fdb_info->vid, false);
>  		if (err)
>  			err = notifier_from_errno(err);
>  		break;
> diff --git a/net/bridge/br_fdb.c b/net/bridge/br_fdb.c
> index a1c409c..b19e310 100644
> --- a/net/bridge/br_fdb.c
> +++ b/net/bridge/br_fdb.c
> @@ -40,7 +40,7 @@ static struct kmem_cache *br_fdb_cache __read_mostly;
>  static int fdb_insert(struct net_bridge *br, struct net_bridge_port *source,
>  		      const unsigned char *addr, u16 vid);
>  static void fdb_notify(struct net_bridge *br,
> -		       const struct net_bridge_fdb_entry *, int);
> +		       const struct net_bridge_fdb_entry *, int, bool);
>  
>  int __init br_fdb_init(void)
>  {
> @@ -195,7 +195,8 @@ static void fdb_del_hw_addr(struct net_bridge *br, const unsigned char *addr)
>  	}
>  }
>  
> -static void fdb_delete(struct net_bridge *br, struct net_bridge_fdb_entry *f)
> +static void fdb_delete(struct net_bridge *br, struct net_bridge_fdb_entry *f,
> +		       bool swdev_notify)
>  {
>  	trace_fdb_delete(br, f);
>  
> @@ -205,7 +206,7 @@ static void fdb_delete(struct net_bridge *br, struct net_bridge_fdb_entry *f)
>  	hlist_del_init_rcu(&f->fdb_node);
>  	rhashtable_remove_fast(&br->fdb_hash_tbl, &f->rhnode,
>  			       br_fdb_rht_params);
> -	fdb_notify(br, f, RTM_DELNEIGH);
> +	fdb_notify(br, f, RTM_DELNEIGH, swdev_notify);
>  	call_rcu(&f->rcu, fdb_rcu_free);
>  }
>  
> @@ -241,7 +242,7 @@ static void fdb_delete_local(struct net_bridge *br,
>  		return;
>  	}
>  
> -	fdb_delete(br, f);
> +	fdb_delete(br, f, true);
>  }
>  
>  void br_fdb_find_delete_local(struct net_bridge *br,
> @@ -356,7 +357,7 @@ void br_fdb_cleanup(struct work_struct *work)
>  		} else {
>  			spin_lock_bh(&br->hash_lock);
>  			if (!hlist_unhashed(&f->fdb_node))
> -				fdb_delete(br, f);
> +				fdb_delete(br, f, true);
>  			spin_unlock_bh(&br->hash_lock);
>  		}
>  	}
> @@ -376,7 +377,7 @@ void br_fdb_flush(struct net_bridge *br)
>  	spin_lock_bh(&br->hash_lock);
>  	hlist_for_each_entry_safe(f, tmp, &br->fdb_list, fdb_node) {
>  		if (!f->is_static)
> -			fdb_delete(br, f);
> +			fdb_delete(br, f, true);
>  	}
>  	spin_unlock_bh(&br->hash_lock);
>  }
> @@ -405,7 +406,7 @@ void br_fdb_delete_by_port(struct net_bridge *br,
>  		if (f->is_local)
>  			fdb_delete_local(br, p, f);
>  		else
> -			fdb_delete(br, f);
> +			fdb_delete(br, f, true);
>  	}
>  	spin_unlock_bh(&br->hash_lock);
>  }
> @@ -531,7 +532,7 @@ static int fdb_insert(struct net_bridge *br, struct net_bridge_port *source,
>  			return 0;
>  		br_warn(br, "adding interface %s with same address as a received packet (addr:%pM, vlan:%u)\n",
>  		       source ? source->dev->name : br->dev->name, addr, vid);
> -		fdb_delete(br, fdb);
> +		fdb_delete(br, fdb, true);
>  	}
>  
>  	fdb = fdb_create(br, source, addr, vid, 1, 1);
> @@ -539,7 +540,7 @@ static int fdb_insert(struct net_bridge *br, struct net_bridge_port *source,
>  		return -ENOMEM;
>  
>  	fdb_add_hw_addr(br, addr);
> -	fdb_notify(br, fdb, RTM_NEWNEIGH);
> +	fdb_notify(br, fdb, RTM_NEWNEIGH, true);
>  	return 0;
>  }
>  
> @@ -594,7 +595,7 @@ void br_fdb_update(struct net_bridge *br, struct net_bridge_port *source,
>  				fdb->added_by_user = 1;
>  			if (unlikely(fdb_modified)) {
>  				trace_br_fdb_update(br, source, addr, vid, added_by_user);
> -				fdb_notify(br, fdb, RTM_NEWNEIGH);
> +				fdb_notify(br, fdb, RTM_NEWNEIGH, true);
>  			}
>  		}
>  	} else {
> @@ -605,7 +606,7 @@ void br_fdb_update(struct net_bridge *br, struct net_bridge_port *source,
>  				fdb->added_by_user = 1;
>  			trace_br_fdb_update(br, source, addr, vid,
>  					    added_by_user);
> -			fdb_notify(br, fdb, RTM_NEWNEIGH);
> +			fdb_notify(br, fdb, RTM_NEWNEIGH, true);
>  		}
>  		/* else  we lose race and someone else inserts
>  		 * it first, don't bother updating
> @@ -687,13 +688,15 @@ static inline size_t fdb_nlmsg_size(void)
>  }
>  
>  static void fdb_notify(struct net_bridge *br,
> -		       const struct net_bridge_fdb_entry *fdb, int type)
> +		       const struct net_bridge_fdb_entry *fdb, int type,
> +		       bool swdev_notify)
>  {
>  	struct net *net = dev_net(br->dev);
>  	struct sk_buff *skb;
>  	int err = -ENOBUFS;
>  
> -	br_switchdev_fdb_notify(fdb, type);
> +	if (swdev_notify)
> +		br_switchdev_fdb_notify(fdb, type);
>  
>  	skb = nlmsg_new(fdb_nlmsg_size(), GFP_ATOMIC);
>  	if (skb == NULL)
> @@ -832,7 +835,7 @@ static int fdb_add_entry(struct net_bridge *br, struct net_bridge_port *source,
>  	fdb->used = jiffies;
>  	if (modified) {
>  		fdb->updated = jiffies;
> -		fdb_notify(br, fdb, RTM_NEWNEIGH);
> +		fdb_notify(br, fdb, RTM_NEWNEIGH, true);
>  	}
>  
>  	return 0;
> @@ -856,7 +859,7 @@ static int __br_fdb_add(struct ndmsg *ndm, struct net_bridge *br,
>  		rcu_read_unlock();
>  		local_bh_enable();
>  	} else if (ndm->ndm_flags & NTF_EXT_LEARNED) {
> -		err = br_fdb_external_learn_add(br, p, addr, vid);
> +		err = br_fdb_external_learn_add(br, p, addr, vid, true);
>  	} else {
>  		spin_lock_bh(&br->hash_lock);
>  		err = fdb_add_entry(br, p, addr, ndm->ndm_state,
> @@ -945,7 +948,7 @@ static int fdb_delete_by_addr_and_port(struct net_bridge *br,
>  	if (!fdb || fdb->dst != p)
>  		return -ENOENT;
>  
> -	fdb_delete(br, fdb);
> +	fdb_delete(br, fdb, true);
>  
>  	return 0;
>  }
> @@ -1065,7 +1068,8 @@ void br_fdb_unsync_static(struct net_bridge *br, struct net_bridge_port *p)
>  }
>  
>  int br_fdb_external_learn_add(struct net_bridge *br, struct net_bridge_port *p,
> -			      const unsigned char *addr, u16 vid)
> +			      const unsigned char *addr, u16 vid,
> +			      bool swdev_notify)
>  {
>  	struct net_bridge_fdb_entry *fdb;
>  	bool modified = false;
> @@ -1083,7 +1087,7 @@ int br_fdb_external_learn_add(struct net_bridge *br, struct net_bridge_port *p,
>  			goto err_unlock;
>  		}
>  		fdb->added_by_external_learn = 1;
> -		fdb_notify(br, fdb, RTM_NEWNEIGH);
> +		fdb_notify(br, fdb, RTM_NEWNEIGH, swdev_notify);
>  	} else {
>  		fdb->updated = jiffies;
>  
> @@ -1102,7 +1106,7 @@ int br_fdb_external_learn_add(struct net_bridge *br, struct net_bridge_port *p,
>  		}
>  
>  		if (modified)
> -			fdb_notify(br, fdb, RTM_NEWNEIGH);
> +			fdb_notify(br, fdb, RTM_NEWNEIGH, swdev_notify);
>  	}
>  
>  err_unlock:
> @@ -1112,7 +1116,8 @@ int br_fdb_external_learn_add(struct net_bridge *br, struct net_bridge_port *p,
>  }
>  
>  int br_fdb_external_learn_del(struct net_bridge *br, struct net_bridge_port *p,
> -			      const unsigned char *addr, u16 vid)
> +			      const unsigned char *addr, u16 vid,
> +			      bool swdev_notify)
>  {
>  	struct net_bridge_fdb_entry *fdb;
>  	int err = 0;
> @@ -1121,7 +1126,7 @@ int br_fdb_external_learn_del(struct net_bridge *br, struct net_bridge_port *p,
>  
>  	fdb = br_fdb_find(br, addr, vid);
>  	if (fdb && fdb->added_by_external_learn)
> -		fdb_delete(br, fdb);
> +		fdb_delete(br, fdb, swdev_notify);
>  	else
>  		err = -ENOENT;
>  
> diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
> index 1a50931..80a69b8 100644
> --- a/net/bridge/br_private.h
> +++ b/net/bridge/br_private.h
> @@ -553,9 +553,11 @@ int br_fdb_dump(struct sk_buff *skb, struct netlink_callback *cb,
>  int br_fdb_sync_static(struct net_bridge *br, struct net_bridge_port *p);
>  void br_fdb_unsync_static(struct net_bridge *br, struct net_bridge_port *p);
>  int br_fdb_external_learn_add(struct net_bridge *br, struct net_bridge_port *p,
> -			      const unsigned char *addr, u16 vid);
> +			      const unsigned char *addr, u16 vid,
> +			      bool swdev_notify);
>  int br_fdb_external_learn_del(struct net_bridge *br, struct net_bridge_port *p,
> -			      const unsigned char *addr, u16 vid);
> +			      const unsigned char *addr, u16 vid,
> +			      bool swdev_notify);
>  void br_fdb_offloaded_set(struct net_bridge *br, struct net_bridge_port *p,
>  			  const unsigned char *addr, u16 vid);
>  
> diff --git a/net/bridge/br_switchdev.c b/net/bridge/br_switchdev.c
> index 71a03c4..35474d4 100644
> --- a/net/bridge/br_switchdev.c
> +++ b/net/bridge/br_switchdev.c
> @@ -118,7 +118,7 @@ br_switchdev_fdb_call_notifiers(bool adding, const unsigned char *mac,
>  void
>  br_switchdev_fdb_notify(const struct net_bridge_fdb_entry *fdb, int type)
>  {
> -	if (!fdb->added_by_user || !fdb->dst)
> +	if (!fdb->dst)
>  		return;
>  
>  	switch (type) {
> 

Acked-by: Ivan Vecera <ivecera@redhat.com>

^ permalink raw reply

* Re: [PATCH v3 09/20] iommu: Remove depends on HAS_DMA in case of platform dependency
From: Joerg Roedel @ 2018-05-03 13:02 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Ulf Hansson, Wolfram Sang, linux-iio-u79uwXL29TY76Z2rM5mHXA,
	linux-fpga-u79uwXL29TY76Z2rM5mHXA,
	linux-remoteproc-u79uwXL29TY76Z2rM5mHXA,
	alsa-devel-K7yf7f+aM1XWsZ/bQMPhNw, Bjorn Andersson, Eric Anholt,
	netdev-u79uwXL29TY76Z2rM5mHXA,
	linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-i2c-u79uwXL29TY76Z2rM5mHXA,
	linux1394-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f,
	Christoph Hellwig, Stefan Wahren, Boris Brezillon, Herbert Xu,
	Richard Weinberger, Jassi Brar, Marek Vasut,
	linux-serial-u79uwXL29TY76Z2rM5mHXA, Matias Bjorling,
	linux-media-u79uwXL29TY76Z2rM5mHXA, Ohad Ben-Cohen,
	devel-gWbeCf7V1WCQmaza687I9mD2FQJk+8+b, Alan Tull,
	Bartlomiej Zolnierkiewicz <
In-Reply-To: <1523987360-18760-10-git-send-email-geert-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.org>

On Tue, Apr 17, 2018 at 07:49:09PM +0200, Geert Uytterhoeven wrote:
> Remove dependencies on HAS_DMA where a Kconfig symbol depends on another
> symbol that implies HAS_DMA, and, optionally, on "|| COMPILE_TEST".
> In most cases this other symbol is an architecture or platform specific
> symbol, or PCI.
> 
> Generic symbols and drivers without platform dependencies keep their
> dependencies on HAS_DMA, to prevent compiling subsystems or drivers that
> cannot work anyway.
> 
> This simplifies the dependencies, and allows to improve compile-testing.
> 
> Signed-off-by: Geert Uytterhoeven <geert-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.org>
> Reviewed-by: Mark Brown <broonie-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> Acked-by: Robin Murphy <robin.murphy-5wv7dgnIgG8@public.gmane.org>
> Acked-by: Joerg Roedel <jroedel-l3A5Bk7waGM@public.gmane.org>

Applied, thanks.

^ permalink raw reply

* [PATCH] dt-bindings: can: rcar_can: Fix R8A7796 SoC name
From: Geert Uytterhoeven @ 2018-05-03 13:02 UTC (permalink / raw)
  To: Wolfgang Grandegger, Marc Kleine-Budde, Rob Herring, Mark Rutland
  Cc: Chris Paterson, linux-can, netdev, devicetree, linux-renesas-soc,
	Geert Uytterhoeven

R8A7796 is R-Car M3-W.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
 Documentation/devicetree/bindings/net/can/rcar_canfd.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/net/can/rcar_canfd.txt b/Documentation/devicetree/bindings/net/can/rcar_canfd.txt
index 93c3a6ae32f995e9..1a4ee1d2506de532 100644
--- a/Documentation/devicetree/bindings/net/can/rcar_canfd.txt
+++ b/Documentation/devicetree/bindings/net/can/rcar_canfd.txt
@@ -5,7 +5,7 @@ Required properties:
 - compatible: Must contain one or more of the following:
   - "renesas,rcar-gen3-canfd" for R-Car Gen3 compatible controller.
   - "renesas,r8a7795-canfd" for R8A7795 (R-Car H3) compatible controller.
-  - "renesas,r8a7796-canfd" for R8A7796 (R-Car M3) compatible controller.
+  - "renesas,r8a7796-canfd" for R8A7796 (R-Car M3-W) compatible controller.
 
   When compatible with the generic version, nodes must list the
   SoC-specific version corresponding to the platform first, followed by the
-- 
2.7.4

^ permalink raw reply related

* Re: [PATCH net-next mlxsw v2 2/2] net: bridge: Notify about !added_by_user FDB entries
From: Petr Machata @ 2018-05-03 13:07 UTC (permalink / raw)
  To: Nikolay Aleksandrov
  Cc: netdev, bridge, jiri, idosch, ivecera, davem, stephen, andrew,
	vivien.didelot, f.fainelli
In-Reply-To: <39b65224-cd35-4716-1a8c-cc1bfaff0654@cumulusnetworks.com>

Nikolay Aleksandrov <nikolay@cumulusnetworks.com> writes:

> On 03/05/18 15:43, Petr Machata wrote:
>> Do not automatically bail out on sending notifications about activity on
>> non-user-added FDB entries. Instead, notify about this activity except
>> for cases where the activity itself originates in a notification, to
>> avoid sending duplicate notifications.
>>
>> Signed-off-by: Petr Machata <petrm@mellanox.com>
>> ---
>>   net/bridge/br.c           |  4 ++--
>>   net/bridge/br_fdb.c       | 47 ++++++++++++++++++++++++++---------------------
>>   net/bridge/br_private.h   |  6 ++++--
>>   net/bridge/br_switchdev.c |  2 +-
>>   4 files changed, 33 insertions(+), 26 deletions(-)
>>
>
> Thanks, looks good to me! In the future please add the reviewers to the CC list
> when sending a v2, I actually missed the v2 set and saw your reply to the cover
> letter patch later.

Sorry, I'm still tweaking my process for direct submission after going
almost exclusively through mlxsw for so long!

> Acked-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>

Thanks,
Petr

^ permalink raw reply

* Re: [PATCH v1 net-next] net: dsa: mv88e6xxx: Fix name string for 88E6141
From: Andrew Lunn @ 2018-05-03 13:11 UTC (permalink / raw)
  To: Marek Behún
  Cc: netdev, Greg Kroah-Hartman, Vivien Didelot, Arkadi Sharshevsky,
	David S . Miller
In-Reply-To: <20180503130655.23275-1-marek.behun@nic.cz>

On Thu, May 03, 2018 at 03:06:55PM +0200, Marek Behún wrote:
> The structure was copied from 88E6341 but the name was not changed.
> 
> Signed-off-by: Marek Behun <marek.behun@nic.cz>

Hi Marek

Which tree is this against?

> ---
>  drivers/net/dsa/mv88e6xxx/chip.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
> index e9600a82dc83..fae362020305 100644
> --- a/drivers/net/dsa/mv88e6xxx/chip.c
> +++ b/drivers/net/dsa/mv88e6xxx/chip.c
> @@ -3206,7 +3206,7 @@ static const struct mv88e6xxx_info mv88e6xxx_table[] = {
>  	[MV88E6141] = {
>  		.prod_num = MV88E6XXX_PORT_SWITCH_ID_PROD_6141,
>  		.family = MV88E6XXX_FAMILY_6341,
> -		.name = "Marvell 88E6341",
> +		.name = "Marvell 88E6141",
>  		.num_databases = 4096,
>  		.num_ports = 6,
>  		.max_vid = 4095,
> -- 
> 2.16.1
> 

commit 79a68b2631d8ec3e149081b1ecfb23509c040b4e
Author: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
Date:   Tue Mar 20 10:44:40 2018 +0100

    net: dsa: mv88e6xxx: Fix name of switch 88E6141
    
    The switch name is emitted in the kernel log, so having the right name
    there is nice.
    
    Fixes: 1558727a1c1b ("net: dsa: mv88e6xxx: Add support for ethernet switch 88E6141")
    Reviewed-by: Andrew Lunn <andrew@lunn.ch>
    Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
    Signed-off-by: David S. Miller <davem@davemloft.net>

^ permalink raw reply

* Re: [PATCH net-next v2 00/15] ARM: sun8i: r40: Add Ethernet support
From: Maxime Ripard @ 2018-05-03 13:12 UTC (permalink / raw)
  To: David Miller
  Cc: wens, mturquette, sboyd, peppe.cavallaro, robh+dt, mark.rutland,
	broonie, linux-arm-kernel, linux-clk, devicetree, netdev,
	clabbe.montjoie, icenowy
In-Reply-To: <20180502.110617.304411895589508709.davem@davemloft.net>

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

Hi Dave,

On Wed, May 02, 2018 at 11:06:17AM -0400, David Miller wrote:
> From: Chen-Yu Tsai <wens@csie.org>
> Date: Wed, 2 May 2018 00:33:45 +0800
> 
> > I should've mentioned that patches 3 ~ 10, and only these, should go
> > through net-next. sunxi will handle the remaining clk, device tree, and
> > soc driver patches.
> 
> Ok, I just noticed this.
> 
> Why don't you just post those patches separately as a series on their
> own then, in order to avoid confusion?
> 
> Then you can adjust the patch series header posting to explain the
> non-net-next changes, where they got merged, and what they provide
> in order to faciliate the net-next changes.

I now that we usually have some feedback from non-net maintainers that
they actually prefer seeing the full picture (and I also tend to
prefer that as well) and having all the patches relevant to enable a
particular feature, even if it means getting multiple maintainers
involved.

Just to make sure we understood you fully, do you want Chen-Yu to
resend his serie following your comments, or was that just a general
remark for next time?

Thanks!
Maxime

-- 
Maxime Ripard, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com

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

^ permalink raw reply

* [PATCH v1 net-next] net: dsa: mv88e6xxx: Fix name string for 88E6141
From: Marek Behún @ 2018-05-03 13:06 UTC (permalink / raw)
  To: netdev
  Cc: Andrew Lunn, Greg Kroah-Hartman, Vivien Didelot,
	Arkadi Sharshevsky, David S . Miller, Marek Behún

The structure was copied from 88E6341 but the name was not changed.

Signed-off-by: Marek Behun <marek.behun@nic.cz>
---
 drivers/net/dsa/mv88e6xxx/chip.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index e9600a82dc83..fae362020305 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -3206,7 +3206,7 @@ static const struct mv88e6xxx_info mv88e6xxx_table[] = {
 	[MV88E6141] = {
 		.prod_num = MV88E6XXX_PORT_SWITCH_ID_PROD_6141,
 		.family = MV88E6XXX_FAMILY_6341,
-		.name = "Marvell 88E6341",
+		.name = "Marvell 88E6141",
 		.num_databases = 4096,
 		.num_ports = 6,
 		.max_vid = 4095,
-- 
2.16.1

^ permalink raw reply related

* [PATCH v1 net-next] net: dsa: mv88e6xxx: 88E6141/6341 SERDES support
From: Marek Behún @ 2018-05-03 13:06 UTC (permalink / raw)
  To: netdev
  Cc: Andrew Lunn, Greg Kroah-Hartman, Vivien Didelot,
	Arkadi Sharshevsky, David S . Miller, Marek Behún

The 88E6141/6341 switches (also known as Topaz) have 1 SGMII lane,
which can be configured the same way as the SERDES lane on 88E6390.

Signed-off-by: Marek Behun <marek.behun@nic.cz>
---
 drivers/net/dsa/mv88e6xxx/chip.c   |  2 ++
 drivers/net/dsa/mv88e6xxx/serdes.c | 20 ++++++++++++++++++++
 drivers/net/dsa/mv88e6xxx/serdes.h |  1 +
 3 files changed, 23 insertions(+)

diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index eebda5ec9676..e9600a82dc83 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -2426,6 +2426,7 @@ static const struct mv88e6xxx_ops mv88e6141_ops = {
 	.reset = mv88e6352_g1_reset,
 	.vtu_getnext = mv88e6352_g1_vtu_getnext,
 	.vtu_loadpurge = mv88e6352_g1_vtu_loadpurge,
+	.serdes_power = mv88e6341_serdes_power,
 };
 
 static const struct mv88e6xxx_ops mv88e6161_ops = {
@@ -2924,6 +2925,7 @@ static const struct mv88e6xxx_ops mv88e6341_ops = {
 	.reset = mv88e6352_g1_reset,
 	.vtu_getnext = mv88e6352_g1_vtu_getnext,
 	.vtu_loadpurge = mv88e6352_g1_vtu_loadpurge,
+	.serdes_power = mv88e6341_serdes_power,
 };
 
 static const struct mv88e6xxx_ops mv88e6350_ops = {
diff --git a/drivers/net/dsa/mv88e6xxx/serdes.c b/drivers/net/dsa/mv88e6xxx/serdes.c
index f3c01119b3d1..cd03a62946e3 100644
--- a/drivers/net/dsa/mv88e6xxx/serdes.c
+++ b/drivers/net/dsa/mv88e6xxx/serdes.c
@@ -227,3 +227,23 @@ int mv88e6390_serdes_power(struct mv88e6xxx_chip *chip, int port, bool on)
 
 	return 0;
 }
+
+int mv88e6341_serdes_power(struct mv88e6xxx_chip *chip, int port, bool on)
+{
+	int err;
+	u8 cmode;
+
+	if (port != 5)
+		return 0;
+
+	err = mv88e6xxx_port_get_cmode(chip, port, &cmode);
+	if (err)
+		return err;
+
+	if ((cmode == MV88E6XXX_PORT_STS_CMODE_1000BASE_X) ||
+	     (cmode == MV88E6XXX_PORT_STS_CMODE_SGMII) ||
+	     (cmode == MV88E6XXX_PORT_STS_CMODE_2500BASEX))
+		return mv88e6390_serdes_sgmii(chip, 0x15, on);
+
+	return 0;
+}
diff --git a/drivers/net/dsa/mv88e6xxx/serdes.h b/drivers/net/dsa/mv88e6xxx/serdes.h
index 5c1cd6d8e9a5..d7dc6decf69b 100644
--- a/drivers/net/dsa/mv88e6xxx/serdes.h
+++ b/drivers/net/dsa/mv88e6xxx/serdes.h
@@ -42,6 +42,7 @@
 #define MV88E6390_SGMII_CONTROL_LOOPBACK	BIT(14)
 #define MV88E6390_SGMII_CONTROL_PDOWN		BIT(11)
 
+int mv88e6341_serdes_power(struct mv88e6xxx_chip *chip, int port, bool on);
 int mv88e6352_serdes_power(struct mv88e6xxx_chip *chip, int port, bool on);
 int mv88e6390_serdes_power(struct mv88e6xxx_chip *chip, int port, bool on);
 
-- 
2.16.1

^ permalink raw reply related

* Re: [PATCH v1 net-next] net: dsa: mv88e6xxx: 88E6141/6341 SERDES support
From: Andrew Lunn @ 2018-05-03 13:15 UTC (permalink / raw)
  To: Marek Behún
  Cc: netdev, Greg Kroah-Hartman, Vivien Didelot, Arkadi Sharshevsky,
	David S . Miller
In-Reply-To: <20180503130648.23225-1-marek.behun@nic.cz>

On Thu, May 03, 2018 at 03:06:48PM +0200, Marek Behún wrote:
> The 88E6141/6341 switches (also known as Topaz) have 1 SGMII lane,
> which can be configured the same way as the SERDES lane on 88E6390.
> 
> Signed-off-by: Marek Behun <marek.behun@nic.cz>

> +int mv88e6341_serdes_power(struct mv88e6xxx_chip *chip, int port, bool on)
> +{
> +	int err;
> +	u8 cmode;
> +
> +	if (port != 5)
> +		return 0;
> +
> +	err = mv88e6xxx_port_get_cmode(chip, port, &cmode);
> +	if (err)
> +		return err;
> +
> +	if ((cmode == MV88E6XXX_PORT_STS_CMODE_1000BASE_X) ||
> +	     (cmode == MV88E6XXX_PORT_STS_CMODE_SGMII) ||
> +	     (cmode == MV88E6XXX_PORT_STS_CMODE_2500BASEX))
> +		return mv88e6390_serdes_sgmii(chip, 0x15, on);

Hi Marek

Please add a #define for this 0x15.

Otherwise, this looks good.

	   Andrew

^ permalink raw reply

* Re: [PATCH 1/2] ixgbe: release lock for the duration of ixgbe_suspend_close()
From: Alexander Duyck @ 2018-05-03 13:20 UTC (permalink / raw)
  To: Pavel Tatashin
  Cc: steven.sistare, daniel.m.jordan, LKML, Jeff Kirsher,
	intel-wired-lan, Netdev, Greg KH
In-Reply-To: <20180503035931.22439-2-pasha.tatashin@oracle.com>

On Wed, May 2, 2018 at 8:59 PM, Pavel Tatashin
<pasha.tatashin@oracle.com> wrote:
> Currently, during device_shutdown() ixgbe holds rtnl_lock for the duration
> of lengthy ixgbe_close_suspend(). On machines with multiple ixgbe cards
> this lock prevents scaling if device_shutdown() function is multi-threaded.
>
> It is not necessary to hold this lock during ixgbe_close_suspend()
> as it is not held when ixgbe_close() is called also during shutdown but for
> kexec case.
>
> Signed-off-by: Pavel Tatashin <pasha.tatashin@oracle.com>
> ---
>  drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> index afadba99f7b8..e7875b58854b 100644
> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> @@ -6748,8 +6748,15 @@ static int __ixgbe_shutdown(struct pci_dev *pdev, bool *enable_wake)
>         rtnl_lock();
>         netif_device_detach(netdev);
>
> -       if (netif_running(netdev))
> +       if (netif_running(netdev)) {
> +               /* Suspend takes a long time, device_shutdown may be
> +                * parallelized this function, so drop lock for the
> +                * duration of this call.
> +                */
> +               rtnl_unlock();
>                 ixgbe_close_suspend(adapter);
> +               rtnl_lock();
> +       }
>
>         ixgbe_clear_interrupt_scheme(adapter);
>         rtnl_unlock();

I'm not a fan of dropping the mutex while we go through
ixgbe_close_suspend. I'm concerned it will result in us having a
number of races on shutdown.

If anything, I think we would need to find a replacement for the mutex
that can operate at the per-netdev level if you are wanting to
parallelize this.

- Alex

^ permalink raw reply

* Re: [PATCH 1/2] ixgbe: release lock for the duration of ixgbe_suspend_close()
From: Pavel Tatashin @ 2018-05-03 13:30 UTC (permalink / raw)
  To: alexander.duyck
  Cc: Steven Sistare, Daniel Jordan, LKML, jeffrey.t.kirsher,
	intel-wired-lan, netdev, gregkh
In-Reply-To: <CAKgT0UdocfEm9oXZ1dkEMari8m3OA4uVTrYg45uj9fk2V41bxQ@mail.gmail.com>

Hi Alex,

> I'm not a fan of dropping the mutex while we go through
> ixgbe_close_suspend. I'm concerned it will result in us having a
> number of races on shutdown.

I would agree, but ixgbe_close_suspend() is already called without this
mutex from ixgbe_close(). This path is executed also during machine
shutdown but when kexec'ed. So, it is either an existing bug or there are
no races. But, because ixgbe_close() is called from the userland, and a
little earlier than ixgbe_shutdown() I think this means there are no races.


> If anything, I think we would need to find a replacement for the mutex
> that can operate at the per-netdev level if you are wanting to
> parallelize this.

Yes, if lock is necessary, it can be replaced in this place (and added to
ixgbe_close())  with something scalable.

Thank you,
Pavel

^ permalink raw reply

* Re: [PATCH] sctp: fix a potential missing-check bug
From: Wenwen Wang @ 2018-05-03 13:31 UTC (permalink / raw)
  To: Marcelo Ricardo Leitner
  Cc: Kangjie Lu, Vlad Yasevich, Neil Horman, David S. Miller,
	open list:SCTP PROTOCOL, open list:NETWORKING [GENERAL],
	open list, Wenwen Wang
In-Reply-To: <20180503124629.GM5105@localhost.localdomain>

On Thu, May 3, 2018 at 7:46 AM, Marcelo Ricardo Leitner
<marcelo.leitner@gmail.com> wrote:
> On Thu, May 03, 2018 at 07:01:51AM -0500, Wenwen Wang wrote:
>> On Wed, May 2, 2018 at 8:48 PM, Marcelo Ricardo Leitner
>> <marcelo.leitner@gmail.com> wrote:
>> > On Wed, May 02, 2018 at 08:27:05PM -0500, Wenwen Wang wrote:
>> >> On Wed, May 2, 2018 at 8:24 PM, Marcelo Ricardo Leitner
>> >> <marcelo.leitner@gmail.com> wrote:
>> >> > On Wed, May 02, 2018 at 08:15:45PM -0500, Wenwen Wang wrote:
>> >> >> In sctp_setsockopt_maxseg(), the integer 'val' is compared against min_len
>> >> >> and max_len to check whether it is in the appropriate range. If it is not,
>> >> >> an error code -EINVAL will be returned. This is enforced by a security
>> >> >> check. But, this check is only executed when 'val' is not 0. In fact, if
>> >> >> 'val' is 0, it will be assigned with a new value (if the return value of
>> >> >> the function sctp_id2assoc() is not 0) in the following execution. However,
>> >> >> this new value of 'val' is not checked before it is used to assigned to
>> >> >> asoc->user_frag. That means it is possible that the new value of 'val'
>> >> >> could be out of the expected range. This can cause security issues
>> >> >> such as buffer overflows, e.g., the new value of 'val' is used as an index
>> >> >> to access a buffer.
>> >> >>
>> >> >> This patch inserts a check for the new value of 'val' to see if it is in
>> >> >> the expected range. If it is not, an error code -EINVAL will be returned.
>> >> >>
>> >> >> Signed-off-by: Wenwen Wang <wang6495@umn.edu>
>> >> >> ---
>> >> >>  net/sctp/socket.c | 22 +++++++++++-----------
>> >> >>  1 file changed, 11 insertions(+), 11 deletions(-)
>> >> >
>> >> > ?
>> >> > This patch is the same as previous one. git send-email <old file>
>> >> > maybe?
>> >> >
>> >> >   Marcelo
>> >>
>> >> Thanks for your suggestion, Marcelo. I can send the old file. But, I
>> >> have added a line of comment in this patch.
>> >
>> > I meant if you had sent the old patch again by accident, because you
>> > said you worked on an old version of the tree, but then posted a patch
>> > that also doesn't use the new MTU function I mentioned.
>> >
>> >   Marcelo
>>
>> I worked on the latest kernel. But, I didn't find the MTU function
>> sctp_mtu_payload().
>
> Which tree are you using?
> [a] git://git.kernel.org/pub/scm/linux/kernel/git/davem/net.git
>    or
> [b] git://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git
> ?
>
> The function isn't on [a] yet, but it is on [b].
>
>   Marcelo

Many thanks for your patience, Marcelo :)

The tree I am working on is:
git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git

Wenwen

^ permalink raw reply

* [PATCH] net/mlx5: fix spelling mistake: "modfiy" -> "modify"
From: Colin King @ 2018-05-03 13:35 UTC (permalink / raw)
  To: Saeed Mahameed, Matan Barak, Leon Romanovsky, David S . Miller,
	netdev, linux-rdma
  Cc: kernel-janitors, linux-kernel

From: Colin Ian King <colin.king@canonical.com>

Trivial fix to spelling mistake in netdev_warn warning message

Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
 drivers/net/ethernet/mellanox/mlx5/core/en_arfs.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_arfs.c b/drivers/net/ethernet/mellanox/mlx5/core/en_arfs.c
index 610d485c4b03..f64b5e78519b 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_arfs.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_arfs.c
@@ -565,7 +565,7 @@ static void arfs_modify_rule_rq(struct mlx5e_priv *priv,
 	err =  mlx5_modify_rule_destination(rule, &dst, NULL);
 	if (err)
 		netdev_warn(priv->netdev,
-			    "Failed to modfiy aRFS rule destination to rq=%d\n", rxq);
+			    "Failed to modify aRFS rule destination to rq=%d\n", rxq);
 }
 
 static void arfs_handle_work(struct work_struct *work)
-- 
2.17.0

^ permalink raw reply related

* Re: [PATCH 2/2] drivers core: multi-threading device shutdown
From: Pavel Tatashin @ 2018-05-03 13:38 UTC (permalink / raw)
  To: tobin
  Cc: Steven Sistare, Daniel Jordan, LKML, jeffrey.t.kirsher,
	intel-wired-lan, netdev, gregkh
In-Reply-To: <20180503055407.GN3791@eros>

On Thu, May 3, 2018 at 1:54 AM Tobin C. Harding <tobin@apporbit.com> wrote:

> This code was a pleasure to read, super clean.


Hi Tobin,

Thank you very much for your review, I will address all of your comments in
the next revision.

BTW, I found  a lock ordering issue in my work that  that I will need to
fix:

In device_shutdown() device_lock() must be taken before
devices_kset->list_lock. Instead I will use device_trylock(), and if that
fails, will drop devices_kset->list_lock and acquiring the device lock
outside, and check that the device is still in the list after taking the
list lock again.

Pavel

^ permalink raw reply

* Re: [PATCH bpf-next 00/12] Move ld_abs/ld_ind to native BPF
From: Daniel Borkmann @ 2018-05-03 13:39 UTC (permalink / raw)
  To: ast; +Cc: netdev
In-Reply-To: <20180503010536.7917-1-daniel@iogearbox.net>

On 05/03/2018 03:05 AM, Daniel Borkmann wrote:
> This set simplifies BPF JITs significantly by moving ld_abs/ld_ind
> to native BPF, for details see individual patches. Main rationale
> is in patch 'implement ld_abs/ld_ind in native bpf'. Thanks!
[...]

Noticed a minor issue, therefore will respin the series in a v2.

^ permalink raw reply

* Re: [PATCH] sctp: fix a potential missing-check bug
From: Marcelo Ricardo Leitner @ 2018-05-03 13:39 UTC (permalink / raw)
  To: Wenwen Wang
  Cc: Kangjie Lu, Vlad Yasevich, Neil Horman, David S. Miller,
	open list:SCTP PROTOCOL, open list:NETWORKING [GENERAL],
	open list
In-Reply-To: <CAAa=b7dvAFnVbFuMuUsCfEUJiKFgbW0UoNSuHEXwmVPdb_pWuA@mail.gmail.com>

On Thu, May 03, 2018 at 08:31:28AM -0500, Wenwen Wang wrote:
> On Thu, May 3, 2018 at 7:46 AM, Marcelo Ricardo Leitner
> <marcelo.leitner@gmail.com> wrote:
> > On Thu, May 03, 2018 at 07:01:51AM -0500, Wenwen Wang wrote:
> >> On Wed, May 2, 2018 at 8:48 PM, Marcelo Ricardo Leitner
> >> <marcelo.leitner@gmail.com> wrote:
> >> > On Wed, May 02, 2018 at 08:27:05PM -0500, Wenwen Wang wrote:
> >> >> On Wed, May 2, 2018 at 8:24 PM, Marcelo Ricardo Leitner
> >> >> <marcelo.leitner@gmail.com> wrote:
> >> >> > On Wed, May 02, 2018 at 08:15:45PM -0500, Wenwen Wang wrote:
> >> >> >> In sctp_setsockopt_maxseg(), the integer 'val' is compared against min_len
> >> >> >> and max_len to check whether it is in the appropriate range. If it is not,
> >> >> >> an error code -EINVAL will be returned. This is enforced by a security
> >> >> >> check. But, this check is only executed when 'val' is not 0. In fact, if
> >> >> >> 'val' is 0, it will be assigned with a new value (if the return value of
> >> >> >> the function sctp_id2assoc() is not 0) in the following execution. However,
> >> >> >> this new value of 'val' is not checked before it is used to assigned to
> >> >> >> asoc->user_frag. That means it is possible that the new value of 'val'
> >> >> >> could be out of the expected range. This can cause security issues
> >> >> >> such as buffer overflows, e.g., the new value of 'val' is used as an index
> >> >> >> to access a buffer.
> >> >> >>
> >> >> >> This patch inserts a check for the new value of 'val' to see if it is in
> >> >> >> the expected range. If it is not, an error code -EINVAL will be returned.
> >> >> >>
> >> >> >> Signed-off-by: Wenwen Wang <wang6495@umn.edu>
> >> >> >> ---
> >> >> >>  net/sctp/socket.c | 22 +++++++++++-----------
> >> >> >>  1 file changed, 11 insertions(+), 11 deletions(-)
> >> >> >
> >> >> > ?
> >> >> > This patch is the same as previous one. git send-email <old file>
> >> >> > maybe?
> >> >> >
> >> >> >   Marcelo
> >> >>
> >> >> Thanks for your suggestion, Marcelo. I can send the old file. But, I
> >> >> have added a line of comment in this patch.
> >> >
> >> > I meant if you had sent the old patch again by accident, because you
> >> > said you worked on an old version of the tree, but then posted a patch
> >> > that also doesn't use the new MTU function I mentioned.
> >> >
> >> >   Marcelo
> >>
> >> I worked on the latest kernel. But, I didn't find the MTU function
> >> sctp_mtu_payload().
> >
> > Which tree are you using?
> > [a] git://git.kernel.org/pub/scm/linux/kernel/git/davem/net.git
> >    or
> > [b] git://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git
> > ?
> >
> > The function isn't on [a] yet, but it is on [b].
> >
> >   Marcelo
>
> Many thanks for your patience, Marcelo :)
>
> The tree I am working on is:
> git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git

Ahh! That explains the discrepancy :)
For networking patches, please refer to
Documentation/networking/netdev-FAQ.txt
It describes what the 2 trees I pointed out are and how they should be
used.
In short, both net and net-next are always newer than the one you're
using for networking subsystem.

Regards,
Marcelo

^ permalink raw reply

* Re: [PATCH] sctp: fix a potential missing-check bug
From: Wenwen Wang @ 2018-05-03 13:43 UTC (permalink / raw)
  To: Marcelo Ricardo Leitner
  Cc: Kangjie Lu, Vlad Yasevich, Neil Horman, David S. Miller,
	open list:SCTP PROTOCOL, open list:NETWORKING [GENERAL],
	open list, Wenwen Wang
In-Reply-To: <20180503133945.GN5105@localhost.localdomain>

On Thu, May 3, 2018 at 8:39 AM, Marcelo Ricardo Leitner
<marcelo.leitner@gmail.com> wrote:
> On Thu, May 03, 2018 at 08:31:28AM -0500, Wenwen Wang wrote:
>> On Thu, May 3, 2018 at 7:46 AM, Marcelo Ricardo Leitner
>> <marcelo.leitner@gmail.com> wrote:
>> > On Thu, May 03, 2018 at 07:01:51AM -0500, Wenwen Wang wrote:
>> >> On Wed, May 2, 2018 at 8:48 PM, Marcelo Ricardo Leitner
>> >> <marcelo.leitner@gmail.com> wrote:
>> >> > On Wed, May 02, 2018 at 08:27:05PM -0500, Wenwen Wang wrote:
>> >> >> On Wed, May 2, 2018 at 8:24 PM, Marcelo Ricardo Leitner
>> >> >> <marcelo.leitner@gmail.com> wrote:
>> >> >> > On Wed, May 02, 2018 at 08:15:45PM -0500, Wenwen Wang wrote:
>> >> >> >> In sctp_setsockopt_maxseg(), the integer 'val' is compared against min_len
>> >> >> >> and max_len to check whether it is in the appropriate range. If it is not,
>> >> >> >> an error code -EINVAL will be returned. This is enforced by a security
>> >> >> >> check. But, this check is only executed when 'val' is not 0. In fact, if
>> >> >> >> 'val' is 0, it will be assigned with a new value (if the return value of
>> >> >> >> the function sctp_id2assoc() is not 0) in the following execution. However,
>> >> >> >> this new value of 'val' is not checked before it is used to assigned to
>> >> >> >> asoc->user_frag. That means it is possible that the new value of 'val'
>> >> >> >> could be out of the expected range. This can cause security issues
>> >> >> >> such as buffer overflows, e.g., the new value of 'val' is used as an index
>> >> >> >> to access a buffer.
>> >> >> >>
>> >> >> >> This patch inserts a check for the new value of 'val' to see if it is in
>> >> >> >> the expected range. If it is not, an error code -EINVAL will be returned.
>> >> >> >>
>> >> >> >> Signed-off-by: Wenwen Wang <wang6495@umn.edu>
>> >> >> >> ---
>> >> >> >>  net/sctp/socket.c | 22 +++++++++++-----------
>> >> >> >>  1 file changed, 11 insertions(+), 11 deletions(-)
>> >> >> >
>> >> >> > ?
>> >> >> > This patch is the same as previous one. git send-email <old file>
>> >> >> > maybe?
>> >> >> >
>> >> >> >   Marcelo
>> >> >>
>> >> >> Thanks for your suggestion, Marcelo. I can send the old file. But, I
>> >> >> have added a line of comment in this patch.
>> >> >
>> >> > I meant if you had sent the old patch again by accident, because you
>> >> > said you worked on an old version of the tree, but then posted a patch
>> >> > that also doesn't use the new MTU function I mentioned.
>> >> >
>> >> >   Marcelo
>> >>
>> >> I worked on the latest kernel. But, I didn't find the MTU function
>> >> sctp_mtu_payload().
>> >
>> > Which tree are you using?
>> > [a] git://git.kernel.org/pub/scm/linux/kernel/git/davem/net.git
>> >    or
>> > [b] git://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git
>> > ?
>> >
>> > The function isn't on [a] yet, but it is on [b].
>> >
>> >   Marcelo
>>
>> Many thanks for your patience, Marcelo :)
>>
>> The tree I am working on is:
>> git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
>
> Ahh! That explains the discrepancy :)
> For networking patches, please refer to
> Documentation/networking/netdev-FAQ.txt
> It describes what the 2 trees I pointed out are and how they should be
> used.
> In short, both net and net-next are always newer than the one you're
> using for networking subsystem.
>
> Regards,
> Marcelo

I see now. Will work on the new networking trees. Thanks!

Wenwen

^ permalink raw reply

* Re: [PATCH 1/2] ixgbe: release lock for the duration of ixgbe_suspend_close()
From: Alexander Duyck @ 2018-05-03 13:46 UTC (permalink / raw)
  To: Pavel Tatashin
  Cc: Steven Sistare, Daniel Jordan, LKML, Jeff Kirsher,
	intel-wired-lan, Netdev, Greg KH
In-Reply-To: <CAGM2rea3XXJ7OqGeAdexyOUb1_7W_84t5HAb_hp7Rb=TkTM86Q@mail.gmail.com>

On Thu, May 3, 2018 at 6:30 AM, Pavel Tatashin
<pasha.tatashin@oracle.com> wrote:
> Hi Alex,

Hi Pavel,

>> I'm not a fan of dropping the mutex while we go through
>> ixgbe_close_suspend. I'm concerned it will result in us having a
>> number of races on shutdown.
>
> I would agree, but ixgbe_close_suspend() is already called without this
> mutex from ixgbe_close(). This path is executed also during machine
> shutdown but when kexec'ed. So, it is either an existing bug or there are
> no races. But, because ixgbe_close() is called from the userland, and a
> little earlier than ixgbe_shutdown() I think this means there are no races.

All callers of ixgbe_close are supposed to already be holding the RTNL
mutex. That is the whole reason why we need to hold it here as the
shutdown path doesn't have the mutex held otherwise. The mutex was
added here because shutdown was racing with the open/close calls.

>
>> If anything, I think we would need to find a replacement for the mutex
>> that can operate at the per-netdev level if you are wanting to
>> parallelize this.
>
> Yes, if lock is necessary, it can be replaced in this place (and added to
> ixgbe_close())  with something scalable.

I wouldn't suggest adding yet another lock for all this. Instead it
might make more sense for us to start looking at breaking up the
locking since most of the networking subsystem uses the rtnl_lock call
it might be time to start looking at doing something like cutting back
on the use of this in cases where all the resources needed are
specific to one device and instead have a per device lock that could
address those kind of cases.

- Alex

^ permalink raw reply

* [PATCH RESEND] SUNRPC: fix include for cmpxchg_relaxed()
From: Mark Rutland @ 2018-05-03 13:50 UTC (permalink / raw)
  To: linux-kernel
  Cc: Mark Rutland, Trond Myklebust, Anna Schumaker, J . Bruce Fields,
	Jeff Layton, David S . Miller, linux-nfs, netdev

Currently net/sunrpc/xprtmultipath.c is the only file outside of arch/
headers and asm-generic/ headers to include <asm/cmpxhcg.h>, apparently
for the use of cmpxchg_relaxed().

However, many architectures do not provide cmpxchg_relaxed() in their
<asm/cmpxhcg.h>, and it is necessary to include <linux/atomic.h> to get
this definition, as noted in Documentation/core-api/atomic_ops.rst:

  If someone wants to use xchg(), cmpxchg() and their variants,
  linux/atomic.h should be included rather than asm/cmpxchg.h, unless
  the code is in arch/* and can take care of itself.

Evidently we're getting the right header this via some transitive
include today, but this isn't something we can/should rely upon,
especially with ongoing rework of the atomic headers for KASAN
instrumentation.

Let's fix the code to include <linux/atomic.h>, avoiding fragility.

Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Cc: Trond Myklebust <trond.myklebust@primarydata.com>
Cc: Anna Schumaker <anna.schumaker@netapp.com>
Cc: J. Bruce Fields <bfields@fieldses.org>
Cc: Jeff Layton <jlayton@poochiereds.net>
Cc: David S. Miller <davem@davemloft.net>
Cc: linux-nfs@vger.kernel.org
Cc: netdev@vger.kernel.org
---
 net/sunrpc/xprtmultipath.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

I sent this about a year ago [1], but got no response. This still applies atop
of v4.17-rc3.

I'm currently trying to implement instrumented atomics for arm64, and it would
be great to have this fixed.

Mark.

[1] https://lkml.kernel.org/r/1489574142-20856-1-git-send-email-mark.rutland@arm.com

diff --git a/net/sunrpc/xprtmultipath.c b/net/sunrpc/xprtmultipath.c
index e2d64c7138c3..d897f41be244 100644
--- a/net/sunrpc/xprtmultipath.c
+++ b/net/sunrpc/xprtmultipath.c
@@ -13,7 +13,7 @@
 #include <linux/rcupdate.h>
 #include <linux/rculist.h>
 #include <linux/slab.h>
-#include <asm/cmpxchg.h>
+#include <linux/atomic.h>
 #include <linux/spinlock.h>
 #include <linux/sunrpc/xprt.h>
 #include <linux/sunrpc/addr.h>
-- 
2.11.0

^ permalink raw reply related

* Re: DSA switch
From: Ran Shalit @ 2018-05-03 13:54 UTC (permalink / raw)
  To: Jiri Pirko; +Cc: Andrew Lunn, netdev
In-Reply-To: <20180503071124.GM19250@nanopsycho>

On Thu, May 3, 2018 at 10:11 AM, Jiri Pirko <jiri@resnulli.us> wrote:
> Thu, May 03, 2018 at 08:50:52AM CEST, ranshalit@gmail.com wrote:
>>On Wed, May 2, 2018 at 11:56 PM, Andrew Lunn <andrew@lunn.ch> wrote:
>>> On Wed, May 02, 2018 at 11:20:05PM +0300, Ran Shalit wrote:
>>>> Hello,
>>>>
>>>> Is it possible to use switch just like external real switch,
>>>> connecting all ports to the same subnet ?
>>>
>>> Yes. Just bridge all ports/interfaces together and put your host IP
>>> address on the bridge.
>>>
>>>         Andrew
>>
>>
>>Hi,
>>
>>I get error on trying to add bridge.
>>I am trying to =understand which configuration is missing probably in my kernel,
>> I ran strace, but not sure , does it point to any missing configuration ?
>>
>>root@dm814x-evm:~# ip link add br0 type bridge
>
> Is the bridge module enabled in the kernel config?
>
>

Hi,

It seems that although the bridge command functions, it takes several
seconds (~6-7 seconds !) from the time it resturns to shell till a
real communication works (ping between 2 PCs connected to switch).
Does it usually takes so much time ?

Thank you,
ranran

^ permalink raw reply

* Re: [RFC v3 4/5] virtio_ring: add event idx support in packed ring
From: Tiwei Bie @ 2018-05-03 13:54 UTC (permalink / raw)
  To: Jason Wang
  Cc: Michael S. Tsirkin, virtualization, linux-kernel, netdev, wexu,
	jfreimann
In-Reply-To: <9f0b4e37-63ff-42f9-f2e6-3747a19a0206@redhat.com>

On Thu, May 03, 2018 at 03:25:29PM +0800, Jason Wang wrote:
> On 2018年05月03日 10:09, Tiwei Bie wrote:
> > > > > So how about we use the straightforward way then?
> > > > You mean we do new += vq->vring_packed.num instead
> > > > of event_idx -= vq->vring_packed.num before calling
> > > > vring_need_event()?
> > > > 
> > > > The problem is that, the second param (new_idx) of
> > > > vring_need_event() will be used for:
> > > > 
> > > > (__u16)(new_idx - event_idx - 1)
> > > > (__u16)(new_idx - old)
> > > > 
> > > > So if we change new, we will need to change old too.
> > > I think that since we have a branch there anyway,
> > > we are better off just special-casing if (wrap_counter != vq->wrap_counter).
> > > Treat is differenty and avoid casts.
> > > 
> > > > And that would be an ugly hack..
> > > > 
> > > > Best regards,
> > > > Tiwei Bie
> > > I consider casts and huge numbers with two's complement
> > > games even uglier.
> > The dependency on two's complement game is introduced
> > since the split ring.
> > 
> > In packed ring, old is calculated via:
> > 
> > old = vq->next_avail_idx - vq->num_added;
> > 
> > In split ring, old is calculated via:
> > 
> > old = vq->avail_idx_shadow - vq->num_added;
> > 
> > In both cases, when vq->num_added is bigger, old will
> > be a big number.
> > 
> > Best regards,
> > Tiwei Bie
> > 
> 
> How about just do something like vhost:
> 
> static u16 vhost_idx_diff(struct vhost_virtqueue *vq, u16 old, u16 new)
> {
>     if (new > old)
>         return new - old;
>     return  (new + vq->num - old);
> }
> 
> static bool vhost_vring_packed_need_event(struct vhost_virtqueue *vq,
>                       __u16 event_off, __u16 new,
>                       __u16 old)
> {
>     return (__u16)(vhost_idx_diff(vq, new, event_off) - 1) <
>            (__u16)vhost_idx_diff(vq, new, old);
> }
> 
> ?

It seems that there is a typo in above code. The second
param of vhost_idx_diff() is `old`, but when calling this
function in vhost_vring_packed_need_event(), `new` is
passed as the second param.

If we assume the second param of vhost_idx_diff() is new
and the third one is old, i.e.:

static u16 vhost_idx_diff(struct vhost_virtqueue *vq, u16 new, u16 old)
{
    if (new > old)
        return new - old;
    return  (new + vq->num - old);
}

I think it's still not right.

Because in virtqueue_enable_cb_delayed(), we may set an
event_off which is bigger than new and both of them have
wrapped. And in this case, although new is smaller than
event_off (i.e. the third param -- old), new shouldn't
add vq->num, and actually we are expecting a very big
idx diff.

Best regards,
Tiwei Bie

^ permalink raw reply

* Re: [PATCH bpf-next v3 00/15] Introducing AF_XDP support
From: Willem de Bruijn @ 2018-05-03 13:55 UTC (permalink / raw)
  To: Björn Töpel
  Cc: Karlsson, Magnus, Alexander Duyck, Alexander Duyck,
	John Fastabend, Alexei Starovoitov, Jesper Dangaard Brouer,
	Daniel Borkmann, Michael S. Tsirkin, Network Development,
	Björn Töpel, michael.lundkvist, Brandeburg, Jesse,
	Singhai, Anjali, Zhang, Qi Z
In-Reply-To: <20180502110136.3738-1-bjorn.topel@gmail.com>

On Wed, May 2, 2018 at 1:01 PM, Björn Töpel <bjorn.topel@gmail.com> wrote:
> From: Björn Töpel <bjorn.topel@intel.com>
>
> This patch set introduces a new address family called AF_XDP that is
> optimized for high performance packet processing

Great patchset, thanks.

> and, in upcoming
> patch sets, zero-copy semantics.

And looking forward to this!

> Thanks: Björn and Magnus
>
> Björn Töpel (7):
>   net: initial AF_XDP skeleton
>   xsk: add user memory registration support sockopt
>   xsk: add Rx queue setup and mmap support
>   xsk: add Rx receive functions and poll support
>   bpf: introduce new bpf AF_XDP map type BPF_MAP_TYPE_XSKMAP
>   xsk: wire up XDP_DRV side of AF_XDP
>   xsk: wire up XDP_SKB side of AF_XDP
>
> Magnus Karlsson (8):
>   xsk: add umem fill queue support and mmap
>   xsk: add support for bind for Rx
>   xsk: add umem completion queue support and mmap
>   xsk: add Tx queue setup and mmap support
>   dev: packet: make packet_direct_xmit a common function
>   xsk: support for Tx
>   xsk: statistics support
>   samples/bpf: sample application and documentation for AF_XDP sockets

For the series

Acked-by: Willem de Bruijn <willemb@google.com>

^ permalink raw reply

* Re: [PATCH net] tcp: restore autocorking
From: Eric Dumazet @ 2018-05-03 13:55 UTC (permalink / raw)
  To: Tariq Toukan
  Cc: David Miller, Tal Gilboa, netdev, Michael Wenig, Eric Dumazet
In-Reply-To: <298c3b30-d3d3-6069-dd46-bcf0ce315126@mellanox.com>

On Thu, May 3, 2018 at 4:06 AM Tariq Toukan <tariqt@mellanox.com> wrote:



> On 03/05/2018 6:25 AM, Eric Dumazet wrote:
> > When adding rb-tree for TCP retransmit queue, we inadvertently broke
> > TCP autocorking.
> >
> > tcp_should_autocork() should really check if the rtx queue is not empty.
> >

> Hi Eric,

> We are glad to see that the issue that Tal investigated and reported [1]
> is now addressed.
> Thanks for doing that!

> Tal, let’s perf test to see the effect of this fix.

> Best,
> Tariq

> [1] https://patchwork.ozlabs.org/cover/822218/

Yes, but you never really gave any  insights of what exact tests you were
running :/

Sometimes the crystal ball is silent, sometimes it gives a hint :)

^ 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