Previous implementation overwrites PCP value, assuming the default value is
0, instead of 7.
Avoid this by modifying helper function ethsw_port_set_tci() to
ethsw_port_set_pvid() and make it update only the vlan_id of the tci_cfg
struct.
Signed-off-by: Razvan Stefanescu <[email protected]>
---
drivers/staging/fsl-dpaa2/ethsw/dpsw-cmd.h | 13 +++++++++
drivers/staging/fsl-dpaa2/ethsw/dpsw.c | 42 ++++++++++++++++++++++++++++++
drivers/staging/fsl-dpaa2/ethsw/dpsw.h | 6 +++++
drivers/staging/fsl-dpaa2/ethsw/ethsw.c | 37 +++++++++++++-------------
4 files changed, 79 insertions(+), 19 deletions(-)
diff --git a/drivers/staging/fsl-dpaa2/ethsw/dpsw-cmd.h b/drivers/staging/fsl-dpaa2/ethsw/dpsw-cmd.h
index 1c203e6..da744f2 100644
--- a/drivers/staging/fsl-dpaa2/ethsw/dpsw-cmd.h
+++ b/drivers/staging/fsl-dpaa2/ethsw/dpsw-cmd.h
@@ -49,6 +49,8 @@
#define DPSW_CMDID_IF_SET_FLOODING DPSW_CMD_ID(0x047)
#define DPSW_CMDID_IF_SET_BROADCAST DPSW_CMD_ID(0x048)
+#define DPSW_CMDID_IF_GET_TCI DPSW_CMD_ID(0x04A)
+
#define DPSW_CMDID_IF_SET_LINK_CFG DPSW_CMD_ID(0x04C)
#define DPSW_CMDID_VLAN_ADD DPSW_CMD_ID(0x060)
@@ -206,6 +208,17 @@ struct dpsw_cmd_if_set_tci {
__le16 conf;
};
+struct dpsw_cmd_if_get_tci {
+ __le16 if_id;
+};
+
+struct dpsw_rsp_if_get_tci {
+ __le16 pad;
+ __le16 vlan_id;
+ u8 dei;
+ u8 pcp;
+};
+
#define DPSW_STATE_SHIFT 0
#define DPSW_STATE_SIZE 4
diff --git a/drivers/staging/fsl-dpaa2/ethsw/dpsw.c b/drivers/staging/fsl-dpaa2/ethsw/dpsw.c
index 9b9bc60..3ea957c 100644
--- a/drivers/staging/fsl-dpaa2/ethsw/dpsw.c
+++ b/drivers/staging/fsl-dpaa2/ethsw/dpsw.c
@@ -529,6 +529,48 @@ int dpsw_if_set_tci(struct fsl_mc_io *mc_io,
}
/**
+ * dpsw_if_get_tci() - Get default VLAN Tag Control Information (TCI)
+ * @mc_io: Pointer to MC portal's I/O object
+ * @cmd_flags: Command flags; one or more of 'MC_CMD_FLAG_'
+ * @token: Token of DPSW object
+ * @if_id: Interface Identifier
+ * @cfg: Tag Control Information Configuration
+ *
+ * Return: Completion status. '0' on Success; Error code otherwise.
+ */
+int dpsw_if_get_tci(struct fsl_mc_io *mc_io,
+ u32 cmd_flags,
+ u16 token,
+ u16 if_id,
+ struct dpsw_tci_cfg *cfg)
+{
+ struct mc_command cmd = { 0 };
+ struct dpsw_cmd_if_get_tci *cmd_params;
+ struct dpsw_rsp_if_get_tci *rsp_params;
+ int err;
+
+ /* prepare command */
+ cmd.header = mc_encode_cmd_header(DPSW_CMDID_IF_GET_TCI,
+ cmd_flags,
+ token);
+ cmd_params = (struct dpsw_cmd_if_get_tci *)cmd.params;
+ cmd_params->if_id = cpu_to_le16(if_id);
+
+ /* send command to mc*/
+ err = mc_send_command(mc_io, &cmd);
+ if (err)
+ return err;
+
+ /* retrieve response parameters */
+ rsp_params = (struct dpsw_rsp_if_get_tci *)cmd.params;
+ cfg->pcp = rsp_params->pcp;
+ cfg->dei = rsp_params->dei;
+ cfg->vlan_id = le16_to_cpu(rsp_params->vlan_id);
+
+ return 0;
+}
+
+/**
* dpsw_if_set_stp() - Function sets Spanning Tree Protocol (STP) state.
* @mc_io: Pointer to MC portal's I/O object
* @cmd_flags: Command flags; one or more of 'MC_CMD_FLAG_'
diff --git a/drivers/staging/fsl-dpaa2/ethsw/dpsw.h b/drivers/staging/fsl-dpaa2/ethsw/dpsw.h
index 3335add..82f80c40 100644
--- a/drivers/staging/fsl-dpaa2/ethsw/dpsw.h
+++ b/drivers/staging/fsl-dpaa2/ethsw/dpsw.h
@@ -306,6 +306,12 @@ int dpsw_if_set_tci(struct fsl_mc_io *mc_io,
u16 if_id,
const struct dpsw_tci_cfg *cfg);
+int dpsw_if_get_tci(struct fsl_mc_io *mc_io,
+ u32 cmd_flags,
+ u16 token,
+ u16 if_id,
+ struct dpsw_tci_cfg *cfg);
+
/**
* enum dpsw_stp_state - Spanning Tree Protocol (STP) states
* @DPSW_STP_STATE_BLOCKING: Blocking state
diff --git a/drivers/staging/fsl-dpaa2/ethsw/ethsw.c b/drivers/staging/fsl-dpaa2/ethsw/ethsw.c
index c723a04..ab81a6c 100644
--- a/drivers/staging/fsl-dpaa2/ethsw/ethsw.c
+++ b/drivers/staging/fsl-dpaa2/ethsw/ethsw.c
@@ -50,14 +50,23 @@ static int ethsw_add_vlan(struct ethsw_core *ethsw, u16 vid)
return 0;
}
-static int ethsw_port_set_tci(struct ethsw_port_priv *port_priv,
- struct dpsw_tci_cfg *tci_cfg)
+static int ethsw_port_set_pvid(struct ethsw_port_priv *port_priv, u16 pvid)
{
struct ethsw_core *ethsw = port_priv->ethsw_data;
struct net_device *netdev = port_priv->netdev;
+ struct dpsw_tci_cfg tci_cfg = { 0 };
bool is_oper;
int err, ret;
+ err = dpsw_if_get_tci(ethsw->mc_io, 0, ethsw->dpsw_handle,
+ port_priv->idx, &tci_cfg);
+ if (err) {
+ netdev_err(netdev, "dpsw_if_get_tci err %d\n", err);
+ return err;
+ }
+
+ tci_cfg.vlan_id = pvid;
+
/* Interface needs to be down to change PVID */
is_oper = netif_oper_up(netdev);
if (is_oper) {
@@ -71,17 +80,16 @@ static int ethsw_port_set_tci(struct ethsw_port_priv *port_priv,
}
err = dpsw_if_set_tci(ethsw->mc_io, 0, ethsw->dpsw_handle,
- port_priv->idx, tci_cfg);
+ port_priv->idx, &tci_cfg);
if (err) {
netdev_err(netdev, "dpsw_if_set_tci err %d\n", err);
goto set_tci_error;
}
/* Delete previous PVID info and mark the new one */
- if (port_priv->pvid)
- port_priv->vlans[port_priv->pvid] &= ~ETHSW_VLAN_PVID;
- port_priv->vlans[tci_cfg->vlan_id] |= ETHSW_VLAN_PVID;
- port_priv->pvid = tci_cfg->vlan_id;
+ port_priv->vlans[port_priv->pvid] &= ~ETHSW_VLAN_PVID;
+ port_priv->vlans[pvid] |= ETHSW_VLAN_PVID;
+ port_priv->pvid = pvid;
set_tci_error:
if (is_oper) {
@@ -133,13 +141,7 @@ static int ethsw_port_add_vlan(struct ethsw_port_priv *port_priv,
}
if (flags & BRIDGE_VLAN_INFO_PVID) {
- struct dpsw_tci_cfg tci_cfg = {
- .pcp = 0,
- .dei = 0,
- .vlan_id = vid,
- };
-
- err = ethsw_port_set_tci(port_priv, &tci_cfg);
+ err = ethsw_port_set_pvid(port_priv, vid);
if (err)
return err;
}
@@ -819,9 +821,7 @@ static int ethsw_port_del_vlan(struct ethsw_port_priv *port_priv, u16 vid)
return -ENOENT;
if (port_priv->vlans[vid] & ETHSW_VLAN_PVID) {
- struct dpsw_tci_cfg tci_cfg = { 0 };
-
- err = ethsw_port_set_tci(port_priv, &tci_cfg);
+ err = ethsw_port_set_pvid(port_priv, 0);
if (err)
return err;
}
@@ -1254,7 +1254,6 @@ static int ethsw_port_init(struct ethsw_port_priv *port_priv, u16 port)
const char def_mcast[ETH_ALEN] = {0x01, 0x00, 0x5e, 0x00, 0x00, 0x01};
struct net_device *netdev = port_priv->netdev;
struct ethsw_core *ethsw = port_priv->ethsw_data;
- struct dpsw_tci_cfg tci_cfg = {0};
struct dpsw_vlan_if_cfg vcfg;
int err;
@@ -1272,7 +1271,7 @@ static int ethsw_port_init(struct ethsw_port_priv *port_priv, u16 port)
return err;
}
- err = ethsw_port_set_tci(port_priv, &tci_cfg);
+ err = ethsw_port_set_pvid(port_priv, 0);
if (err)
return err;
--
1.9.1
On Tue, Mar 27, 2018 at 08:10:50AM -0500, Razvan Stefanescu wrote:
> Previous implementation overwrites PCP value, assuming the default value is
> 0, instead of 7.
>
> Avoid this by modifying helper function ethsw_port_set_tci() to
> ethsw_port_set_pvid() and make it update only the vlan_id of the tci_cfg
> struct.
Hi Razvan
It is a good idea to explain acronyms, especially for staging, since
there are patches for all sorts of devices, can you cannot expect
everybody to know network specific acronyms.
By PCP you mean Priority Code Point. TCI i have no idea about.
Looking at the code, i think you are changing the flow to become
read/modify/write, instead of just write, which is overwriting the
previously configured Priority Code Point?
Please try to add more details to your change logs, to help us
understand the change.
Andrew
> -----Original Message-----
> From: [email protected] [mailto:linux-kernel-
> [email protected]] On Behalf Of Andrew Lunn
> Sent: Tuesday, March 27, 2018 4:38 PM
> To: Razvan Stefanescu <[email protected]>
> Cc: [email protected]; [email protected]; linux-
> [email protected]; [email protected]; Alexandru Marginean
> <[email protected]>; Ruxandra Ioana Ciocoi Radulescu
> <[email protected]>; Ioana Ciornei <[email protected]>;
> Laurentiu Tudor <[email protected]>; [email protected]
> Subject: Re: [PATCH] staging: fsl-dpaa2/ethsw: Fix TCI values overwrite
>
> On Tue, Mar 27, 2018 at 08:10:50AM -0500, Razvan Stefanescu wrote:
> > Previous implementation overwrites PCP value, assuming the default value
> is
> > 0, instead of 7.
> >
> > Avoid this by modifying helper function ethsw_port_set_tci() to
> > ethsw_port_set_pvid() and make it update only the vlan_id of the tci_cfg
> > struct.
>
> Hi Razvan
>
> It is a good idea to explain acronyms, especially for staging, since
> there are patches for all sorts of devices, can you cannot expect
> everybody to know network specific acronyms.
>
> By PCP you mean Priority Code Point. TCI i have no idea about.
>
> Looking at the code, i think you are changing the flow to become
> read/modify/write, instead of just write, which is overwriting the
> previously configured Priority Code Point?
>
> Please try to add more details to your change logs, to help us
> understand the change.
>
Thank you Andrew. I'll address this in v2.
Best regards,
Razvan Stefanescu
Hi Razvan,
I love your patch! Yet something to improve:
[auto build test ERROR on staging/staging-testing]
[also build test ERROR on next-20180327]
[cannot apply to v4.16-rc7]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Razvan-Stefanescu/staging-fsl-dpaa2-ethsw-Fix-TCI-values-overwrite/20180328-154703
config: i386-allmodconfig (attached as .config)
compiler: gcc-7 (Debian 7.3.0-1) 7.3.0
reproduce:
# save the attached .config to linux build tree
make ARCH=i386
All error/warnings (new ones prefixed by >>):
drivers/staging/fsl-dpaa2/ethsw/dpsw.c: In function 'dpsw_if_get_tci':
>> drivers/staging/fsl-dpaa2/ethsw/dpsw.c:547:9: error: variable 'cmd' has initializer but incomplete type
struct mc_command cmd = { 0 };
^~~~~~~~~~
>> drivers/staging/fsl-dpaa2/ethsw/dpsw.c:547:28: warning: excess elements in struct initializer
struct mc_command cmd = { 0 };
^
drivers/staging/fsl-dpaa2/ethsw/dpsw.c:547:28: note: (near initialization for 'cmd')
>> drivers/staging/fsl-dpaa2/ethsw/dpsw.c:547:20: error: storage size of 'cmd' isn't known
struct mc_command cmd = { 0 };
^~~
drivers/staging/fsl-dpaa2/ethsw/dpsw.c:547:20: warning: unused variable 'cmd' [-Wunused-variable]
vim +/cmd +547 drivers/staging/fsl-dpaa2/ethsw/dpsw.c
530
531 /**
532 * dpsw_if_get_tci() - Get default VLAN Tag Control Information (TCI)
533 * @mc_io: Pointer to MC portal's I/O object
534 * @cmd_flags: Command flags; one or more of 'MC_CMD_FLAG_'
535 * @token: Token of DPSW object
536 * @if_id: Interface Identifier
537 * @cfg: Tag Control Information Configuration
538 *
539 * Return: Completion status. '0' on Success; Error code otherwise.
540 */
541 int dpsw_if_get_tci(struct fsl_mc_io *mc_io,
542 u32 cmd_flags,
543 u16 token,
544 u16 if_id,
545 struct dpsw_tci_cfg *cfg)
546 {
> 547 struct mc_command cmd = { 0 };
548 struct dpsw_cmd_if_get_tci *cmd_params;
549 struct dpsw_rsp_if_get_tci *rsp_params;
550 int err;
551
552 /* prepare command */
553 cmd.header = mc_encode_cmd_header(DPSW_CMDID_IF_GET_TCI,
554 cmd_flags,
555 token);
556 cmd_params = (struct dpsw_cmd_if_get_tci *)cmd.params;
557 cmd_params->if_id = cpu_to_le16(if_id);
558
559 /* send command to mc*/
560 err = mc_send_command(mc_io, &cmd);
561 if (err)
562 return err;
563
564 /* retrieve response parameters */
565 rsp_params = (struct dpsw_rsp_if_get_tci *)cmd.params;
566 cfg->pcp = rsp_params->pcp;
567 cfg->dei = rsp_params->dei;
568 cfg->vlan_id = le16_to_cpu(rsp_params->vlan_id);
569
570 return 0;
571 }
572
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation