On Thu, 2020-11-05 at 16:12 -0800, Shannon Nelson wrote:
> The _ionic_lif_rx_mode() is only used once and really doesn't
> need to be broken out.
>
> Signed-off-by: Shannon Nelson <snel...@pensando.io>
> ---
> .../net/ethernet/pensando/ionic/ionic_lif.c | 38 ++++++++---------
> --
> 1 file changed, 16 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/net/ethernet/pensando/ionic/ionic_lif.c
> b/drivers/net/ethernet/pensando/ionic/ionic_lif.c
> index a0d26fe4cbc3..ef092ee33e59 100644
> --- a/drivers/net/ethernet/pensando/ionic/ionic_lif.c
> +++ b/drivers/net/ethernet/pensando/ionic/ionic_lif.c
> @@ -1129,29 +1129,10 @@ static void ionic_lif_rx_mode(struct
> ionic_lif *lif, unsigned int rx_mode)
> lif->rx_mode = rx_mode;
> }
>
> -static void _ionic_lif_rx_mode(struct ionic_lif *lif, unsigned int
> rx_mode,
> - bool from_ndo)
> -{
> - struct ionic_deferred_work *work;
> -
> - if (from_ndo) {
> - work = kzalloc(sizeof(*work), GFP_ATOMIC);
> - if (!work) {
> - netdev_err(lif->netdev, "%s OOM\n", __func__);
> - return;
> - }
> - work->type = IONIC_DW_TYPE_RX_MODE;
> - work->rx_mode = rx_mode;
> - netdev_dbg(lif->netdev, "deferred: rx_mode\n");
> - ionic_lif_deferred_enqueue(&lif->deferred, work);
> - } else {
> - ionic_lif_rx_mode(lif, rx_mode);
> - }
> -}
> -
> static void ionic_set_rx_mode(struct net_device *netdev, bool
> from_ndo)
> {
> struct ionic_lif *lif = netdev_priv(netdev);
> + struct ionic_deferred_work *work;
> unsigned int nfilters;
> unsigned int rx_mode;
>
> @@ -1197,8 +1178,21 @@ static void ionic_set_rx_mode(struct
> net_device *netdev, bool from_ndo)
> rx_mode &= ~IONIC_RX_MODE_F_ALLMULTI;
> }
>
> - if (lif->rx_mode != rx_mode)
> - _ionic_lif_rx_mode(lif, rx_mode, from_ndo);
> + if (lif->rx_mode != rx_mode) {
> + if (from_ndo) {
> + work = kzalloc(sizeof(*work), GFP_ATOMIC);
> + if (!work) {
> + netdev_err(lif->netdev, "%s OOM\n",
> __func__);
> + return;
> + }
> + work->type = IONIC_DW_TYPE_RX_MODE;
> + work->rx_mode = rx_mode;
> + netdev_dbg(lif->netdev, "deferred: rx_mode\n");
> + ionic_lif_deferred_enqueue(&lif->deferred,
> work);
> + } else {
> + ionic_lif_rx_mode(lif, rx_mode);
> + }
> + }
> }
You could move this logic one level up and totally eliminate the if
condition
ionic_set_rx_mode_needed() {
//sync driver data base
return lif->rx_mode != rx_mode;
}
ndo_set_rx_mode() {
if (!ionic_set_rx_mode_needed())
return; // no change;
schedule_work(set_rx_mode_hw);
}
none_ndo_set_rx_mode() {
if (!ionic_set_rx_mode_needed())
return; // no change;
set_rx_mode_hw();
}
Future improvement:
One more thing I've noticed about you current ionic_set_rx_mode()
is that in case of from_ndo, when it syncs mac addresses it will
schedule a deferred mac address update work to hw per address. which i
think is an overkill, a simpler design which will totally eliminate the
need for from_ndo flags, is to do similar to the above but with a minor
change.
ionic_set_rx_mode_needed() {
// Just sync driver mac table here and update hw later
// in one deferred work rather than scheduling multi work
addr_changed = ionic_dev_uc_sync();
addr_changed |= ionic_dev_mc_sync();
rx_mode_changed = sync_driver_rx_mode(rx_mode);
return rx_mode_changed || addr_changed;
}
/* might sleep */
set_rx_mode_hw() {
commit_addr_change_to_hw();
commit_rx_mode_changes_to_hw();
}
ndo_set_rx_mode() {
if (!ionic_set_rx_mode_needed())
return; // no change;
schedule_work(set_rx_mode_hw);
}
none_ndo_set_rx_mode() {
if (!ionic_set_rx_mode_needed())
return; // no change;
set_rx_mode_hw();
}
