[PATCH net-next 0/6] net: dsa: push switchdev prepare phase in FDB ops

[PATCH net-next 0/6] net: dsa: push switchdev prepare phase in FDB ops

[Vivien Didelot]

This patchset pushes the switchdev prepare phase for the FDB add and del
operations down to the DSA drivers. Currently only mv88e6xxx is affected.

Since the dump requires a bit of refactoring in the driver, it'll come in a
future patchset.

The first 3 patches removes the dsa.h include from linux/netdevice.h, which
broke the inclusion of switchdev.h in dsa.h.

The last 3 patches add port_fdb_prepare and change port_fdb_add and
port_fdb_del to use the switchdev FDB object structure.

To be more specific about the include dependency issue, here's a snippet of
what happens currently if you include switchdev.h in dsa.h:

[...]
    include/net/switchdev.h:52:30: error: field ‘ppid’ has incomplete type
       struct netdev_phys_item_id ppid; /* PORT_PARENT_ID */
                                  ^
    include/net/switchdev.h:185:14: warning: ‘struct nlmsghdr’ declared inside parameter list [enabled by default]
           struct nlmsghdr *nlh, u16 flags);
                  ^
    include/net/switchdev.h:195:7: warning: ‘struct ndmsg’ declared inside parameter list [enabled by default]
    include/net/switchdev.h:198:7: warning: ‘struct nlattr’ declared inside parameter list [enabled by default]
           u16 vid);
           ^
    include/net/switchdev.h:201:15: warning: ‘struct netlink_callback’ declared inside parameter list [enabled by default]
            struct net_device *filter_dev, int idx);
                   ^
[...]

Removing the dsa.h include from linux/netdevice.h gets rid of these errors but
then the DSA code complains if you don't include it in dsa_priv.h:

[...]
net/dsa/slave.c: In function ‘dsa_slave_set_mac_address’:
net/dsa/slave.c:178:39: error: dereferencing pointer to incomplete type
  struct net_device *master = p->parent->dst->master_netdev;
                                       ^
In file included from include/linux/list.h:8:0,
                 from net/dsa/slave.c:11:
net/dsa/slave.c: In function ‘dsa_bridge_check_vlan_range’:
net/dsa/slave.c:209:26: error: ‘DSA_MAX_PORTS’ undeclared (first use in this function)
  DECLARE_BITMAP(members, DSA_MAX_PORTS);
                          ^
net/dsa/slave.c:209:26: note: each undeclared identifier is reported only once for each function it appears in
  DECLARE_BITMAP(members, DSA_MAX_PORTS);
                          ^
include/linux/kernel.h:67:30: note: in definition of macro ‘DIV_ROUND_UP’
 #define DIV_ROUND_UP(n,d) (((n) + (d) - 1) / (d))
                              ^
include/linux/types.h:10:21: note: in expansion of macro ‘BITS_TO_LONGS’
  unsigned long name[BITS_TO_LONGS(bits)]
                     ^
net/dsa/slave.c:209:2: note: in expansion of macro ‘DECLARE_BITMAP’
  DECLARE_BITMAP(members, DSA_MAX_PORTS);
  ^
net/dsa/slave.c:1190:7: error: ‘DSA_TAG_PROTO_EDSA’ undeclared (first use in this function)
  case DSA_TAG_PROTO_EDSA:
       ^
net/dsa/slave.c: In function ‘dsa_slave_get_iflink’:
net/dsa/slave.c:64:1: warning: control reaches end of non-void function [-Wreturn-type]
 }
 ^
[...]


Thanks,
-v

Vivien Didelot (6):
  net: dsa: add uses_hw_tag
  net: dsa: include dsa.h in dsa_priv.h
  net: remove dsa.h include from linux/netdevice.h
  net: dsa: add port_fdb_prepare
  net: dsa: push prepare phase in port_fdb_add
  net: dsa: use switchdev obj in port_fdb_del

 drivers/net/dsa/mv88e6171.c |  1 +
 drivers/net/dsa/mv88e6352.c |  1 +
 drivers/net/dsa/mv88e6xxx.c | 23 +++++++++++++++++------
 drivers/net/dsa/mv88e6xxx.h |  8 ++++++--
 include/linux/netdevice.h   |  9 ++++++---
 include/net/dsa.h           | 14 +++++++-------
 net/dsa/dsa.c               |  1 +
 net/dsa/dsa_priv.h          |  1 +
 net/dsa/slave.c             | 11 +++++++----
 9 files changed, 47 insertions(+), 22 deletions(-)

-- 
2.6.0

[PATCH net-next 1/6] net: dsa: add uses_hw_tag

[Vivien Didelot]

Instead of checking that the dsa_switch_tree rcv pointer is not NULL,
add a uses_hw_tag boolean to net_device to explicit whether it uses
hardware inserted tag or not.

Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
---
 include/linux/netdevice.h | 6 ++++--
 include/net/dsa.h         | 5 -----
 net/dsa/dsa.c             | 1 +
 3 files changed, 5 insertions(+), 7 deletions(-)

diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index b337440..73f0510 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -1422,6 +1422,7 @@ enum netdev_priv_flags {
  *	@allmulti:		Counter, enables or disables allmulticast mode
  *
  *	@vlan_info:	VLAN info
+ *	@uses_hw_tag:	Whether the device uses hardware inserted tag or not
  *	@dsa_ptr:	dsa specific data
  *	@tipc_ptr:	TIPC specific data
  *	@atalk_ptr:	AppleTalk link
@@ -1640,6 +1641,7 @@ struct net_device {
 	struct vlan_info __rcu	*vlan_info;
 #endif
 #if IS_ENABLED(CONFIG_NET_DSA)
+	bool uses_hw_tag;
 	struct dsa_switch_tree	*dsa_ptr;
 #endif
 #if IS_ENABLED(CONFIG_TIPC)
@@ -1891,8 +1893,8 @@ void dev_net_set(struct net_device *dev, struct net *net)
 static inline bool netdev_uses_dsa(struct net_device *dev)
 {
 #if IS_ENABLED(CONFIG_NET_DSA)
-	if (dev->dsa_ptr != NULL)
-		return dsa_uses_tagged_protocol(dev->dsa_ptr);
+	if (dev->uses_hw_tag)
+		return true;
 #endif
 	return false;
 }
diff --git a/include/net/dsa.h b/include/net/dsa.h
index b34d812..3e9eb6c 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -333,9 +333,4 @@ static inline void *ds_to_priv(struct dsa_switch *ds)
 {
 	return (void *)(ds + 1);
 }
-
-static inline bool dsa_uses_tagged_protocol(struct dsa_switch_tree *dst)
-{
-	return dst->rcv != NULL;
-}
 #endif
diff --git a/net/dsa/dsa.c b/net/dsa/dsa.c
index aa398bc..51b3815 100644
--- a/net/dsa/dsa.c
+++ b/net/dsa/dsa.c
@@ -834,6 +834,7 @@ static void dsa_setup_dst(struct dsa_switch_tree *dst, struct net_device *dev,
 	 */
 	wmb();
 	dev->dsa_ptr = (void *)dst;
+	dev->uses_hw_tag = dst->tag_protocol != DSA_TAG_PROTO_NONE;
 
 	if (dst->link_poll_needed) {
 		INIT_WORK(&dst->link_poll_work, dsa_link_poll_work);
-- 
2.6.0

[PATCH net-next 2/6] net: dsa: include dsa.h in dsa_priv.h

[Vivien Didelot]

dsa_priv.h uses dsa specific structures, as well as the files using it,
so include dsa.h here.

Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
---
 net/dsa/dsa_priv.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/net/dsa/dsa_priv.h b/net/dsa/dsa_priv.h
index 311796c8..4522f47 100644
--- a/net/dsa/dsa_priv.h
+++ b/net/dsa/dsa_priv.h
@@ -14,6 +14,7 @@
 #include <linux/phy.h>
 #include <linux/netdevice.h>
 #include <linux/netpoll.h>
+#include <net/dsa.h>
 
 struct dsa_device_ops {
 	struct sk_buff *(*xmit)(struct sk_buff *skb, struct net_device *dev);
-- 
2.6.0

[PATCH net-next 3/6] net: remove dsa.h include from linux/netdevice.h

[Vivien Didelot]

Forward declare struct dsa_switch_tree in netdevice.h instead of
including the dsa.h header.

Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
---
 include/linux/netdevice.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 73f0510..d0bcabb 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -41,7 +41,6 @@
 
 #include <linux/ethtool.h>
 #include <net/net_namespace.h>
-#include <net/dsa.h>
 #ifdef CONFIG_DCB
 #include <net/dcbnl.h>
 #endif
@@ -60,6 +59,8 @@ struct wireless_dev;
 /* 802.15.4 specific */
 struct wpan_dev;
 struct mpls_dev;
+/* DSA specific */
+struct dsa_switch_tree;
 
 void netdev_set_default_ethtool_ops(struct net_device *dev,
 				    const struct ethtool_ops *ops);
-- 
2.6.0

Re: [PATCH net-next 3/6] net: remove dsa.h include from linux/netdevice.h

[kbuild test robot]

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

Hi Vivien,

[auto build test ERROR on net-next/master -- if it's inappropriate base, please ignore]

config: arm64-allyesconfig (attached as .config)
reproduce:
        wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=arm64 

All errors (new ones prefixed by >>):

   In file included from drivers/net/ethernet/hisilicon/hns/hnae.c:15:0:
>> drivers/net/ethernet/hisilicon/hns/hnae.h:465:2: error: unknown type name 'phy_interface_t'
     phy_interface_t phy_if;
     ^

vim +/phy_interface_t +465 drivers/net/ethernet/hisilicon/hns/hnae.h

6fe6611f huangdaode 2015-09-17  449  struct hnae_ae_dev {
6fe6611f huangdaode 2015-09-17  450  	struct device cls_dev; /* the class dev */
6fe6611f huangdaode 2015-09-17  451  	struct device *dev; /* the presented dev */
6fe6611f huangdaode 2015-09-17  452  	struct hnae_ae_ops *ops;
6fe6611f huangdaode 2015-09-17  453  	struct list_head node;
6fe6611f huangdaode 2015-09-17  454  	struct module *owner; /* the module who provides this dev */
6fe6611f huangdaode 2015-09-17  455  	int id;
6fe6611f huangdaode 2015-09-17  456  	char name[AE_NAME_SIZE];
6fe6611f huangdaode 2015-09-17  457  	struct list_head handle_list;
6fe6611f huangdaode 2015-09-17  458  	spinlock_t lock; /* lock to protect the handle_list */
6fe6611f huangdaode 2015-09-17  459  };
6fe6611f huangdaode 2015-09-17  460  
6fe6611f huangdaode 2015-09-17  461  struct hnae_handle {
6fe6611f huangdaode 2015-09-17  462  	struct device *owner_dev; /* the device which make use of this handle */
6fe6611f huangdaode 2015-09-17  463  	struct hnae_ae_dev *dev;  /* the device who provides this handle */
6fe6611f huangdaode 2015-09-17  464  	struct device_node *phy_node;
6fe6611f huangdaode 2015-09-17 @465  	phy_interface_t phy_if;
6fe6611f huangdaode 2015-09-17  466  	u32 if_support;
6fe6611f huangdaode 2015-09-17  467  	int q_num;
6fe6611f huangdaode 2015-09-17  468  	int vf_id;
6fe6611f huangdaode 2015-09-17  469  	u32 eport_id;
6fe6611f huangdaode 2015-09-17  470  	enum hnae_port_type port_type;
6fe6611f huangdaode 2015-09-17  471  	struct list_head node;    /* list to hnae_ae_dev->handle_list */
6fe6611f huangdaode 2015-09-17  472  	struct hnae_buf_ops *bops; /* operation for the buffer */
6fe6611f huangdaode 2015-09-17  473  	struct hnae_queue **qs;  /* array base of all queues */

:::::: The code at line 465 was first introduced by commit
:::::: 6fe6611ff275522a4e4c0359e2f46cdd07780d2f net: add Hisilicon Network Subsystem hnae framework support

:::::: TO: huangdaode <huangdaode@hisilicon.com>
:::::: CC: David S. Miller <davem@davemloft.net>

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

[-- Attachment #2: .config.gz --]
[-- Type: application/octet-stream, Size: 45880 bytes --]

Re: [PATCH net-next 3/6] net: remove dsa.h include from linux/netdevice.h

[Jiri Pirko]

Thu, Oct 08, 2015 at 11:04:48AM CEST, lkp@intel.com wrote:
>Hi Vivien,
>
>[auto build test ERROR on net-next/master -- if it's inappropriate base, please ignore]
>
>config: arm64-allyesconfig (attached as .config)
>reproduce:
>        wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
>        chmod +x ~/bin/make.cross
>        # save the attached .config to linux build tree
>        make.cross ARCH=arm64 
>
>All errors (new ones prefixed by >>):
>
>   In file included from drivers/net/ethernet/hisilicon/hns/hnae.c:15:0:
>>> drivers/net/ethernet/hisilicon/hns/hnae.h:465:2: error: unknown type name 'phy_interface_t'
>     phy_interface_t phy_if;
>     ^
>
>vim +/phy_interface_t +465 drivers/net/ethernet/hisilicon/hns/hnae.h


Looks like hnae.c needs to do "#include <linux/phy.h>" directly.
Cc'ing maintainer.


>
>6fe6611f huangdaode 2015-09-17  449  struct hnae_ae_dev {
>6fe6611f huangdaode 2015-09-17  450  	struct device cls_dev; /* the class dev */
>6fe6611f huangdaode 2015-09-17  451  	struct device *dev; /* the presented dev */
>6fe6611f huangdaode 2015-09-17  452  	struct hnae_ae_ops *ops;
>6fe6611f huangdaode 2015-09-17  453  	struct list_head node;
>6fe6611f huangdaode 2015-09-17  454  	struct module *owner; /* the module who provides this dev */
>6fe6611f huangdaode 2015-09-17  455  	int id;
>6fe6611f huangdaode 2015-09-17  456  	char name[AE_NAME_SIZE];
>6fe6611f huangdaode 2015-09-17  457  	struct list_head handle_list;
>6fe6611f huangdaode 2015-09-17  458  	spinlock_t lock; /* lock to protect the handle_list */
>6fe6611f huangdaode 2015-09-17  459  };
>6fe6611f huangdaode 2015-09-17  460  
>6fe6611f huangdaode 2015-09-17  461  struct hnae_handle {
>6fe6611f huangdaode 2015-09-17  462  	struct device *owner_dev; /* the device which make use of this handle */
>6fe6611f huangdaode 2015-09-17  463  	struct hnae_ae_dev *dev;  /* the device who provides this handle */
>6fe6611f huangdaode 2015-09-17  464  	struct device_node *phy_node;
>6fe6611f huangdaode 2015-09-17 @465  	phy_interface_t phy_if;
>6fe6611f huangdaode 2015-09-17  466  	u32 if_support;
>6fe6611f huangdaode 2015-09-17  467  	int q_num;
>6fe6611f huangdaode 2015-09-17  468  	int vf_id;
>6fe6611f huangdaode 2015-09-17  469  	u32 eport_id;
>6fe6611f huangdaode 2015-09-17  470  	enum hnae_port_type port_type;
>6fe6611f huangdaode 2015-09-17  471  	struct list_head node;    /* list to hnae_ae_dev->handle_list */
>6fe6611f huangdaode 2015-09-17  472  	struct hnae_buf_ops *bops; /* operation for the buffer */
>6fe6611f huangdaode 2015-09-17  473  	struct hnae_queue **qs;  /* array base of all queues */
>
>:::::: The code at line 465 was first introduced by commit
>:::::: 6fe6611ff275522a4e4c0359e2f46cdd07780d2f net: add Hisilicon Network Subsystem hnae framework support
>
>:::::: TO: huangdaode <huangdaode@hisilicon.com>
>:::::: CC: David S. Miller <davem@davemloft.net>
>
>---
>0-DAY kernel test infrastructure                Open Source Technology Center
>https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

Re: [PATCH net-next 3/6] net: remove dsa.h include from linux/netdevice.h

[Wei Xu]

On 10/8/2015 10:18 AM, Jiri Pirko wrote:
> Thu, Oct 08, 2015 at 11:04:48AM CEST, lkp@intel.com wrote:
>> Hi Vivien,
>>
>> [auto build test ERROR on net-next/master -- if it's inappropriate base, please ignore]
>>
>> config: arm64-allyesconfig (attached as .config)
>> reproduce:
>>        wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
>>        chmod +x ~/bin/make.cross
>>        # save the attached .config to linux build tree
>>        make.cross ARCH=arm64 
>>
>> All errors (new ones prefixed by >>):
>>
>>   In file included from drivers/net/ethernet/hisilicon/hns/hnae.c:15:0:
>>>> drivers/net/ethernet/hisilicon/hns/hnae.h:465:2: error: unknown type name 'phy_interface_t'
>>     phy_interface_t phy_if;
>>     ^
>>
>> vim +/phy_interface_t +465 drivers/net/ethernet/hisilicon/hns/hnae.h
> 
> 

Hi Jiri,

> Looks like hnae.c needs to do "#include <linux/phy.h>" directly.
> Cc'ing maintainer.
> 

Thanks!
We will send the fix patch soon.

Best Regards,
Wei

> 
>>
>> 6fe6611f huangdaode 2015-09-17  449  struct hnae_ae_dev {
>> 6fe6611f huangdaode 2015-09-17  450  	struct device cls_dev; /* the class dev */
>> 6fe6611f huangdaode 2015-09-17  451  	struct device *dev; /* the presented dev */
>> 6fe6611f huangdaode 2015-09-17  452  	struct hnae_ae_ops *ops;
>> 6fe6611f huangdaode 2015-09-17  453  	struct list_head node;
>> 6fe6611f huangdaode 2015-09-17  454  	struct module *owner; /* the module who provides this dev */
>> 6fe6611f huangdaode 2015-09-17  455  	int id;
>> 6fe6611f huangdaode 2015-09-17  456  	char name[AE_NAME_SIZE];
>> 6fe6611f huangdaode 2015-09-17  457  	struct list_head handle_list;
>> 6fe6611f huangdaode 2015-09-17  458  	spinlock_t lock; /* lock to protect the handle_list */
>> 6fe6611f huangdaode 2015-09-17  459  };
>> 6fe6611f huangdaode 2015-09-17  460  
>> 6fe6611f huangdaode 2015-09-17  461  struct hnae_handle {
>> 6fe6611f huangdaode 2015-09-17  462  	struct device *owner_dev; /* the device which make use of this handle */
>> 6fe6611f huangdaode 2015-09-17  463  	struct hnae_ae_dev *dev;  /* the device who provides this handle */
>> 6fe6611f huangdaode 2015-09-17  464  	struct device_node *phy_node;
>> 6fe6611f huangdaode 2015-09-17 @465  	phy_interface_t phy_if;
>> 6fe6611f huangdaode 2015-09-17  466  	u32 if_support;
>> 6fe6611f huangdaode 2015-09-17  467  	int q_num;
>> 6fe6611f huangdaode 2015-09-17  468  	int vf_id;
>> 6fe6611f huangdaode 2015-09-17  469  	u32 eport_id;
>> 6fe6611f huangdaode 2015-09-17  470  	enum hnae_port_type port_type;
>> 6fe6611f huangdaode 2015-09-17  471  	struct list_head node;    /* list to hnae_ae_dev->handle_list */
>> 6fe6611f huangdaode 2015-09-17  472  	struct hnae_buf_ops *bops; /* operation for the buffer */
>> 6fe6611f huangdaode 2015-09-17  473  	struct hnae_queue **qs;  /* array base of all queues */
>>
>> :::::: The code at line 465 was first introduced by commit
>> :::::: 6fe6611ff275522a4e4c0359e2f46cdd07780d2f net: add Hisilicon Network Subsystem hnae framework support
>>
>> :::::: TO: huangdaode <huangdaode@hisilicon.com>
>> :::::: CC: David S. Miller <davem@davemloft.net>
>>
>> ---
>> 0-DAY kernel test infrastructure                Open Source Technology Center
>> https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
> 
> 
> 
> .
> 

Re: [PATCH net-next 3/6] net: remove dsa.h include from linux/netdevice.h

[Jiri Pirko]

Thu, Oct 08, 2015 at 02:11:11PM CEST, huangdaode@hisilicon.com wrote:
>On 2015/10/8 17:18, Jiri Pirko wrote:
>>Thu, Oct 08, 2015 at 11:04:48AM CEST, lkp@intel.com wrote:
>>>Hi Vivien,
>>>
>>>[auto build test ERROR on net-next/master -- if it's inappropriate base, please ignore]
>>>
>>>config: arm64-allyesconfig (attached as .config)
>>>reproduce:
>>>        wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
>>>        chmod +x ~/bin/make.cross
>>>        # save the attached .config to linux build tree
>>>        make.cross ARCH=arm64
>>>
>>>All errors (new ones prefixed by >>):
>>>
>>>   In file included from drivers/net/ethernet/hisilicon/hns/hnae.c:15:0:
>>>>>drivers/net/ethernet/hisilicon/hns/hnae.h:465:2: error: unknown type name 'phy_interface_t'
>>>     phy_interface_t phy_if;
>>>     ^
>>>
>>>vim +/phy_interface_t +465 drivers/net/ethernet/hisilicon/hns/hnae.h
>>
>>Looks like hnae.c needs to do "#include <linux/phy.h>" directly.
>>Cc'ing maintainer.
>>
>>
>>>6fe6611f huangdaode 2015-09-17  449  struct hnae_ae_dev {
>>>6fe6611f huangdaode 2015-09-17  450  	struct device cls_dev; /* the class dev */
>>>6fe6611f huangdaode 2015-09-17  451  	struct device *dev; /* the presented dev */
>>>6fe6611f huangdaode 2015-09-17  452  	struct hnae_ae_ops *ops;
>>>6fe6611f huangdaode 2015-09-17  453  	struct list_head node;
>>>6fe6611f huangdaode 2015-09-17  454  	struct module *owner; /* the module who provides this dev */
>>>6fe6611f huangdaode 2015-09-17  455  	int id;
>>>6fe6611f huangdaode 2015-09-17  456  	char name[AE_NAME_SIZE];
>>>6fe6611f huangdaode 2015-09-17  457  	struct list_head handle_list;
>>>6fe6611f huangdaode 2015-09-17  458  	spinlock_t lock; /* lock to protect the handle_list */
>>>6fe6611f huangdaode 2015-09-17  459  };
>>>6fe6611f huangdaode 2015-09-17  460
>>>6fe6611f huangdaode 2015-09-17  461  struct hnae_handle {
>>>6fe6611f huangdaode 2015-09-17  462  	struct device *owner_dev; /* the device which make use of this handle */
>>>6fe6611f huangdaode 2015-09-17  463  	struct hnae_ae_dev *dev;  /* the device who provides this handle */
>>>6fe6611f huangdaode 2015-09-17  464  	struct device_node *phy_node;
>>>6fe6611f huangdaode 2015-09-17 @465  	phy_interface_t phy_if;
>>>6fe6611f huangdaode 2015-09-17  466  	u32 if_support;
>>>6fe6611f huangdaode 2015-09-17  467  	int q_num;
>>>6fe6611f huangdaode 2015-09-17  468  	int vf_id;
>>>6fe6611f huangdaode 2015-09-17  469  	u32 eport_id;
>>>6fe6611f huangdaode 2015-09-17  470  	enum hnae_port_type port_type;
>>>6fe6611f huangdaode 2015-09-17  471  	struct list_head node;    /* list to hnae_ae_dev->handle_list */
>>>6fe6611f huangdaode 2015-09-17  472  	struct hnae_buf_ops *bops; /* operation for the buffer */
>>>6fe6611f huangdaode 2015-09-17  473  	struct hnae_queue **qs;  /* array base of all queues */
>>>
>>>:::::: The code at line 465 was first introduced by commit
>>>:::::: 6fe6611ff275522a4e4c0359e2f46cdd07780d2f net: add Hisilicon Network Subsystem hnae framework support
>>>
>>>:::::: TO: huangdaode <huangdaode@hisilicon.com>
>>>:::::: CC: David S. Miller <davem@davemloft.net>
>>>
>>>---
>>>0-DAY kernel test infrastructure                Open Source Technology Center
>>>https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
>>
>>--
>>To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
>>the body of a message to majordomo@vger.kernel.org
>>More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>Please read the FAQ at  http://www.tux.org/lkml/
>>
>>.
>>
>Hi Jiri
>thanks for your information. i have compiled the latest net-next repo using
>your config file, but don't find the error you mentioned.
>the attachment is the build log and the  config file.
>
>also, i used the following command to compile, but still fail to reproduce
>your issue.
>/  daode@Turing-Arch-b:~/work/net-next$ cat build.sh //
>//        export ARCH=arm64 //
>//        export CROSS_COMPILE=aarch64-linux-gnu-//
>//        make allyesconfig//
>//        make -j16 //
>////daode@Turing-Arch-b:~/work/net-next$ /
>
>so could you please help me to reproduce the issue.

The patch is not in tree. That does not change the fact that you should
include linux/phy.h directly. Please send the patch adding that. Thanks!

Re: [PATCH net-next 3/6] net: remove dsa.h include from linux/netdevice.h

[huangdaode]

On 2015/10/8 20:18, Jiri Pirko wrote:
> Thu, Oct 08, 2015 at 02:11:11PM CEST, huangdaode@hisilicon.com wrote:
>> On 2015/10/8 17:18, Jiri Pirko wrote:
>>> Thu, Oct 08, 2015 at 11:04:48AM CEST, lkp@intel.com wrote:
>>>> Hi Vivien,
>>>>
>>>> [auto build test ERROR on net-next/master -- if it's inappropriate base, please ignore]
>>>>
>>>> config: arm64-allyesconfig (attached as .config)
>>>> reproduce:
>>>>         wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
>>>>         chmod +x ~/bin/make.cross
>>>>         # save the attached .config to linux build tree
>>>>         make.cross ARCH=arm64
>>>>
>>>> All errors (new ones prefixed by >>):
>>>>
>>>>    In file included from drivers/net/ethernet/hisilicon/hns/hnae.c:15:0:
>>>>>> drivers/net/ethernet/hisilicon/hns/hnae.h:465:2: error: unknown type name 'phy_interface_t'
>>>>      phy_interface_t phy_if;
>>>>      ^
>>>>
>>>> vim +/phy_interface_t +465 drivers/net/ethernet/hisilicon/hns/hnae.h
>>> Looks like hnae.c needs to do "#include <linux/phy.h>" directly.
>>> Cc'ing maintainer.
>>>
>>>
>>>> 6fe6611f huangdaode 2015-09-17  449  struct hnae_ae_dev {
>>>> 6fe6611f huangdaode 2015-09-17  450  	struct device cls_dev; /* the class dev */
>>>> 6fe6611f huangdaode 2015-09-17  451  	struct device *dev; /* the presented dev */
>>>> 6fe6611f huangdaode 2015-09-17  452  	struct hnae_ae_ops *ops;
>>>> 6fe6611f huangdaode 2015-09-17  453  	struct list_head node;
>>>> 6fe6611f huangdaode 2015-09-17  454  	struct module *owner; /* the module who provides this dev */
>>>> 6fe6611f huangdaode 2015-09-17  455  	int id;
>>>> 6fe6611f huangdaode 2015-09-17  456  	char name[AE_NAME_SIZE];
>>>> 6fe6611f huangdaode 2015-09-17  457  	struct list_head handle_list;
>>>> 6fe6611f huangdaode 2015-09-17  458  	spinlock_t lock; /* lock to protect the handle_list */
>>>> 6fe6611f huangdaode 2015-09-17  459  };
>>>> 6fe6611f huangdaode 2015-09-17  460
>>>> 6fe6611f huangdaode 2015-09-17  461  struct hnae_handle {
>>>> 6fe6611f huangdaode 2015-09-17  462  	struct device *owner_dev; /* the device which make use of this handle */
>>>> 6fe6611f huangdaode 2015-09-17  463  	struct hnae_ae_dev *dev;  /* the device who provides this handle */
>>>> 6fe6611f huangdaode 2015-09-17  464  	struct device_node *phy_node;
>>>> 6fe6611f huangdaode 2015-09-17 @465  	phy_interface_t phy_if;
>>>> 6fe6611f huangdaode 2015-09-17  466  	u32 if_support;
>>>> 6fe6611f huangdaode 2015-09-17  467  	int q_num;
>>>> 6fe6611f huangdaode 2015-09-17  468  	int vf_id;
>>>> 6fe6611f huangdaode 2015-09-17  469  	u32 eport_id;
>>>> 6fe6611f huangdaode 2015-09-17  470  	enum hnae_port_type port_type;
>>>> 6fe6611f huangdaode 2015-09-17  471  	struct list_head node;    /* list to hnae_ae_dev->handle_list */
>>>> 6fe6611f huangdaode 2015-09-17  472  	struct hnae_buf_ops *bops; /* operation for the buffer */
>>>> 6fe6611f huangdaode 2015-09-17  473  	struct hnae_queue **qs;  /* array base of all queues */
>>>>
>>>> :::::: The code at line 465 was first introduced by commit
>>>> :::::: 6fe6611ff275522a4e4c0359e2f46cdd07780d2f net: add Hisilicon Network Subsystem hnae framework support
>>>>
>>>> :::::: TO: huangdaode <huangdaode@hisilicon.com>
>>>> :::::: CC: David S. Miller <davem@davemloft.net>
>>>>
>>>> ---
>>>> 0-DAY kernel test infrastructure                Open Source Technology Center
>>>> https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
>>> the body of a message to majordomo@vger.kernel.org
>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>> Please read the FAQ at  http://www.tux.org/lkml/
>>>
>>> .
>>>
>> Hi Jiri
>> thanks for your information. i have compiled the latest net-next repo using
>> your config file, but don't find the error you mentioned.
>> the attachment is the build log and the  config file.
>>
>> also, i used the following command to compile, but still fail to reproduce
>> your issue.
>> /  daode@Turing-Arch-b:~/work/net-next$ cat build.sh //
>> //        export ARCH=arm64 //
>> //        export CROSS_COMPILE=aarch64-linux-gnu-//
>> //        make allyesconfig//
>> //        make -j16 //
>> ////daode@Turing-Arch-b:~/work/net-next$ /
>>
>> so could you please help me to reproduce the issue.
> The patch is not in tree. That does not change the fact that you should
> include linux/phy.h directly. Please send the patch adding that. Thanks!
>
>
>
> .
>
ok,
thanks!

-- 
Best Regards
Daode Huang

Re: [PATCH net-next 3/6] net: remove dsa.h include from linux/netdevice.h

[kbuild test robot]

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

Hi Vivien,

[auto build test ERROR on net-next/master -- if it's inappropriate base, please ignore]

config: x86_64-randconfig-n0-10110700 (attached as .config)
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All errors (new ones prefixed by >>):

   drivers/net/usb/lan78xx.c: In function 'lan78xx_link_reset':
>> drivers/net/usb/lan78xx.c:841:8: error: implicit declaration of function 'phy_read' [-Werror=implicit-function-declaration]
     ret = phy_read(phydev, LAN88XX_INT_STS);
           ^
>> drivers/net/usb/lan78xx.c:850:2: error: implicit declaration of function 'phy_read_status' [-Werror=implicit-function-declaration]
     phy_read_status(phydev);
     ^
>> drivers/net/usb/lan78xx.c:852:13: error: dereferencing pointer to incomplete type 'struct phy_device'
     if (!phydev->link && dev->link_on) {
                ^
>> drivers/net/usb/lan78xx.c:867:3: error: implicit declaration of function 'phy_ethtool_gset' [-Werror=implicit-function-declaration]
      phy_ethtool_gset(phydev, &ecmd);
      ^
   drivers/net/usb/lan78xx.c: In function 'lan78xx_set_wol':
>> drivers/net/usb/lan78xx.c:1067:2: error: implicit declaration of function 'phy_ethtool_set_wol' [-Werror=implicit-function-declaration]
     phy_ethtool_set_wol(netdev->phydev, wol);
     ^
   drivers/net/usb/lan78xx.c: In function 'lan78xx_get_eee':
>> drivers/net/usb/lan78xx.c:1085:8: error: implicit declaration of function 'phy_ethtool_get_eee' [-Werror=implicit-function-declaration]
     ret = phy_ethtool_get_eee(phydev, edata);
           ^
   drivers/net/usb/lan78xx.c: In function 'lan78xx_set_eee':
>> drivers/net/usb/lan78xx.c:1127:3: error: implicit declaration of function 'phy_ethtool_set_eee' [-Werror=implicit-function-declaration]
      phy_ethtool_set_eee(net->phydev, edata);
      ^
   drivers/net/usb/lan78xx.c: In function 'lan78xx_nway_reset':
>> drivers/net/usb/lan78xx.c:1151:9: error: implicit declaration of function 'phy_start_aneg' [-Werror=implicit-function-declaration]
     return phy_start_aneg(net->phydev);
            ^
   drivers/net/usb/lan78xx.c: In function 'lan78xx_get_mdix_status':
>> drivers/net/usb/lan78xx.c:1183:2: error: implicit declaration of function 'phy_write' [-Werror=implicit-function-declaration]
     phy_write(phydev, LAN88XX_EXT_PAGE_ACCESS, LAN88XX_EXT_PAGE_SPACE_1);
     ^
   drivers/net/usb/lan78xx.c: In function 'lan78xx_set_settings':
>> drivers/net/usb/lan78xx.c:1275:8: error: implicit declaration of function 'phy_ethtool_sset' [-Werror=implicit-function-declaration]
     ret = phy_ethtool_sset(phydev, cmd);
           ^
   drivers/net/usb/lan78xx.c: In function 'lan78xx_ioctl':
>> drivers/net/usb/lan78xx.c:1315:9: error: implicit declaration of function 'phy_mii_ioctl' [-Werror=implicit-function-declaration]
     return phy_mii_ioctl(netdev->phydev, rq, cmd);
            ^
   drivers/net/usb/lan78xx.c: In function 'lan78xx_mdiobus_read':
>> drivers/net/usb/lan78xx.c:1374:31: error: dereferencing pointer to incomplete type 'struct mii_bus'
     struct lan78xx_net *dev = bus->priv;
                                  ^
   drivers/net/usb/lan78xx.c: In function 'lan78xx_mdio_init':
>> drivers/net/usb/lan78xx.c:1447:17: error: implicit declaration of function 'mdiobus_alloc' [-Werror=implicit-function-declaration]
     dev->mdiobus = mdiobus_alloc();
                    ^
   drivers/net/usb/lan78xx.c:1447:15: warning: assignment makes pointer from integer without a cast [-Wint-conversion]
     dev->mdiobus = mdiobus_alloc();
                  ^
>> drivers/net/usb/lan78xx.c:1458:29: error: 'MII_BUS_ID_SIZE' undeclared (first use in this function)
     snprintf(dev->mdiobus->id, MII_BUS_ID_SIZE, "usb-%03d:%03d",
                                ^
   drivers/net/usb/lan78xx.c:1458:29: note: each undeclared identifier is reported only once for each function it appears in
>> drivers/net/usb/lan78xx.c:1461:44: error: 'PHY_MAX_ADDR' undeclared (first use in this function)
     dev->mdiobus->irq = kzalloc(sizeof(int) * PHY_MAX_ADDR, GFP_KERNEL);
                                               ^
>> drivers/net/usb/lan78xx.c:1469:26: error: 'PHY_IGNORE_INTERRUPT' undeclared (first use in this function)
      dev->mdiobus->irq[i] = PHY_IGNORE_INTERRUPT;
                             ^
>> drivers/net/usb/lan78xx.c:1479:8: error: implicit declaration of function 'mdiobus_register' [-Werror=implicit-function-declaration]
     ret = mdiobus_register(dev->mdiobus);
           ^
>> drivers/net/usb/lan78xx.c:1490:2: error: implicit declaration of function 'mdiobus_free' [-Werror=implicit-function-declaration]
     mdiobus_free(dev->mdiobus);
     ^
   drivers/net/usb/lan78xx.c: In function 'lan78xx_remove_mdio':
>> drivers/net/usb/lan78xx.c:1496:2: error: implicit declaration of function 'mdiobus_unregister' [-Werror=implicit-function-declaration]
     mdiobus_unregister(dev->mdiobus);
     ^
   drivers/net/usb/lan78xx.c: In function 'lan78xx_phy_init':
>> drivers/net/usb/lan78xx.c:1511:11: error: implicit declaration of function 'phy_find_first' [-Werror=implicit-function-declaration]
     phydev = phy_find_first(dev->mdiobus);
              ^
   drivers/net/usb/lan78xx.c:1511:9: warning: assignment makes pointer from integer without a cast [-Wint-conversion]
     phydev = phy_find_first(dev->mdiobus);
            ^

vim +/phy_read +841 drivers/net/usb/lan78xx.c

ce85e13ad Woojung.Huh@microchip.com 2015-09-16  835  	struct phy_device *phydev = dev->net->phydev;
55d7de9de Woojung.Huh@microchip.com 2015-07-30  836  	struct ethtool_cmd ecmd = { .cmd = ETHTOOL_GSET };
99c79eceb Geert Uytterhoeven        2015-09-04  837  	int ladv, radv, ret;
55d7de9de Woojung.Huh@microchip.com 2015-07-30  838  	u32 buf;
55d7de9de Woojung.Huh@microchip.com 2015-07-30  839  
55d7de9de Woojung.Huh@microchip.com 2015-07-30  840  	/* clear PHY interrupt status */
bdfba55e0 Woojung.Huh@microchip.com 2015-09-16 @841  	ret = phy_read(phydev, LAN88XX_INT_STS);
55d7de9de Woojung.Huh@microchip.com 2015-07-30  842  	if (unlikely(ret < 0))
55d7de9de Woojung.Huh@microchip.com 2015-07-30  843  		return -EIO;
55d7de9de Woojung.Huh@microchip.com 2015-07-30  844  
55d7de9de Woojung.Huh@microchip.com 2015-07-30  845  	/* clear LAN78xx interrupt status */
55d7de9de Woojung.Huh@microchip.com 2015-07-30  846  	ret = lan78xx_write_reg(dev, INT_STS, INT_STS_PHY_INT_);
55d7de9de Woojung.Huh@microchip.com 2015-07-30  847  	if (unlikely(ret < 0))
55d7de9de Woojung.Huh@microchip.com 2015-07-30  848  		return -EIO;
55d7de9de Woojung.Huh@microchip.com 2015-07-30  849  
ce85e13ad Woojung.Huh@microchip.com 2015-09-16 @850  	phy_read_status(phydev);
ce85e13ad Woojung.Huh@microchip.com 2015-09-16  851  
ce85e13ad Woojung.Huh@microchip.com 2015-09-16 @852  	if (!phydev->link && dev->link_on) {
55d7de9de Woojung.Huh@microchip.com 2015-07-30  853  		dev->link_on = false;
55d7de9de Woojung.Huh@microchip.com 2015-07-30  854  		netif_carrier_off(dev->net);
55d7de9de Woojung.Huh@microchip.com 2015-07-30  855  
55d7de9de Woojung.Huh@microchip.com 2015-07-30  856  		/* reset MAC */
55d7de9de Woojung.Huh@microchip.com 2015-07-30  857  		ret = lan78xx_read_reg(dev, MAC_CR, &buf);
55d7de9de Woojung.Huh@microchip.com 2015-07-30  858  		if (unlikely(ret < 0))
55d7de9de Woojung.Huh@microchip.com 2015-07-30  859  			return -EIO;
55d7de9de Woojung.Huh@microchip.com 2015-07-30  860  		buf |= MAC_CR_RST_;
55d7de9de Woojung.Huh@microchip.com 2015-07-30  861  		ret = lan78xx_write_reg(dev, MAC_CR, buf);
55d7de9de Woojung.Huh@microchip.com 2015-07-30  862  		if (unlikely(ret < 0))
55d7de9de Woojung.Huh@microchip.com 2015-07-30  863  			return -EIO;
ce85e13ad Woojung.Huh@microchip.com 2015-09-16  864  	} else if (phydev->link && !dev->link_on) {
55d7de9de Woojung.Huh@microchip.com 2015-07-30  865  		dev->link_on = true;
55d7de9de Woojung.Huh@microchip.com 2015-07-30  866  
ce85e13ad Woojung.Huh@microchip.com 2015-09-16 @867  		phy_ethtool_gset(phydev, &ecmd);
55d7de9de Woojung.Huh@microchip.com 2015-07-30  868  
bdfba55e0 Woojung.Huh@microchip.com 2015-09-16  869  		ret = phy_read(phydev, LAN88XX_INT_STS);
55d7de9de Woojung.Huh@microchip.com 2015-07-30  870  

:::::: The code at line 841 was first introduced by commit
:::::: bdfba55e0d541a9547d737573ae11db7ed72e2bb lan78xx: Remove phy defines in lan78xx.h and use defines in include/linux/microchipphy.h

:::::: TO: Woojung.Huh@microchip.com <Woojung.Huh@microchip.com>
:::::: CC: David S. Miller <davem@davemloft.net>

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

[-- Attachment #2: .config.gz --]
[-- Type: application/octet-stream, Size: 24162 bytes --]

[PATCH net-next 4/6] net: dsa: add port_fdb_prepare

[Vivien Didelot]

Push the prepare phase for FDB operations down to the DSA drivers, with
a new port_fdb_prepare function. Currently only mv88e6xxx is affected.

Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
---
 drivers/net/dsa/mv88e6171.c |  1 +
 drivers/net/dsa/mv88e6352.c |  1 +
 drivers/net/dsa/mv88e6xxx.c | 10 ++++++++++
 drivers/net/dsa/mv88e6xxx.h |  3 +++
 include/net/dsa.h           |  4 ++++
 net/dsa/slave.c             |  7 +++++--
 6 files changed, 24 insertions(+), 2 deletions(-)

diff --git a/drivers/net/dsa/mv88e6171.c b/drivers/net/dsa/mv88e6171.c
index c95cfab..ca3330a 100644
--- a/drivers/net/dsa/mv88e6171.c
+++ b/drivers/net/dsa/mv88e6171.c
@@ -121,6 +121,7 @@ struct dsa_switch_driver mv88e6171_switch_driver = {
 	.port_vlan_add		= mv88e6xxx_port_vlan_add,
 	.port_vlan_del		= mv88e6xxx_port_vlan_del,
 	.vlan_getnext		= mv88e6xxx_vlan_getnext,
+	.port_fdb_prepare	= mv88e6xxx_port_fdb_prepare,
 	.port_fdb_add		= mv88e6xxx_port_fdb_add,
 	.port_fdb_del		= mv88e6xxx_port_fdb_del,
 	.port_fdb_getnext	= mv88e6xxx_port_fdb_getnext,
diff --git a/drivers/net/dsa/mv88e6352.c b/drivers/net/dsa/mv88e6352.c
index 3736706..078a358 100644
--- a/drivers/net/dsa/mv88e6352.c
+++ b/drivers/net/dsa/mv88e6352.c
@@ -348,6 +348,7 @@ struct dsa_switch_driver mv88e6352_switch_driver = {
 	.port_vlan_add		= mv88e6xxx_port_vlan_add,
 	.port_vlan_del		= mv88e6xxx_port_vlan_del,
 	.vlan_getnext		= mv88e6xxx_vlan_getnext,
+	.port_fdb_prepare	= mv88e6xxx_port_fdb_prepare,
 	.port_fdb_add		= mv88e6xxx_port_fdb_add,
 	.port_fdb_del		= mv88e6xxx_port_fdb_del,
 	.port_fdb_getnext	= mv88e6xxx_port_fdb_getnext,
diff --git a/drivers/net/dsa/mv88e6xxx.c b/drivers/net/dsa/mv88e6xxx.c
index 6053d11..9fbb727 100644
--- a/drivers/net/dsa/mv88e6xxx.c
+++ b/drivers/net/dsa/mv88e6xxx.c
@@ -1841,6 +1841,16 @@ static int _mv88e6xxx_port_fdb_load(struct dsa_switch *ds, int port,
 	return _mv88e6xxx_atu_load(ds, &entry);
 }
 
+int mv88e6xxx_port_fdb_prepare(struct dsa_switch *ds, int port,
+			       const struct switchdev_obj_port_fdb *fdb,
+			       struct switchdev_trans *trans)
+{
+	/* We don't need any dynamic resource from the kernel (yet),
+	 * so skip the prepare phase.
+	 */
+	return 0;
+}
+
 int mv88e6xxx_port_fdb_add(struct dsa_switch *ds, int port,
 			   const unsigned char *addr, u16 vid)
 {
diff --git a/drivers/net/dsa/mv88e6xxx.h b/drivers/net/dsa/mv88e6xxx.h
index 39b261f..4475640 100644
--- a/drivers/net/dsa/mv88e6xxx.h
+++ b/drivers/net/dsa/mv88e6xxx.h
@@ -479,6 +479,9 @@ int mv88e6xxx_port_vlan_add(struct dsa_switch *ds, int port, u16 vid,
 int mv88e6xxx_port_vlan_del(struct dsa_switch *ds, int port, u16 vid);
 int mv88e6xxx_vlan_getnext(struct dsa_switch *ds, u16 *vid,
 			   unsigned long *ports, unsigned long *untagged);
+int mv88e6xxx_port_fdb_prepare(struct dsa_switch *ds, int port,
+			       const struct switchdev_obj_port_fdb *fdb,
+			       struct switchdev_trans *trans);
 int mv88e6xxx_port_fdb_add(struct dsa_switch *ds, int port,
 			   const unsigned char *addr, u16 vid);
 int mv88e6xxx_port_fdb_del(struct dsa_switch *ds, int port,
diff --git a/include/net/dsa.h b/include/net/dsa.h
index 3e9eb6c..3aee8a5 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -19,6 +19,7 @@
 #include <linux/phy.h>
 #include <linux/phy_fixed.h>
 #include <linux/ethtool.h>
+#include <net/switchdev.h>
 
 enum dsa_tag_protocol {
 	DSA_TAG_PROTO_NONE = 0,
@@ -316,6 +317,9 @@ struct dsa_switch_driver {
 	/*
 	 * Forwarding database
 	 */
+	int	(*port_fdb_prepare)(struct dsa_switch *ds, int port,
+				    const struct switchdev_obj_port_fdb *fdb,
+				    struct switchdev_trans *trans);
 	int	(*port_fdb_add)(struct dsa_switch *ds, int port,
 				const unsigned char *addr, u16 vid);
 	int	(*port_fdb_del)(struct dsa_switch *ds, int port,
diff --git a/net/dsa/slave.c b/net/dsa/slave.c
index 4f607bc..48e8c15 100644
--- a/net/dsa/slave.c
+++ b/net/dsa/slave.c
@@ -346,10 +346,13 @@ static int dsa_slave_port_fdb_add(struct net_device *dev,
 {
 	struct dsa_slave_priv *p = netdev_priv(dev);
 	struct dsa_switch *ds = p->parent;
-	int ret = -EOPNOTSUPP;
+	int ret;
+
+	if (!ds->drv->port_fdb_prepare || !ds->drv->port_fdb_add)
+		return -EOPNOTSUPP;
 
 	if (switchdev_trans_ph_prepare(trans))
-		ret = ds->drv->port_fdb_add ? 0 : -EOPNOTSUPP;
+		ret = ds->drv->port_fdb_prepare(ds, p->port, fdb, trans);
 	else
 		ret = ds->drv->port_fdb_add(ds, p->port, fdb->addr, fdb->vid);
 
-- 
2.6.0

Re: [PATCH net-next 4/6] net: dsa: add port_fdb_prepare

[Andrew Lunn]

On Wed, Oct 07, 2015 at 07:48:29PM -0400, Vivien Didelot wrote:
> Push the prepare phase for FDB operations down to the DSA drivers, with
> a new port_fdb_prepare function. Currently only mv88e6xxx is affected.
> 
> Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
> ---
>  drivers/net/dsa/mv88e6171.c |  1 +
>  drivers/net/dsa/mv88e6352.c |  1 +
>  drivers/net/dsa/mv88e6xxx.c | 10 ++++++++++
>  drivers/net/dsa/mv88e6xxx.h |  3 +++
>  include/net/dsa.h           |  4 ++++
>  net/dsa/slave.c             |  7 +++++--
>  6 files changed, 24 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/dsa/mv88e6171.c b/drivers/net/dsa/mv88e6171.c
> index c95cfab..ca3330a 100644
> --- a/drivers/net/dsa/mv88e6171.c
> +++ b/drivers/net/dsa/mv88e6171.c
> @@ -121,6 +121,7 @@ struct dsa_switch_driver mv88e6171_switch_driver = {
>  	.port_vlan_add		= mv88e6xxx_port_vlan_add,
>  	.port_vlan_del		= mv88e6xxx_port_vlan_del,
>  	.vlan_getnext		= mv88e6xxx_vlan_getnext,
> +	.port_fdb_prepare	= mv88e6xxx_port_fdb_prepare,

Hi Vivien

Bike shedding a bit, but i would call this
mv88e6xxx_port_fdb_prepare_add.

>  	.port_fdb_add		= mv88e6xxx_port_fdb_add,
>  	.port_fdb_del		= mv88e6xxx_port_fdb_del,
>  	.port_fdb_getnext	= mv88e6xxx_port_fdb_getnext,

Taking a theoretical example, say mv88e6xxx_port_fdb_getnext needed a
prepare call to allocate memory to put the returned ATU into. What
would you call that?

mv88e6xxx_port_fdb_prepare_add and mv88e6xxx_port_fdb_prepare_getnext
just seems unambiguous and future proof.

     Andrew

Re: [PATCH net-next 4/6] net: dsa: add port_fdb_prepare

[Vivien Didelot]

Hi Andrew,

On Oct. Thursday 08 (41) 02:25 AM, Andrew Lunn wrote:
> On Wed, Oct 07, 2015 at 07:48:29PM -0400, Vivien Didelot wrote:
> > Push the prepare phase for FDB operations down to the DSA drivers, with
> > a new port_fdb_prepare function. Currently only mv88e6xxx is affected.
> > 
> > Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
> > ---
> >  drivers/net/dsa/mv88e6171.c |  1 +
> >  drivers/net/dsa/mv88e6352.c |  1 +
> >  drivers/net/dsa/mv88e6xxx.c | 10 ++++++++++
> >  drivers/net/dsa/mv88e6xxx.h |  3 +++
> >  include/net/dsa.h           |  4 ++++
> >  net/dsa/slave.c             |  7 +++++--
> >  6 files changed, 24 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/net/dsa/mv88e6171.c b/drivers/net/dsa/mv88e6171.c
> > index c95cfab..ca3330a 100644
> > --- a/drivers/net/dsa/mv88e6171.c
> > +++ b/drivers/net/dsa/mv88e6171.c
> > @@ -121,6 +121,7 @@ struct dsa_switch_driver mv88e6171_switch_driver = {
> >  	.port_vlan_add		= mv88e6xxx_port_vlan_add,
> >  	.port_vlan_del		= mv88e6xxx_port_vlan_del,
> >  	.vlan_getnext		= mv88e6xxx_vlan_getnext,
> > +	.port_fdb_prepare	= mv88e6xxx_port_fdb_prepare,
> 
> Hi Vivien
> 
> Bike shedding a bit, but i would call this
> mv88e6xxx_port_fdb_prepare_add.

I think port_fdb_prepare is fine because it is the only step that
actually needs the 2-phase model. del and dump are safe and don't need
pre-check.

> >  	.port_fdb_add		= mv88e6xxx_port_fdb_add,
> >  	.port_fdb_del		= mv88e6xxx_port_fdb_del,
> >  	.port_fdb_getnext	= mv88e6xxx_port_fdb_getnext,
> 
> Taking a theoretical example, say mv88e6xxx_port_fdb_getnext needed a
> prepare call to allocate memory to put the returned ATU into. What
> would you call that?
> 
> mv88e6xxx_port_fdb_prepare_add and mv88e6xxx_port_fdb_prepare_getnext
> just seems unambiguous and future proof.

the switchdev dump operation is called just once, so no preparation is
implied (from the switchdev point of view). It is the responsability of
the driver to call the switchdev dump callback itself.

Thanks,
-v

Re: [PATCH net-next 4/6] net: dsa: add port_fdb_prepare

[Andrew Lunn]

> > Hi Vivien
> > 
> > Bike shedding a bit, but i would call this
> > mv88e6xxx_port_fdb_prepare_add.
> 
> I think port_fdb_prepare is fine because it is the only step that
> actually needs the 2-phase model. del and dump are safe and don't need
> pre-check.

O.K. I don't have a strong opinion, i just think sometime later we
might run into a naming consistency issue. If this does happen, we can
fix it then.

    Andrew

Re: [PATCH net-next 4/6] net: dsa: add port_fdb_prepare

[Scott Feldman]

On Wed, Oct 7, 2015 at 4:48 PM, Vivien Didelot
<vivien.didelot@savoirfairelinux.com> wrote:
> Push the prepare phase for FDB operations down to the DSA drivers, with
> a new port_fdb_prepare function. Currently only mv88e6xxx is affected.
>
> Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>

Reviewed-by: Scott Feldman <sfeldma@gmail.com>

[PATCH net-next 5/6] net: dsa: push prepare phase in port_fdb_add

[Vivien Didelot]

Now that the prepare phase is pushed down to the DSA drivers, propagate
it to the port_fdb_add function.

Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
---
 drivers/net/dsa/mv88e6xxx.c | 7 ++++---
 drivers/net/dsa/mv88e6xxx.h | 3 ++-
 include/net/dsa.h           | 3 ++-
 net/dsa/slave.c             | 2 +-
 4 files changed, 9 insertions(+), 6 deletions(-)

diff --git a/drivers/net/dsa/mv88e6xxx.c b/drivers/net/dsa/mv88e6xxx.c
index 9fbb727..916c98e 100644
--- a/drivers/net/dsa/mv88e6xxx.c
+++ b/drivers/net/dsa/mv88e6xxx.c
@@ -1852,16 +1852,17 @@ int mv88e6xxx_port_fdb_prepare(struct dsa_switch *ds, int port,
 }
 
 int mv88e6xxx_port_fdb_add(struct dsa_switch *ds, int port,
-			   const unsigned char *addr, u16 vid)
+			   const struct switchdev_obj_port_fdb *fdb,
+			   struct switchdev_trans *trans)
 {
-	int state = is_multicast_ether_addr(addr) ?
+	int state = is_multicast_ether_addr(fdb->addr) ?
 		GLOBAL_ATU_DATA_STATE_MC_STATIC :
 		GLOBAL_ATU_DATA_STATE_UC_STATIC;
 	struct mv88e6xxx_priv_state *ps = ds_to_priv(ds);
 	int ret;
 
 	mutex_lock(&ps->smi_mutex);
-	ret = _mv88e6xxx_port_fdb_load(ds, port, addr, vid, state);
+	ret = _mv88e6xxx_port_fdb_load(ds, port, fdb->addr, fdb->vid, state);
 	mutex_unlock(&ps->smi_mutex);
 
 	return ret;
diff --git a/drivers/net/dsa/mv88e6xxx.h b/drivers/net/dsa/mv88e6xxx.h
index 4475640..e688bee 100644
--- a/drivers/net/dsa/mv88e6xxx.h
+++ b/drivers/net/dsa/mv88e6xxx.h
@@ -483,7 +483,8 @@ int mv88e6xxx_port_fdb_prepare(struct dsa_switch *ds, int port,
 			       const struct switchdev_obj_port_fdb *fdb,
 			       struct switchdev_trans *trans);
 int mv88e6xxx_port_fdb_add(struct dsa_switch *ds, int port,
-			   const unsigned char *addr, u16 vid);
+			   const struct switchdev_obj_port_fdb *fdb,
+			   struct switchdev_trans *trans);
 int mv88e6xxx_port_fdb_del(struct dsa_switch *ds, int port,
 			   const unsigned char *addr, u16 vid);
 int mv88e6xxx_port_fdb_getnext(struct dsa_switch *ds, int port,
diff --git a/include/net/dsa.h b/include/net/dsa.h
index 3aee8a5..2d86d31 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -321,7 +321,8 @@ struct dsa_switch_driver {
 				    const struct switchdev_obj_port_fdb *fdb,
 				    struct switchdev_trans *trans);
 	int	(*port_fdb_add)(struct dsa_switch *ds, int port,
-				const unsigned char *addr, u16 vid);
+				const struct switchdev_obj_port_fdb *fdb,
+				struct switchdev_trans *trans);
 	int	(*port_fdb_del)(struct dsa_switch *ds, int port,
 				const unsigned char *addr, u16 vid);
 	int	(*port_fdb_getnext)(struct dsa_switch *ds, int port,
diff --git a/net/dsa/slave.c b/net/dsa/slave.c
index 48e8c15..6f7f27e 100644
--- a/net/dsa/slave.c
+++ b/net/dsa/slave.c
@@ -354,7 +354,7 @@ static int dsa_slave_port_fdb_add(struct net_device *dev,
 	if (switchdev_trans_ph_prepare(trans))
 		ret = ds->drv->port_fdb_prepare(ds, p->port, fdb, trans);
 	else
-		ret = ds->drv->port_fdb_add(ds, p->port, fdb->addr, fdb->vid);
+		ret = ds->drv->port_fdb_add(ds, p->port, fdb, trans);
 
 	return ret;
 }
-- 
2.6.0

Re: [PATCH net-next 5/6] net: dsa: push prepare phase in port_fdb_add

[Scott Feldman]

On Wed, Oct 7, 2015 at 4:48 PM, Vivien Didelot
<vivien.didelot@savoirfairelinux.com> wrote:
> Now that the prepare phase is pushed down to the DSA drivers, propagate
> it to the port_fdb_add function.
>
> Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>

Reviewed-by: Scott Feldman <sfeldma@gmail.com>

[PATCH net-next 6/6] net: dsa: use switchdev obj in port_fdb_del

[Vivien Didelot]

For consistency with the FDB add operation, propagate the
switchdev_obj_port_fdb structure in the DSA drivers.

Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
---
 drivers/net/dsa/mv88e6xxx.c | 4 ++--
 drivers/net/dsa/mv88e6xxx.h | 2 +-
 include/net/dsa.h           | 2 +-
 net/dsa/slave.c             | 2 +-
 4 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/net/dsa/mv88e6xxx.c b/drivers/net/dsa/mv88e6xxx.c
index 916c98e..f73c953 100644
--- a/drivers/net/dsa/mv88e6xxx.c
+++ b/drivers/net/dsa/mv88e6xxx.c
@@ -1869,13 +1869,13 @@ int mv88e6xxx_port_fdb_add(struct dsa_switch *ds, int port,
 }
 
 int mv88e6xxx_port_fdb_del(struct dsa_switch *ds, int port,
-			   const unsigned char *addr, u16 vid)
+			   const struct switchdev_obj_port_fdb *fdb)
 {
 	struct mv88e6xxx_priv_state *ps = ds_to_priv(ds);
 	int ret;
 
 	mutex_lock(&ps->smi_mutex);
-	ret = _mv88e6xxx_port_fdb_load(ds, port, addr, vid,
+	ret = _mv88e6xxx_port_fdb_load(ds, port, fdb->addr, fdb->vid,
 				       GLOBAL_ATU_DATA_STATE_UNUSED);
 	mutex_unlock(&ps->smi_mutex);
 
diff --git a/drivers/net/dsa/mv88e6xxx.h b/drivers/net/dsa/mv88e6xxx.h
index e688bee..1347a73 100644
--- a/drivers/net/dsa/mv88e6xxx.h
+++ b/drivers/net/dsa/mv88e6xxx.h
@@ -486,7 +486,7 @@ int mv88e6xxx_port_fdb_add(struct dsa_switch *ds, int port,
 			   const struct switchdev_obj_port_fdb *fdb,
 			   struct switchdev_trans *trans);
 int mv88e6xxx_port_fdb_del(struct dsa_switch *ds, int port,
-			   const unsigned char *addr, u16 vid);
+			   const struct switchdev_obj_port_fdb *fdb);
 int mv88e6xxx_port_fdb_getnext(struct dsa_switch *ds, int port,
 			       unsigned char *addr, u16 *vid, bool *is_static);
 int mv88e6xxx_phy_page_read(struct dsa_switch *ds, int port, int page, int reg);
diff --git a/include/net/dsa.h b/include/net/dsa.h
index 2d86d31..b802dab 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -324,7 +324,7 @@ struct dsa_switch_driver {
 				const struct switchdev_obj_port_fdb *fdb,
 				struct switchdev_trans *trans);
 	int	(*port_fdb_del)(struct dsa_switch *ds, int port,
-				const unsigned char *addr, u16 vid);
+				const struct switchdev_obj_port_fdb *fdb);
 	int	(*port_fdb_getnext)(struct dsa_switch *ds, int port,
 				    unsigned char *addr, u16 *vid,
 				    bool *is_static);
diff --git a/net/dsa/slave.c b/net/dsa/slave.c
index 6f7f27e..bb2bd3b 100644
--- a/net/dsa/slave.c
+++ b/net/dsa/slave.c
@@ -367,7 +367,7 @@ static int dsa_slave_port_fdb_del(struct net_device *dev,
 	int ret = -EOPNOTSUPP;
 
 	if (ds->drv->port_fdb_del)
-		ret = ds->drv->port_fdb_del(ds, p->port, fdb->addr, fdb->vid);
+		ret = ds->drv->port_fdb_del(ds, p->port, fdb);
 
 	return ret;
 }
-- 
2.6.0

Re: [PATCH net-next 6/6] net: dsa: use switchdev obj in port_fdb_del

[Scott Feldman]

On Wed, Oct 7, 2015 at 4:48 PM, Vivien Didelot
<vivien.didelot@savoirfairelinux.com> wrote:
> For consistency with the FDB add operation, propagate the
> switchdev_obj_port_fdb structure in the DSA drivers.
>
> Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>

Reviewed-by: Scott Feldman <sfeldma@gmail.com>

Re: [PATCH net-next 0/6] net: dsa: push switchdev prepare phase in FDB ops

[David Miller]

From: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
Date: Wed,  7 Oct 2015 19:48:25 -0400

> The first 3 patches removes the dsa.h include from linux/netdevice.h, which
> broke the inclusion of switchdev.h in dsa.h.

I still don't agree with bloating up struct netdevice just to deal with
an include file ordering issue, sorry.

Re: [PATCH net-next 0/6] net: dsa: push switchdev prepare phase in FDB ops

[Vivien Didelot]

Hi David,

On Oct. Thursday 08 (41) 05:28 AM, David Miller wrote:
> From: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
> Date: Wed,  7 Oct 2015 19:48:25 -0400
> 
> > The first 3 patches removes the dsa.h include from linux/netdevice.h, which
> > broke the inclusion of switchdev.h in dsa.h.
> 
> I still don't agree with bloating up struct netdevice just to deal with
> an include file ordering issue, sorry.

Yes, I just saw your reply on the first version. I will resend the
patchset with the forward declarations instead.

But looking at the issue that Jiri and the kbuild bot pointed out
earlier in the thread, we must agree that having the DSA header in
netdevice.h is wrong.

There are 2 points to note here:

* checking a "rcv" member of a DSA-specific structure to anwser the
  question "does this interface uses hardware-inserted tag?" is not
  generic and not robust at all.

* the "dsa_ptr" of net_device is just used to access the dsa_switch_tree
  from DSA packet_type receive functions. There must be another way to
  pass it, maybe from a netdev_priv or the packet_type->af_packet_priv?

Thanks,
-v

Re: [PATCH net-next 0/6] net: dsa: push switchdev prepare phase in FDB ops

[Jiri Pirko]

Thu, Oct 08, 2015 at 03:32:51PM CEST, vivien.didelot@savoirfairelinux.com wrote:
>Hi David,
>
>On Oct. Thursday 08 (41) 05:28 AM, David Miller wrote:
>> From: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
>> Date: Wed,  7 Oct 2015 19:48:25 -0400
>> 
>> > The first 3 patches removes the dsa.h include from linux/netdevice.h, which
>> > broke the inclusion of switchdev.h in dsa.h.
>> 
>> I still don't agree with bloating up struct netdevice just to deal with
>> an include file ordering issue, sorry.
>
>Yes, I just saw your reply on the first version. I will resend the
>patchset with the forward declarations instead.
>
>But looking at the issue that Jiri and the kbuild bot pointed out
>earlier in the thread, we must agree that having the DSA header in
>netdevice.h is wrong.
>
>There are 2 points to note here:
>
>* checking a "rcv" member of a DSA-specific structure to anwser the
>  question "does this interface uses hardware-inserted tag?" is not
>  generic and not robust at all.
>
>* the "dsa_ptr" of net_device is just used to access the dsa_switch_tree
>  from DSA packet_type receive functions. There must be another way to
>  pass it, maybe from a netdev_priv or the packet_type->af_packet_priv?

I sent previously patch for this:
http://patchwork.ozlabs.org/patch/336940/
So now my patch would have another user :)

Vivien, I will refresh the patch and send it to you, the you can
use the priv by dsa and send my patch along with your patchset. How does
that sound?

Re: [PATCH net-next 0/6] net: dsa: push switchdev prepare phase in FDB ops

[Vivien Didelot]

Hi Jiri, David,

On Oct. Thursday 08 (41) 03:47 PM, Jiri Pirko wrote:
> Thu, Oct 08, 2015 at 03:32:51PM CEST, vivien.didelot@savoirfairelinux.com wrote:
> >Hi David,
> >
> >On Oct. Thursday 08 (41) 05:28 AM, David Miller wrote:
> >> From: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
> >> Date: Wed,  7 Oct 2015 19:48:25 -0400
> >> 
> >> > The first 3 patches removes the dsa.h include from linux/netdevice.h, which
> >> > broke the inclusion of switchdev.h in dsa.h.
> >> 
> >> I still don't agree with bloating up struct netdevice just to deal with
> >> an include file ordering issue, sorry.
> >
> >Yes, I just saw your reply on the first version. I will resend the
> >patchset with the forward declarations instead.
> >
> >But looking at the issue that Jiri and the kbuild bot pointed out
> >earlier in the thread, we must agree that having the DSA header in
> >netdevice.h is wrong.
> >
> >There are 2 points to note here:
> >
> >* checking a "rcv" member of a DSA-specific structure to anwser the
> >  question "does this interface uses hardware-inserted tag?" is not
> >  generic and not robust at all.
> >
> >* the "dsa_ptr" of net_device is just used to access the dsa_switch_tree
> >  from DSA packet_type receive functions. There must be another way to
> >  pass it, maybe from a netdev_priv or the packet_type->af_packet_priv?
> 
> I sent previously patch for this:
> http://patchwork.ozlabs.org/patch/336940/
> So now my patch would have another user :)

Your patch makes sense. It will reduce the bloating of net_device that
David is talking about. I would also suspect that other <protocol>_ptr
members of the structure are only used in the context of packet_type.

> Vivien, I will refresh the patch and send it to you, the you can
> use the priv by dsa and send my patch along with your patchset. How does
> that sound?

Sure, sounds good.

Thanks,
-v