On 7/24/19 4:45 PM, Saeed Mahameed wrote:
On Mon, 2019-07-22 at 14:40 -0700, Shannon Nelson wrote:
Set up the initial NDO structure and callbacks for netdev
to use, and register the netdev.  This will allow us to do
a few basic operations on the device, but no traffic yet.

Signed-off-by: Shannon Nelson<snel...@pensando.io>
---
  drivers/net/ethernet/pensando/ionic/ionic.h   |   1 +
  .../ethernet/pensando/ionic/ionic_bus_pci.c   |   9 +
  .../net/ethernet/pensando/ionic/ionic_dev.h   |   2 +
  .../net/ethernet/pensando/ionic/ionic_lif.c   | 348
++++++++++++++++++
  .../net/ethernet/pensando/ionic/ionic_lif.h   |   5 +
  5 files changed, 365 insertions(+)

[...]
+static int ionic_set_nic_features(struct lif *lif, netdev_features_t
features);
  static int ionic_notifyq_clean(struct lif *lif, int budget);
+int ionic_open(struct net_device *netdev)
+{
+       struct lif *lif = netdev_priv(netdev);
+
+       netif_carrier_off(netdev);
+
+       set_bit(LIF_UP, lif->state);
+
+       if (netif_carrier_ok(netdev))
always false ? you just invoked netif_carrier_off two lines ago..

Hmmm... an artifact of splitting up an existing driver.  This makes more sense when there's a link status check in between these, which comes in about 3 patches later.  Unless this really causes someone significant heartburn, I'm going to leave this as is for now.


+               netif_tx_wake_all_queues(netdev);
+
+       return 0;
+}
+
+static int ionic_lif_stop(struct lif *lif)
+{
+       struct net_device *ndev = lif->netdev;
+       int err = 0;
+
+       if (!test_bit(LIF_UP, lif->state)) {
+               dev_dbg(lif->ionic->dev, "%s: %s state=DOWN\n",
+                       __func__, lif->name);
+               return 0;
+       }
+       dev_dbg(lif->ionic->dev, "%s: %s state=UP\n", __func__, lif-
name);
+       clear_bit(LIF_UP, lif->state);
+
+       /* carrier off before disabling queues to avoid watchdog
timeout */
+       netif_carrier_off(ndev);
+       netif_tx_stop_all_queues(ndev);
+       netif_tx_disable(ndev);
+       synchronize_rcu();
why synchronize_rcu ?

Looks like a little leakage from a feature in the internal driver, I'll remove it.

+
+       return err;
+}
+
+int ionic_stop(struct net_device *netdev)
+{
+       struct lif *lif = netdev_priv(netdev);
+
+       return ionic_lif_stop(lif);
+}
+
+int ionic_reset_queues(struct lif *lif)
+{
+       bool running;
+       int err = 0;
+
+       /* Put off the next watchdog timeout */
+       netif_trans_update(lif->netdev);
this doesn't seem right to me also this won't help you if the next
while loop takes too long.. also netif_trans_update is marked to be
only used for legacy drivers.

If the loop takes too long, then I don't mind if the watchdog goes off.

Yes, its primary use is now handled by netdev_start_xmit(), but it is still a handy gizmo to give a little more cushion until the carrier is set off in ionic_lif_stop().

sln

Reply via email to