From: "hongbo.wang" <[email protected]>
the following command can be supported:
ip link add link swp1 name swp1.100 type vlan protocol 802.1ad id 100
Signed-off-by: hongbo.wang <[email protected]>
---
include/uapi/linux/if_bridge.h | 1 +
net/dsa/slave.c | 9 +++++++--
2 files changed, 8 insertions(+), 2 deletions(-)
diff --git a/include/uapi/linux/if_bridge.h b/include/uapi/linux/if_bridge.h
index caa6914a3e53..ecd960aa65c7 100644
--- a/include/uapi/linux/if_bridge.h
+++ b/include/uapi/linux/if_bridge.h
@@ -132,6 +132,7 @@ enum {
#define BRIDGE_VLAN_INFO_RANGE_END (1<<4) /* VLAN is end of vlan range */
#define BRIDGE_VLAN_INFO_BRENTRY (1<<5) /* Global bridge VLAN entry */
#define BRIDGE_VLAN_INFO_ONLY_OPTS (1<<6) /* Skip create/delete/flags */
+#define BRIDGE_VLAN_INFO_8021AD (1<<7) /* VLAN is 802.1AD protocol */
struct bridge_vlan_info {
__u16 flags;
diff --git a/net/dsa/slave.c b/net/dsa/slave.c
index 4c7f086a047b..376d7ac5f1e5 100644
--- a/net/dsa/slave.c
+++ b/net/dsa/slave.c
@@ -1232,6 +1232,7 @@ static int dsa_slave_get_ts_info(struct net_device *dev,
static int dsa_slave_vlan_rx_add_vid(struct net_device *dev, __be16 proto,
u16 vid)
{
+ u16 flags = 0;
struct dsa_port *dp = dsa_slave_to_port(dev);
struct bridge_vlan_info info;
int ret;
@@ -1252,7 +1253,10 @@ static int dsa_slave_vlan_rx_add_vid(struct net_device *dev, __be16 proto,
return -EBUSY;
}
- ret = dsa_port_vid_add(dp, vid, 0);
+ if (ntohs(proto) == ETH_P_8021AD)
+ flags |= BRIDGE_VLAN_INFO_8021AD;
+
+ ret = dsa_port_vid_add(dp, vid, flags);
if (ret)
return ret;
@@ -1744,7 +1748,8 @@ int dsa_slave_create(struct dsa_port *port)
slave_dev->features = master->vlan_features | NETIF_F_HW_TC;
if (ds->ops->port_vlan_add && ds->ops->port_vlan_del)
- slave_dev->features |= NETIF_F_HW_VLAN_CTAG_FILTER;
+ slave_dev->features |= NETIF_F_HW_VLAN_CTAG_FILTER |
+ NETIF_F_HW_VLAN_STAG_FILTER;
slave_dev->hw_features |= NETIF_F_HW_TC;
slave_dev->features |= NETIF_F_LLTX;
slave_dev->ethtool_ops = &dsa_slave_ethtool_ops;
--
2.17.1
From: "hongbo.wang" <[email protected]>
the following command will be supported:
Add VLAN:
ip link add link swp1 name swp1.100 type vlan protocol 802.1ad id 100
Delete VLAN:
ip link del link swp1 name swp1.100
Signed-off-by: hongbo.wang <[email protected]>
---
net/dsa/dsa_priv.h | 2 +-
net/dsa/port.c | 3 ++-
net/dsa/slave.c | 6 +++++-
net/dsa/tag_8021q.c | 2 +-
4 files changed, 9 insertions(+), 4 deletions(-)
diff --git a/net/dsa/dsa_priv.h b/net/dsa/dsa_priv.h
index adecf73bd608..5cd804c1d7e3 100644
--- a/net/dsa/dsa_priv.h
+++ b/net/dsa/dsa_priv.h
@@ -165,7 +165,7 @@ int dsa_port_vlan_add(struct dsa_port *dp,
int dsa_port_vlan_del(struct dsa_port *dp,
const struct switchdev_obj_port_vlan *vlan);
int dsa_port_vid_add(struct dsa_port *dp, u16 vid, u16 flags);
-int dsa_port_vid_del(struct dsa_port *dp, u16 vid);
+int dsa_port_vid_del(struct dsa_port *dp, u16 vid, u16 flags);
int dsa_port_link_register_of(struct dsa_port *dp);
void dsa_port_link_unregister_of(struct dsa_port *dp);
extern const struct phylink_mac_ops dsa_port_phylink_mac_ops;
diff --git a/net/dsa/port.c b/net/dsa/port.c
index e23ece229c7e..8a8ecb91a030 100644
--- a/net/dsa/port.c
+++ b/net/dsa/port.c
@@ -454,10 +454,11 @@ int dsa_port_vid_add(struct dsa_port *dp, u16 vid, u16 flags)
}
EXPORT_SYMBOL(dsa_port_vid_add);
-int dsa_port_vid_del(struct dsa_port *dp, u16 vid)
+int dsa_port_vid_del(struct dsa_port *dp, u16 vid, u16 flags)
{
struct switchdev_obj_port_vlan vlan = {
.obj.id = SWITCHDEV_OBJ_ID_PORT_VLAN,
+ .flags = flags,
.vid_begin = vid,
.vid_end = vid,
};
diff --git a/net/dsa/slave.c b/net/dsa/slave.c
index 376d7ac5f1e5..14784a6718a9 100644
--- a/net/dsa/slave.c
+++ b/net/dsa/slave.c
@@ -1270,6 +1270,7 @@ static int dsa_slave_vlan_rx_add_vid(struct net_device *dev, __be16 proto,
static int dsa_slave_vlan_rx_kill_vid(struct net_device *dev, __be16 proto,
u16 vid)
{
+ u16 flags = 0;
struct dsa_port *dp = dsa_slave_to_port(dev);
struct bridge_vlan_info info;
int ret;
@@ -1290,10 +1291,13 @@ static int dsa_slave_vlan_rx_kill_vid(struct net_device *dev, __be16 proto,
return -EBUSY;
}
+ if (ntohs(proto) == ETH_P_8021AD)
+ flags |= BRIDGE_VLAN_INFO_8021AD;
+
/* Do not deprogram the CPU port as it may be shared with other user
* ports which can be members of this VLAN as well.
*/
- return dsa_port_vid_del(dp, vid);
+ return dsa_port_vid_del(dp, vid, flags);
}
struct dsa_hw_port {
diff --git a/net/dsa/tag_8021q.c b/net/dsa/tag_8021q.c
index 780b2a15ac9b..87b732c5cccf 100644
--- a/net/dsa/tag_8021q.c
+++ b/net/dsa/tag_8021q.c
@@ -154,7 +154,7 @@ static int dsa_8021q_vid_apply(struct dsa_switch *ds, int port, u16 vid,
if (enabled)
return dsa_port_vid_add(dp, vid, flags);
- return dsa_port_vid_del(dp, vid);
+ return dsa_port_vid_del(dp, vid, flags);
}
/* RX VLAN tagging (left) and TX VLAN tagging (right) setup shown for a single
--
2.17.1
On 20/07/2020 13:41, [email protected] wrote:
> From: "hongbo.wang" <[email protected]>
>
> the following command can be supported:
> ip link add link swp1 name swp1.100 type vlan protocol 802.1ad id 100
>
> Signed-off-by: hongbo.wang <[email protected]>
> ---
> include/uapi/linux/if_bridge.h | 1 +
> net/dsa/slave.c | 9 +++++++--
> 2 files changed, 8 insertions(+), 2 deletions(-)
>
This is not bridge related at all, please leave its flags out of it.
Nacked-by: Nikolay Aleksandrov <[email protected]>
> diff --git a/include/uapi/linux/if_bridge.h b/include/uapi/linux/if_bridge.h
> index caa6914a3e53..ecd960aa65c7 100644
> --- a/include/uapi/linux/if_bridge.h
> +++ b/include/uapi/linux/if_bridge.h
> @@ -132,6 +132,7 @@ enum {
> #define BRIDGE_VLAN_INFO_RANGE_END (1<<4) /* VLAN is end of vlan range */
> #define BRIDGE_VLAN_INFO_BRENTRY (1<<5) /* Global bridge VLAN entry */
> #define BRIDGE_VLAN_INFO_ONLY_OPTS (1<<6) /* Skip create/delete/flags */
> +#define BRIDGE_VLAN_INFO_8021AD (1<<7) /* VLAN is 802.1AD protocol */
>
> struct bridge_vlan_info {
> __u16 flags;
> diff --git a/net/dsa/slave.c b/net/dsa/slave.c
> index 4c7f086a047b..376d7ac5f1e5 100644
> --- a/net/dsa/slave.c
> +++ b/net/dsa/slave.c
> @@ -1232,6 +1232,7 @@ static int dsa_slave_get_ts_info(struct net_device *dev,
> static int dsa_slave_vlan_rx_add_vid(struct net_device *dev, __be16 proto,
> u16 vid)
> {
> + u16 flags = 0;
> struct dsa_port *dp = dsa_slave_to_port(dev);
> struct bridge_vlan_info info;
> int ret;
> @@ -1252,7 +1253,10 @@ static int dsa_slave_vlan_rx_add_vid(struct net_device *dev, __be16 proto,
> return -EBUSY;
> }
>
> - ret = dsa_port_vid_add(dp, vid, 0);
> + if (ntohs(proto) == ETH_P_8021AD)
> + flags |= BRIDGE_VLAN_INFO_8021AD;
> +
> + ret = dsa_port_vid_add(dp, vid, flags);
> if (ret)
> return ret;
>
> @@ -1744,7 +1748,8 @@ int dsa_slave_create(struct dsa_port *port)
>
> slave_dev->features = master->vlan_features | NETIF_F_HW_TC;
> if (ds->ops->port_vlan_add && ds->ops->port_vlan_del)
> - slave_dev->features |= NETIF_F_HW_VLAN_CTAG_FILTER;
> + slave_dev->features |= NETIF_F_HW_VLAN_CTAG_FILTER |
> + NETIF_F_HW_VLAN_STAG_FILTER;
> slave_dev->hw_features |= NETIF_F_HW_TC;
> slave_dev->features |= NETIF_F_LLTX;
> slave_dev->ethtool_ops = &dsa_slave_ethtool_ops;
>
Hi Nikolay,
Thanks for your comments.
The original intention is that I want to run a command to set single port into QinQ mode,
the related commands are:
ip link set br0 type bridge vlan_protocol 802.1ad // this command will set all ports under the bridge br0
ip link add link swp1 name swp1.100 type vlan protocol 802.1ad id 100 // this command can set single port for vlan
I trace the related code of these two commands, find the same issue that dsa_slave_vlan_rx_add_vid didn't pass the parameter "proto" to next port level, so I create this patch.
I understand your concern, If don't use the flags for bridge, another way is that add new item "u16 proto" in struct switchdev_obj_port_vlan, the slave port can get proto from that, like that:
struct switchdev_obj_port_vlan {
struct switchdev_obj obj;
u16 flags;
u16 vid_begin;
u16 vid_end;
+ u16 proto;
};
The related modification has:
int dsa_port_vid_add(struct dsa_port *dp, u16 vid, u16 flags, u16 proto); // add parameter proto
int dsa_port_vid_del(struct dsa_port *dp, u16 vid, u16 proto); // add parameter proto
Any comments?
Thanks
-----Original Message-----
From: Nikolay Aleksandrov <[email protected]>
Sent: 2020年7月20日 18:45
To: Hongbo Wang <[email protected]>; Xiaoliang Yang <[email protected]>; [email protected]; Po Liu <[email protected]>; Claudiu Manoil <[email protected]>; Alexandru Marginean <[email protected]>; Vladimir Oltean <[email protected]>; Leo Li <[email protected]>; Mingkai Hu <[email protected]>; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]
Subject: [EXT] Re: [PATCH 1/2] net: dsa: Add flag for 802.1AD when adding VLAN for dsa switch and port
Caution: EXT Email
On 20/07/2020 13:41, [email protected] wrote:
> From: "hongbo.wang" <[email protected]>
>
> the following command can be supported:
> ip link add link swp1 name swp1.100 type vlan protocol 802.1ad id 100
>
> Signed-off-by: hongbo.wang <[email protected]>
> ---
> include/uapi/linux/if_bridge.h | 1 +
> net/dsa/slave.c | 9 +++++++--
> 2 files changed, 8 insertions(+), 2 deletions(-)
>
This is not bridge related at all, please leave its flags out of it.
Nacked-by: Nikolay Aleksandrov <[email protected]>
> diff --git a/include/uapi/linux/if_bridge.h
> b/include/uapi/linux/if_bridge.h index caa6914a3e53..ecd960aa65c7
> 100644
> --- a/include/uapi/linux/if_bridge.h
> +++ b/include/uapi/linux/if_bridge.h
> @@ -132,6 +132,7 @@ enum {
> #define BRIDGE_VLAN_INFO_RANGE_END (1<<4) /* VLAN is end of vlan range */
> #define BRIDGE_VLAN_INFO_BRENTRY (1<<5) /* Global bridge VLAN entry */
> #define BRIDGE_VLAN_INFO_ONLY_OPTS (1<<6) /* Skip create/delete/flags */
> +#define BRIDGE_VLAN_INFO_8021AD (1<<7) /* VLAN is 802.1AD protocol */
>
> struct bridge_vlan_info {
> __u16 flags;
> diff --git a/net/dsa/slave.c b/net/dsa/slave.c index
> 4c7f086a047b..376d7ac5f1e5 100644
> --- a/net/dsa/slave.c
> +++ b/net/dsa/slave.c
> @@ -1232,6 +1232,7 @@ static int dsa_slave_get_ts_info(struct
> net_device *dev, static int dsa_slave_vlan_rx_add_vid(struct net_device *dev, __be16 proto,
> u16 vid) {
> + u16 flags = 0;
> struct dsa_port *dp = dsa_slave_to_port(dev);
> struct bridge_vlan_info info;
> int ret;
> @@ -1252,7 +1253,10 @@ static int dsa_slave_vlan_rx_add_vid(struct net_device *dev, __be16 proto,
> return -EBUSY;
> }
>
> - ret = dsa_port_vid_add(dp, vid, 0);
> + if (ntohs(proto) == ETH_P_8021AD)
> + flags |= BRIDGE_VLAN_INFO_8021AD;
> +
> + ret = dsa_port_vid_add(dp, vid, flags);
> if (ret)
> return ret;
>
> @@ -1744,7 +1748,8 @@ int dsa_slave_create(struct dsa_port *port)
>
> slave_dev->features = master->vlan_features | NETIF_F_HW_TC;
> if (ds->ops->port_vlan_add && ds->ops->port_vlan_del)
> - slave_dev->features |= NETIF_F_HW_VLAN_CTAG_FILTER;
> + slave_dev->features |= NETIF_F_HW_VLAN_CTAG_FILTER |
> + NETIF_F_HW_VLAN_STAG_FILTER;
> slave_dev->hw_features |= NETIF_F_HW_TC;
> slave_dev->features |= NETIF_F_LLTX;
> slave_dev->ethtool_ops = &dsa_slave_ethtool_ops;
>
On 7/20/2020 3:41 AM, [email protected] wrote:
> From: "hongbo.wang" <[email protected]>
>
> the following command can be supported:
> ip link add link swp1 name swp1.100 type vlan protocol 802.1ad id 100
You should probably include the switch driver that is going to be
benefiting from doing these changes in the patch series, provide a cover
letter when sending more than one patch, and also combine both the add
and delete parts.
Since you already have visibility into proto, it may not be necessary at
all for now to define a BRIDGE_VLAN_INFO_8021AD bit in order to pass
that flag down to DSA for programming the VLAN, just pass proto to
dsa_port_vid_add() or a boolean flag which indicates whether this is the
customer or service tag that you are trying to program?
--
Florian
Thanks for your suggestion!
I will change the code.
thanks.
-----Original Message-----
From: Florian Fainelli <[email protected]>
Sent: 2020年7月21日 12:57
To: Hongbo Wang <[email protected]>; Xiaoliang Yang <[email protected]>; [email protected]; Po Liu <[email protected]>; Claudiu Manoil <[email protected]>; Alexandru Marginean <[email protected]>; Vladimir Oltean <[email protected]>; Leo Li <[email protected]>; Mingkai Hu <[email protected]>; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]
Subject: [EXT] Re: [PATCH 1/2] net: dsa: Add flag for 802.1AD when adding VLAN for dsa switch and port
Caution: EXT Email
On 7/20/2020 3:41 AM, [email protected] wrote:
> From: "hongbo.wang" <[email protected]>
>
> the following command can be supported:
> ip link add link swp1 name swp1.100 type vlan protocol 802.1ad id 100
You should probably include the switch driver that is going to be benefiting from doing these changes in the patch series, provide a cover letter when sending more than one patch, and also combine both the add and delete parts.
Since you already have visibility into proto, it may not be necessary at all for now to define a BRIDGE_VLAN_INFO_8021AD bit in order to pass that flag down to DSA for programming the VLAN, just pass proto to
dsa_port_vid_add() or a boolean flag which indicates whether this is the customer or service tag that you are trying to program?
--
Florian