From: Vivien Didelot <hidden> Date: 2015-10-07 23:50:55
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
From: Vivien Didelot <hidden> Date: 2015-10-07 23:49:03
dsa_priv.h uses dsa specific structures, as well as the files using it,
so include dsa.h here.
Signed-off-by: Vivien Didelot <redacted>
---
net/dsa/dsa_priv.h | 1 +
1 file changed, 1 insertion(+)
From: Vivien Didelot <hidden> Date: 2015-10-07 23:49:07
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 <redacted>
---
include/linux/netdevice.h | 6 ++++--
include/net/dsa.h | 5 -----
net/dsa/dsa.c | 1 +
3 files changed, 5 insertions(+), 7 deletions(-)
From: Vivien Didelot <hidden> Date: 2015-10-07 23:49:10
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 <redacted>
---
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(-)
@@ -1852,16 +1852,17 @@ int mv88e6xxx_port_fdb_prepare(struct dsa_switch *ds, int port,}intmv88e6xxx_port_fdb_add(structdsa_switch*ds,intport,-constunsignedchar*addr,u16vid)+conststructswitchdev_obj_port_fdb*fdb,+structswitchdev_trans*trans){-intstate=is_multicast_ether_addr(addr)?+intstate=is_multicast_ether_addr(fdb->addr)?GLOBAL_ATU_DATA_STATE_MC_STATIC:GLOBAL_ATU_DATA_STATE_UC_STATIC;structmv88e6xxx_priv_state*ps=ds_to_priv(ds);intret;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);returnret;
@@ -483,7 +483,8 @@ int mv88e6xxx_port_fdb_prepare(struct dsa_switch *ds, int port,conststructswitchdev_obj_port_fdb*fdb,structswitchdev_trans*trans);intmv88e6xxx_port_fdb_add(structdsa_switch*ds,intport,-constunsignedchar*addr,u16vid);+conststructswitchdev_obj_port_fdb*fdb,+structswitchdev_trans*trans);intmv88e6xxx_port_fdb_del(structdsa_switch*ds,intport,constunsignedchar*addr,u16vid);intmv88e6xxx_port_fdb_getnext(structdsa_switch*ds,intport,
@@ -1869,13 +1869,13 @@ int mv88e6xxx_port_fdb_add(struct dsa_switch *ds, int port,}intmv88e6xxx_port_fdb_del(structdsa_switch*ds,intport,-constunsignedchar*addr,u16vid)+conststructswitchdev_obj_port_fdb*fdb){structmv88e6xxx_priv_state*ps=ds_to_priv(ds);intret;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);
@@ -486,7 +486,7 @@ int mv88e6xxx_port_fdb_add(struct dsa_switch *ds, int port,conststructswitchdev_obj_port_fdb*fdb,structswitchdev_trans*trans);intmv88e6xxx_port_fdb_del(structdsa_switch*ds,intport,-constunsignedchar*addr,u16vid);+conststructswitchdev_obj_port_fdb*fdb);intmv88e6xxx_port_fdb_getnext(structdsa_switch*ds,intport,unsignedchar*addr,u16*vid,bool*is_static);intmv88e6xxx_phy_page_read(structdsa_switch*ds,intport,intpage,intreg);
From: Vivien Didelot <hidden> Date: 2015-10-07 23:50:11
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 <redacted>
---
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(-)
@@ -1841,6 +1841,16 @@ static int _mv88e6xxx_port_fdb_load(struct dsa_switch *ds, int port,return_mv88e6xxx_atu_load(ds,&entry);}+intmv88e6xxx_port_fdb_prepare(structdsa_switch*ds,intport,+conststructswitchdev_obj_port_fdb*fdb,+structswitchdev_trans*trans)+{+/* We don't need any dynamic resource from the kernel (yet),+*soskipthepreparephase.+*/+return0;+}+intmv88e6xxx_port_fdb_add(structdsa_switch*ds,intport,constunsignedchar*addr,u16vid){
From: Andrew Lunn <andrew@lunn.ch> Date: 2015-10-08 00:25:48
On Wed, Oct 07, 2015 at 07:48:29PM -0400, Vivien Didelot wrote:
quoted hunk
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 <redacted>
---
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(-)
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
From: Scott Feldman <hidden> Date: 2015-10-08 06:19:49
On Wed, Oct 7, 2015 at 4:48 PM, Vivien Didelot
[off-list ref] 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 <redacted>
From: Scott Feldman <hidden> Date: 2015-10-08 06:20:37
On Wed, Oct 7, 2015 at 4:48 PM, Vivien Didelot
[off-list ref] wrote:
For consistency with the FDB add operation, propagate the
switchdev_obj_port_fdb structure in the DSA drivers.
Signed-off-by: Vivien Didelot <redacted>
From: kbuild test robot <hidden> Date: 2015-10-08 09:05:45
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:
quoted
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 [off-list ref]
:::::: CC: David S. Miller [off-list ref]
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
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:
quoted
quoted
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 [off-list ref]
:::::: CC: David S. Miller [off-list ref]
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
Thu, Oct 08, 2015 at 11:04:48AM CEST, lkp@intel.com wrote:
quoted
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:
quoted
quoted
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
quoted
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 [off-list ref]
:::::: CC: David S. Miller [off-list ref]
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
From: Vivien Didelot <hidden> Date: 2015-10-08 13:02:34
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:
quoted
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 <redacted>
---
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(-)
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
From: Vivien Didelot <hidden> Date: 2015-10-08 13:32:58
Hi David,
On Oct. Thursday 08 (41) 05:28 AM, David Miller wrote:
From: Vivien Didelot <redacted>
Date: Wed, 7 Oct 2015 19:48:25 -0400
quoted
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
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:
quoted
From: Vivien Didelot <redacted>
Date: Wed, 7 Oct 2015 19:48:25 -0400
quoted
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?
From: Vivien Didelot <hidden> Date: 2015-10-08 14:17:33
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:
quoted
Hi David,
On Oct. Thursday 08 (41) 05:28 AM, David Miller wrote:
quoted
From: Vivien Didelot <redacted>
Date: Wed, 7 Oct 2015 19:48:25 -0400
quoted
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?
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?
From: Andrew Lunn <andrew@lunn.ch> Date: 2015-10-08 15:07:49
quoted
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
From: kbuild test robot <hidden> Date: 2015-10-10 23:36:39
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':
quoted
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);
^
quoted
drivers/net/usb/lan78xx.c:850:2: error: implicit declaration of function 'phy_read_status' [-Werror=implicit-function-declaration]
phy_read_status(phydev);
^
quoted
drivers/net/usb/lan78xx.c:852:13: error: dereferencing pointer to incomplete type 'struct phy_device'
if (!phydev->link && dev->link_on) {
^
quoted
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':
quoted
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':
quoted
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':
quoted
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':
quoted
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':
quoted
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':
quoted
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':
quoted
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':
quoted
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':
quoted
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();
^
quoted
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
quoted
drivers/net/usb/lan78xx.c:1461:44: error: 'PHY_MAX_ADDR' undeclared (first use in this function)