Message ID | dffc711dc88898f8a5f5387d1fa7d14e0faf9ed5.1512625814.git.sean.wang@mediatek.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
> @@ -25,20 +28,37 @@ static struct sk_buff *mtk_tag_xmit(struct sk_buff *skb, > { > struct dsa_port *dp = dsa_slave_to_port(dev); > u8 *mtk_tag; > + bool is_vlan_skb = true; .. > + /* Mark tag attribute on special tag insertion to notify hardware > + * whether that's a combined special tag with 802.1Q header. > + */ > + mtk_tag[0] = is_vlan_skb ? MTK_HDR_XMIT_TAGGED_TPID_8100 : > + MTK_HDR_XMIT_UNTAGGED; > mtk_tag[1] = (1 << dp->index) & MTK_HDR_XMIT_DP_BIT_MASK; > - mtk_tag[2] = 0; > - mtk_tag[3] = 0; > + > + /* Tag control information is kept for 802.1Q */ > + if (!is_vlan_skb) { > + mtk_tag[2] = 0; > + mtk_tag[3] = 0; > + } > > return skb; > } Hi Sean So you can mark a packet for egress. What about ingress? How do you know the VLAN/PORT combination for packets the CPU receives? I would of expected a similar change to mtk_tag_rcv(). Andrew
On Thu, 2017-12-07 at 16:30 +0100, Andrew Lunn wrote: > > @@ -25,20 +28,37 @@ static struct sk_buff *mtk_tag_xmit(struct sk_buff *skb, > > { > > struct dsa_port *dp = dsa_slave_to_port(dev); > > u8 *mtk_tag; > > + bool is_vlan_skb = true; > > .. > > > + /* Mark tag attribute on special tag insertion to notify hardware > > + * whether that's a combined special tag with 802.1Q header. > > + */ > > + mtk_tag[0] = is_vlan_skb ? MTK_HDR_XMIT_TAGGED_TPID_8100 : > > + MTK_HDR_XMIT_UNTAGGED; > > mtk_tag[1] = (1 << dp->index) & MTK_HDR_XMIT_DP_BIT_MASK; > > - mtk_tag[2] = 0; > > - mtk_tag[3] = 0; > > + > > + /* Tag control information is kept for 802.1Q */ > > + if (!is_vlan_skb) { > > + mtk_tag[2] = 0; > > + mtk_tag[3] = 0; > > + } > > > > return skb; > > } > > Hi Sean > > So you can mark a packet for egress. What about ingress? How do you > know the VLAN/PORT combination for packets the CPU receives? I would > of expected a similar change to mtk_tag_rcv(). > > Andrew Hi, Andrew It's unnecessary for extra handling in mtk_tag_rcv() when VLAN tag is present since it is able to put the VLAN tag after the special tag and then follow the existing way to parse. Sean
On Tue, Dec 12, 2017 at 03:21:21PM +0800, Sean Wang wrote: > On Thu, 2017-12-07 at 16:30 +0100, Andrew Lunn wrote: > > > @@ -25,20 +28,37 @@ static struct sk_buff *mtk_tag_xmit(struct sk_buff *skb, > > > { > > > struct dsa_port *dp = dsa_slave_to_port(dev); > > > u8 *mtk_tag; > > > + bool is_vlan_skb = true; > > > > .. > > > > > + /* Mark tag attribute on special tag insertion to notify hardware > > > + * whether that's a combined special tag with 802.1Q header. > > > + */ > > > + mtk_tag[0] = is_vlan_skb ? MTK_HDR_XMIT_TAGGED_TPID_8100 : > > > + MTK_HDR_XMIT_UNTAGGED; > > > mtk_tag[1] = (1 << dp->index) & MTK_HDR_XMIT_DP_BIT_MASK; > > > - mtk_tag[2] = 0; > > > - mtk_tag[3] = 0; > > > + > > > + /* Tag control information is kept for 802.1Q */ > > > + if (!is_vlan_skb) { > > > + mtk_tag[2] = 0; > > > + mtk_tag[3] = 0; > > > + } > > > > > > return skb; > > > } > > > > Hi Sean > > > > So you can mark a packet for egress. What about ingress? How do you > > know the VLAN/PORT combination for packets the CPU receives? I would > > of expected a similar change to mtk_tag_rcv(). > > > > Andrew > > Hi, Andrew > > It's unnecessary for extra handling in mtk_tag_rcv() when VLAN tag is > present since it is able to put the VLAN tag after the special tag and > then follow the existing way to parse. Hi Sean O.K. Please mention this in the commit message. Since it was something i was expecting, it should be documented why it is not needed. Thanks Andrew
On Tue, 2017-12-12 at 09:28 +0100, Andrew Lunn wrote: > On Tue, Dec 12, 2017 at 03:21:21PM +0800, Sean Wang wrote: > > On Thu, 2017-12-07 at 16:30 +0100, Andrew Lunn wrote: > > > > @@ -25,20 +28,37 @@ static struct sk_buff *mtk_tag_xmit(struct sk_buff *skb, > > > > { > > > > struct dsa_port *dp = dsa_slave_to_port(dev); > > > > u8 *mtk_tag; > > > > + bool is_vlan_skb = true; > > > > > > .. > > > > > > > + /* Mark tag attribute on special tag insertion to notify hardware > > > > + * whether that's a combined special tag with 802.1Q header. > > > > + */ > > > > + mtk_tag[0] = is_vlan_skb ? MTK_HDR_XMIT_TAGGED_TPID_8100 : > > > > + MTK_HDR_XMIT_UNTAGGED; > > > > mtk_tag[1] = (1 << dp->index) & MTK_HDR_XMIT_DP_BIT_MASK; > > > > - mtk_tag[2] = 0; > > > > - mtk_tag[3] = 0; > > > > + > > > > + /* Tag control information is kept for 802.1Q */ > > > > + if (!is_vlan_skb) { > > > > + mtk_tag[2] = 0; > > > > + mtk_tag[3] = 0; > > > > + } > > > > > > > > return skb; > > > > } > > > > > > Hi Sean > > > > > > So you can mark a packet for egress. What about ingress? How do you > > > know the VLAN/PORT combination for packets the CPU receives? I would > > > of expected a similar change to mtk_tag_rcv(). > > > > > > Andrew > > > > Hi, Andrew > > > > It's unnecessary for extra handling in mtk_tag_rcv() when VLAN tag is > > present since it is able to put the VLAN tag after the special tag and > > then follow the existing way to parse. > > Hi Sean > > O.K. Please mention this in the commit message. Since it was something > i was expecting, it should be documented why it is not needed. > > Thanks > Andrew Sure. I will add this in the commit message. Sean >
diff --git a/net/dsa/tag_mtk.c b/net/dsa/tag_mtk.c index 8475434..11535bc 100644 --- a/net/dsa/tag_mtk.c +++ b/net/dsa/tag_mtk.c @@ -13,10 +13,13 @@ */ #include <linux/etherdevice.h> +#include <linux/if_vlan.h> #include "dsa_priv.h" #define MTK_HDR_LEN 4 +#define MTK_HDR_XMIT_UNTAGGED 0 +#define MTK_HDR_XMIT_TAGGED_TPID_8100 1 #define MTK_HDR_RECV_SOURCE_PORT_MASK GENMASK(2, 0) #define MTK_HDR_XMIT_DP_BIT_MASK GENMASK(5, 0) @@ -25,20 +28,37 @@ static struct sk_buff *mtk_tag_xmit(struct sk_buff *skb, { struct dsa_port *dp = dsa_slave_to_port(dev); u8 *mtk_tag; + bool is_vlan_skb = true; - if (skb_cow_head(skb, MTK_HDR_LEN) < 0) - return NULL; - - skb_push(skb, MTK_HDR_LEN); + /* Build the special tag after the MAC Source Address. If VLAN header + * is present, it's required that VLAN header and special tag is + * being combined. Only in this way we can allow the switch can parse + * the both special and VLAN tag at the same time and then look up VLAN + * table with VID. + */ + if (!skb_vlan_tagged(skb)) { + if (skb_cow_head(skb, MTK_HDR_LEN) < 0) + return NULL; - memmove(skb->data, skb->data + MTK_HDR_LEN, 2 * ETH_ALEN); + skb_push(skb, MTK_HDR_LEN); + memmove(skb->data, skb->data + MTK_HDR_LEN, 2 * ETH_ALEN); + is_vlan_skb = false; + } - /* Build the tag after the MAC Source Address */ mtk_tag = skb->data + 2 * ETH_ALEN; - mtk_tag[0] = 0; + + /* Mark tag attribute on special tag insertion to notify hardware + * whether that's a combined special tag with 802.1Q header. + */ + mtk_tag[0] = is_vlan_skb ? MTK_HDR_XMIT_TAGGED_TPID_8100 : + MTK_HDR_XMIT_UNTAGGED; mtk_tag[1] = (1 << dp->index) & MTK_HDR_XMIT_DP_BIT_MASK; - mtk_tag[2] = 0; - mtk_tag[3] = 0; + + /* Tag control information is kept for 802.1Q */ + if (!is_vlan_skb) { + mtk_tag[2] = 0; + mtk_tag[3] = 0; + } return skb; }