Messages in this thread | | | Date | Mon, 26 Jun 2023 13:37:41 +0300 | From | Dan Carpenter <> | Subject | Re: [PATCH net-next v2 4/7] net: dsa: vsc73xx: Add dsa tagging based on 8021q |
| |
On Sun, Jun 25, 2023 at 02:42:27PM +0200, Simon Horman wrote: > + Dan Carpenter > > On Sun, Jun 25, 2023 at 01:53:39PM +0200, Pawel Dembicki wrote: > > This patch is simple implementation of 8021q tagging in vsc73xx driver. > > At this moment devices with DSA_TAG_PROTO_NONE are useless. VSC73XX > > family doesn't provide any tag support for external ethernet ports. > > > > The only way is vlan-based tagging. It require constant hardware vlan > > filtering. VSC73XX family support provider bridging but QinQ only without > > fully implemented 802.1AD. It allow only doubled 0x8100 TPID. > > > > In simple port mode QinQ is enabled to preserve forwarding vlan tagged > > frames. > > > > Tag driver introduce most simple funcionality required for proper taging > > support. > > > > Reviewed-by: Linus Walleij <linus.walleij@linaro.org> > > Signed-off-by: Pawel Dembicki <paweldembicki@gmail.com> > > ... > > > +static void vsc73xx_vlan_rcv(struct sk_buff *skb, int *source_port, > > + int *switch_id, int *vbid, u16 *vid) > > +{ > > + if (vid_is_dsa_8021q(skb_vlan_tag_get(skb) & VLAN_VID_MASK)) > > + return dsa_8021q_rcv(skb, source_port, switch_id, vbid); > > + > > + /* Try our best with imprecise RX */ > > + *vid = skb_vlan_tag_get(skb) & VLAN_VID_MASK; > > +} > > + > > +static struct sk_buff *vsc73xx_rcv(struct sk_buff *skb, > > + struct net_device *netdev) > > +{ > > + int src_port = -1, switch_id = -1, vbid = -1; > > + u16 vid; > > + > > + if (skb_vlan_tag_present(skb)) > > + /* Normal traffic path. */ > > + vsc73xx_vlan_rcv(skb, &src_port, &switch_id, &vbid, &vid); > > + > > + if (vbid >= 1) > > + skb->dev = dsa_tag_8021q_find_port_by_vbid(netdev, vbid); > > + else if (src_port == -1 || switch_id == -1) > > + skb->dev = dsa_find_designated_bridge_port_by_vid(netdev, vid); > > Hi Pawel, > > Smatch warns that vid may be used uninitialised here. > And it's not clear to me why that cannot be the case. >
The problem is only when skb_vlan_tag_present() is false.
If vsc73xx_vlan_rcv() is called then actually it's fine. Either vbid, src_port and switch_id will be set or vid will be initialized. Smatch thinks the vbid is set to 0-7, src_port is set to 0-15 and switch_id is set to 0-7. Smatch kind of ignores the switch_id == -1 condition because it's known to be false given that we already checked src_port == -1.
But the concern again is the skb_vlan_tag_present() is false and then vbid, src_port and switch_id would all be set to -1.
regards, dan carpenter
| |