| Message ID | 1420566212-30081-1-git-send-email-balbi@ti.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
From: Felipe Balbi <balbi@ti.com> Date: Tue, 6 Jan 2015 11:43:32 -0600 > CPSW completely hangs if we add, and later remove, > VLAN ID #1. What happens is that after removing > VLAN ID #1, no packets will be received by CPSW > rendering network unusable. > > In order to "fix" the issue, we're returning -EINVAL > if anybody tries to add VLAN ID #1. While at that, > also filter out any ID > 4095 because we only have > 12 bits for VLAN IDs. > > Fixes: 3b72c2f (drivers: net:ethernet: cpsw: add support for VLAN) > Cc: <stable@vger.kernel.org> # v3.9+ > Cc: Mugunthan V N <mugunthanvnm@ti.com> > Tested-by: Schuyler Patton <spatton@ti.com> > Signed-off-by: Felipe Balbi <balbi@ti.com> You can't just unilaterally make one VLAN ID unusable. A better way to handle this situation must be found, and if that means turning off hw VLAN support completely, that's a much better alternative to this. I'm not applying this patch, sorry. -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi, On Tue, Jan 06, 2015 at 02:13:23PM -0500, David Miller wrote: > From: Felipe Balbi <balbi@ti.com> > Date: Tue, 6 Jan 2015 11:43:32 -0600 > > > CPSW completely hangs if we add, and later remove, > > VLAN ID #1. What happens is that after removing > > VLAN ID #1, no packets will be received by CPSW > > rendering network unusable. > > > > In order to "fix" the issue, we're returning -EINVAL > > if anybody tries to add VLAN ID #1. While at that, > > also filter out any ID > 4095 because we only have > > 12 bits for VLAN IDs. > > > > Fixes: 3b72c2f (drivers: net:ethernet: cpsw: add support for VLAN) > > Cc: <stable@vger.kernel.org> # v3.9+ > > Cc: Mugunthan V N <mugunthanvnm@ti.com> > > Tested-by: Schuyler Patton <spatton@ti.com> > > Signed-off-by: Felipe Balbi <balbi@ti.com> > > You can't just unilaterally make one VLAN ID unusable. > > A better way to handle this situation must be found, > and if that means turning off hw VLAN support completely, > that's a much better alternative to this. > > I'm not applying this patch, sorry. All other IDs work alright, it's just ID 1 which seems to be quirky. In fact when trying to add VLAN ID 1, vconfig itself dumps out a warning that VLAN ID 1 doesn't work on most switches. What you're saying here is that you prefer to drop a feature that works for all other 1023 IDs because 1 ID is quirky. Sounds like overkill to me.
From: Felipe Balbi <balbi@ti.com> Date: Tue, 6 Jan 2015 14:31:19 -0600 > What you're saying here is that you prefer to drop a feature that works > for all other 1023 IDs because 1 ID is quirky. Sounds like overkill > to me. The other option is to software fallback only for VLAN 1. -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi, On Tue, Jan 06, 2015 at 04:59:11PM -0500, David Miller wrote: > From: Felipe Balbi <balbi@ti.com> > Date: Tue, 6 Jan 2015 14:31:19 -0600 > > > What you're saying here is that you prefer to drop a feature that works > > for all other 1023 IDs because 1 ID is quirky. Sounds like overkill > > to me. > > The other option is to software fallback only for VLAN 1. now we're talking. Keep in mind, however, that this IP runs on mere single-core cortex A8 and single-core cortex A9 devices which already have somewhat of a hard-time keeping up with the non-accelerated checksum calculations. But fair enough, if that's the way to go, it is the way to go. cheers
diff --git a/drivers/net/ethernet/ti/cpsw.c b/drivers/net/ethernet/ti/cpsw.c index e61ee8351272..028bb7f3de65 100644 --- a/drivers/net/ethernet/ti/cpsw.c +++ b/drivers/net/ethernet/ti/cpsw.c @@ -1669,6 +1669,13 @@ static int cpsw_ndo_vlan_rx_add_vid(struct net_device *ndev, if (vid == priv->data.default_vlan) return 0; + /* NOTICE: CPSW does not support VID 1. We should + * also filter out VID > 4095 as we only have 12 + * bits for VID entries + */ + if (vid == 1 || vid >= VLAN_N_VID) + return -EINVAL; + dev_info(priv->dev, "Adding vlanid %d to vlan filter\n", vid); return cpsw_add_vlan_ale_entry(priv, vid); } @@ -1682,6 +1689,13 @@ static int cpsw_ndo_vlan_rx_kill_vid(struct net_device *ndev, if (vid == priv->data.default_vlan) return 0; + /* NOTICE: CPSW does not support VID 1. We should + * also filter out VID > 4095 as we only have 12 + * bits for VID entries + */ + if (vid == 1 || vid >= VLAN_N_VID) + return -EINVAL; + dev_info(priv->dev, "removing vlanid %d from vlan filter\n", vid); ret = cpsw_ale_del_vlan(priv->ale, vid, 0); if (ret != 0)