netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Markus Blöchl" <markus.bloechl@ipetronik.com>
To: Woojung Huh <woojung.huh@microchip.com>,
	"David S. Miller" <davem@davemloft.net>,
	Jakub Kicinski <kuba@kernel.org>
Cc: Microchip Linux Driver Support <UNGLinuxDriver@microchip.com>,
	netdev@vger.kernel.org
Subject: [PATCH net] net: lan78xx: Disable hardware vlan filtering in promiscuous mode
Date: Tue, 10 Nov 2020 16:39:58 +0100	[thread overview]
Message-ID: <20201110153958.ci5ekor3o2ekg3ky@ipetronik.com> (raw)

The rx-vlan-filter feature flag prevents unexpected tagged frames on
the wire from reaching the kernel in promiscuous mode.
Disable this offloading feature in the lan7800 controller whenever
IFF_PROMISC is set and make sure that the hardware features
are updated when IFF_PROMISC changes.

Signed-off-by: Markus Blöchl <markus.bloechl@ipetronik.com>
---

Notes:
    When sniffing ethernet using a LAN7800 ethernet controller, vlan-tagged
    frames are silently dropped by the controller due to the
    RFE_CTL_VLAN_FILTER flag being set by default since commit
    4a27327b156e("net: lan78xx: Add support for VLAN filtering.").

    In order to receive those tagged frames it is necessary to manually disable
    rx vlan filtering using ethtool ( `ethtool -K ethX rx-vlan-filter off` or
    corresponding ioctls ). Setting all bits in the vlan filter table to 1 is
    an even worse approach, imho.

    As a user I would probably expect that setting IFF_PROMISC should be enough
    in all cases to receive all valid traffic.
    Therefore I think this behaviour is a bug in the driver, since other
    drivers (notably ixgbe) automatically disable rx-vlan-filter when
    IFF_PROMISC is set. Please correct me if I am wrong here.

 drivers/net/usb/lan78xx.c | 33 +++++++++++++++++++++++++--------
 1 file changed, 25 insertions(+), 8 deletions(-)

diff --git a/drivers/net/usb/lan78xx.c b/drivers/net/usb/lan78xx.c
index 65b315bc60ab..ac6c0beeac21 100644
--- a/drivers/net/usb/lan78xx.c
+++ b/drivers/net/usb/lan78xx.c
@@ -2324,13 +2324,15 @@ static int lan78xx_set_mac_addr(struct net_device *netdev, void *p)
 }

 /* Enable or disable Rx checksum offload engine */
-static int lan78xx_set_features(struct net_device *netdev,
-                               netdev_features_t features)
+static void lan78xx_update_features(struct net_device *netdev,
+                                   netdev_features_t features)
 {
        struct lan78xx_net *dev = netdev_priv(netdev);
        struct lan78xx_priv *pdata = (struct lan78xx_priv *)(dev->data[0]);
        unsigned long flags;
-       int ret;
+
+       if (netdev->flags & IFF_PROMISC)
+               features &= ~NETIF_F_HW_VLAN_CTAG_FILTER;

        spin_lock_irqsave(&pdata->rfe_ctl_lock, flags);

@@ -2353,12 +2355,30 @@ static int lan78xx_set_features(struct net_device *netdev,
                pdata->rfe_ctl &= ~RFE_CTL_VLAN_FILTER_;

        spin_unlock_irqrestore(&pdata->rfe_ctl_lock, flags);
+}
+
+static int lan78xx_set_features(struct net_device *netdev,
+                               netdev_features_t features)
+{
+       struct lan78xx_net *dev = netdev_priv(netdev);
+       struct lan78xx_priv *pdata = (struct lan78xx_priv *)(dev->data[0]);
+       int ret;
+
+       lan78xx_update_features(netdev, features);

        ret = lan78xx_write_reg(dev, RFE_CTL, pdata->rfe_ctl);

        return 0;
 }

+static void lan78xx_set_rx_mode(struct net_device *netdev)
+{
+       /* Enable or disable checksum offload engines */
+       lan78xx_update_features(netdev, netdev->features);
+
+       lan78xx_set_multicast(netdev);
+}
+
 static void lan78xx_deferred_vlan_write(struct work_struct *param)
 {
        struct lan78xx_priv *pdata =
@@ -2528,10 +2548,7 @@ static int lan78xx_reset(struct lan78xx_net *dev)
        pdata->rfe_ctl |= RFE_CTL_BCAST_EN_ | RFE_CTL_DA_PERFECT_;
        ret = lan78xx_write_reg(dev, RFE_CTL, pdata->rfe_ctl);

-       /* Enable or disable checksum offload engines */
-       lan78xx_set_features(dev->net, dev->net->features);
-
-       lan78xx_set_multicast(dev->net);
+       lan78xx_set_rx_mode(dev->net);

        /* reset PHY */
        ret = lan78xx_read_reg(dev, PMT_CTL, &buf);
@@ -3613,7 +3630,7 @@ static const struct net_device_ops lan78xx_netdev_ops = {
        .ndo_set_mac_address    = lan78xx_set_mac_addr,
        .ndo_validate_addr      = eth_validate_addr,
        .ndo_do_ioctl           = phy_do_ioctl_running,
-       .ndo_set_rx_mode        = lan78xx_set_multicast,
+       .ndo_set_rx_mode        = lan78xx_set_rx_mode,
        .ndo_set_features       = lan78xx_set_features,
        .ndo_vlan_rx_add_vid    = lan78xx_vlan_rx_add_vid,
        .ndo_vlan_rx_kill_vid   = lan78xx_vlan_rx_kill_vid,

base-commit: 4e0396c59559264442963b349ab71f66e471f84d
--
2.29.2


Impressum/Imprint: https://www.ipetronik.com/impressum

             reply	other threads:[~2020-11-10 15:40 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-11-10 15:39 Markus Blöchl [this message]
2020-11-11  3:13 ` [PATCH net] net: lan78xx: Disable hardware vlan filtering in promiscuous mode Jakub Kicinski
2020-11-11 15:43 ` Jakub Kicinski
2020-11-11 15:56   ` Florian Fainelli
2020-11-11 16:47     ` Vladimir Oltean
2020-11-11 17:14       ` Vladimir Oltean
2020-11-12 10:53       ` Markus Blöchl
2020-11-14 18:11         ` Vladimir Oltean
2020-11-19 15:37           ` Markus Blöchl
2020-11-19 16:28             ` Vladimir Oltean
2020-11-19 17:31             ` Jakub Kicinski
2020-11-11 18:02   ` Ido Schimmel
2020-11-12 22:52   ` Alexander Duyck
2020-11-13  0:44   ` Jakub Kicinski

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20201110153958.ci5ekor3o2ekg3ky@ipetronik.com \
    --to=markus.bloechl@ipetronik.com \
    --cc=UNGLinuxDriver@microchip.com \
    --cc=davem@davemloft.net \
    --cc=kuba@kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=woojung.huh@microchip.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).