[PATCH net-next 6/8] s390/qeth: Reset address notification in case of buffer overflow

Julian Wiedmann
2020-09-10T17:24:10Z

In case hardware sends more device-to-bridge-address-change notfications
than the qeth-l2 driver can handle, the hardware will send an overflow
event and then stop sending any events. It expects software to flush its
FDB and start over again. Re-enabling address-change-notification will
report all current addresses.

In order to re-enable address-change-notification this patch defines
the functions qeth_l2_dev2br_an_set() and qeth_l2_dev2br_an_set_cb
to enable or disable dev-to-bridge-address-notification.

A following patch will use the learning_sync bridgeport flag to trigger
enabling or disabling of address-change-notification, so we define
priv->brport_features to store the current setting. BRIDGE_INFO and
ADDR_INFO functionality are mutually exclusive, whereas ADDR_INFO and
qeth_l2_vnicc* can be used together.

Alternative implementations to handle buffer overflow:
Just re-enabling notification and adding all newly reported addresses
would cover any lost 'add' events, but not the lost 'delete' events.
Then these invalid addresses would stay in the bridge FDB as long as the
device exists.
Setting the net device down and up, would be an alternative, but is a bit
drastic. If the net device has many secondary addresses this will create
many delete/add events at its peers which could de-stabilize the
network segment.

Signed-off-by: Alexandra Winter <wintera@linux.ibm.com>
Reviewed-by: Julian Wiedmann <jwi@linux.ibm.com>
Signed-off-by: Julian Wiedmann <jwi@linux.ibm.com>
---
drivers/s390/net/qeth_core.h | 1 +
drivers/s390/net/qeth_l2.h | 2 +-
drivers/s390/net/qeth_l2_main.c | 114 +++++++++++++++++++++++++++++++-
drivers/s390/net/qeth_l2_sys.c | 16 ++---
4 files changed, 123 insertions(+), 10 deletions(-)

diff --git a/drivers/s390/net/qeth_core.h b/drivers/s390/net/qeth_core.h
index 4c8134a953c9..2c14012ca35d 100644
--- a/drivers/s390/net/qeth_core.h
+++ b/drivers/s390/net/qeth_core.h
@@ -789,6 +789,7 @@ struct qeth_switch_info {
struct qeth_priv {
unsigned int rx_copybreak;
u32 brport_hw_features;
+ u32 brport_features;
};

#define QETH_NAPI_WEIGHT NAPI_POLL_WEIGHT
diff --git a/drivers/s390/net/qeth_l2.h b/drivers/s390/net/qeth_l2.h
index adf25c9fd2b3..cc95675c8bc4 100644
--- a/drivers/s390/net/qeth_l2.h
+++ b/drivers/s390/net/qeth_l2.h
@@ -23,7 +23,7 @@ int qeth_l2_vnicc_set_state(struct qeth_card *card, u32 vnicc, bool state);
int qeth_l2_vnicc_get_state(struct qeth_card *card, u32 vnicc, bool *state);
int qeth_l2_vnicc_set_timeout(struct qeth_card *card, u32 timeout);
int qeth_l2_vnicc_get_timeout(struct qeth_card *card, u32 *timeout);
-bool qeth_l2_vnicc_is_in_use(struct qeth_card *card);
+bool qeth_bridgeport_allowed(struct qeth_card *card);

struct qeth_mac {
u8 mac_addr[ETH_ALEN];
diff --git a/drivers/s390/net/qeth_l2_main.c b/drivers/s390/net/qeth_l2_main.c
index fffbc50cadc6..ef2962e03546 100644
--- a/drivers/s390/net/qeth_l2_main.c
+++ b/drivers/s390/net/qeth_l2_main.c
@@ -287,6 +287,22 @@ static void qeth_l2_set_pnso_mode(struct qeth_card *card,
drain_workqueue(card->event_wq);
}

+static void qeth_l2_dev2br_fdb_flush(struct qeth_card *card)
+{
+ struct switchdev_notifier_fdb_info info;
+
+ QETH_CARD_TEXT(card, 2, "fdbflush");
+
+ info.addr = NULL;
+ /* flush all VLANs: */
+ info.vid = 0;
+ info.added_by_user = false;
+ info.offloaded = true;
+
+ call_switchdev_notifiers(SWITCHDEV_FDB_FLUSH_TO_BRIDGE,
+ card->dev, &info.info, NULL);
+}
+
static void qeth_l2_stop_card(struct qeth_card *card)
{
QETH_CARD_TEXT(card, 2, "stopcard");
@@ -772,6 +788,54 @@ static void qeth_l2_dev2br_fdb_notify(struct qeth_card *card, u8 code,
}
}

+static void qeth_l2_dev2br_an_set_cb(void *priv,
+ struct chsc_pnso_naid_l2 *entry)
+{
+ u8 code = IPA_ADDR_CHANGE_CODE_MACADDR;
+ struct qeth_card *card = priv;
+
+ if (entry->addr_lnid.lnid < VLAN_N_VID)
+ code |= IPA_ADDR_CHANGE_CODE_VLANID;
+ qeth_l2_dev2br_fdb_notify(card, code,
+ (struct net_if_token *)&entry->nit,
+ (struct mac_addr_lnid *)&entry->addr_lnid);
+}
+
+/**
+ * qeth_l2_dev2br_an_set() -
+ * Enable or disable 'dev to bridge network address notification'
+ * @card: qeth_card structure pointer
+ * @enable: Enable or disable 'dev to bridge network address notification'
+ *
+ * Returns negative errno-compatible error indication or 0 on success.
+ *
+ * On enable, emits a series of address notifications for all
+ * currently registered hosts.
+ *
+ * Must be called under rtnl_lock
+ */
+static int qeth_l2_dev2br_an_set(struct qeth_card *card, bool enable)
+{
+ int rc;
+
+ if (enable) {
+ QETH_CARD_TEXT(card, 2, "anseton");
+ rc = qeth_l2_pnso(card, PNSO_OC_NET_ADDR_INFO, 1,
+ qeth_l2_dev2br_an_set_cb, card);
+ if (rc == -EAGAIN)
+ /* address notification enabled, but inconsistent
+ * addresses reported -> disable address notification
+ */
+ qeth_l2_pnso(card, PNSO_OC_NET_ADDR_INFO, 0,
+ NULL, NULL);
+ } else {
+ QETH_CARD_TEXT(card, 2, "ansetoff");
+ rc = qeth_l2_pnso(card, PNSO_OC_NET_ADDR_INFO, 0, NULL, NULL);
+ }
+
+ return rc;
+}
+
static const struct net_device_ops qeth_l2_netdev_ops = {
.ndo_open = qeth_open,
.ndo_stop = qeth_stop,
@@ -1284,10 +1348,13 @@ static void qeth_l2_dev2br_worker(struct work_struct *work)
struct delayed_work *dwork = to_delayed_work(work);
struct qeth_addr_change_data *data;
struct qeth_card *card;
+ struct qeth_priv *priv;
unsigned int i;
+ int rc;

data = container_of(dwork, struct qeth_addr_change_data, dwork);
card = data->card;
+ priv = netdev_priv(card->dev);

QETH_CARD_TEXT(card, 4, "dev2brew");

@@ -1300,11 +1367,39 @@ static void qeth_l2_dev2br_worker(struct work_struct *work)
msecs_to_jiffies(100));
return;
}
+ if (!netif_device_present(card->dev))
+ goto out_unlock;

if (data->ac_event.lost_event_mask) {
QETH_DBF_MESSAGE(3,
"Address change notification overflow on device %x\n",
CARD_DEVID(card));
+ /* Card fdb and bridge fdb are out of sync, card has stopped
+ * notifications (no need to drain_workqueue). Purge all
+ * 'extern_learn' entries from the parent bridge and restart
+ * the notifications.
+ */
+ qeth_l2_dev2br_fdb_flush(card);
+ rc = qeth_l2_dev2br_an_set(card, true);
+ if (rc) {
+ /* TODO: if we want to retry after -EAGAIN, be
+ * aware there could be stale entries in the
+ * workqueue now, that need to be drained.
+ * For now we give up:
+ */
+ netdev_err(card->dev,
+ "bridge learning_sync failed to recover: %d\n",
+ rc);
+ WRITE_ONCE(card->info.pnso_mode,
+ QETH_PNSO_NONE);
+ /* To remove fdb entries reported by an_set: */
+ qeth_l2_dev2br_fdb_flush(card);
+ priv->brport_features ^= BR_LEARNING_SYNC;
+ } else {
+ QETH_DBF_MESSAGE(3,
+ "Address Notification resynced on device %x\n",
+ CARD_DEVID(card));
+ }
} else {
for (i = 0; i < data->ac_event.num_entries; i++) {
struct qeth_ipacmd_addr_change_entry *entry =
@@ -1315,6 +1410,8 @@ static void qeth_l2_dev2br_worker(struct work_struct *work)
&entry->addr_lnid);
}
}
+
+out_unlock:
rtnl_unlock();

free:
@@ -2035,7 +2132,7 @@ int qeth_l2_vnicc_get_timeout(struct qeth_card *card, u32 *timeout)
}

/* check if VNICC is currently enabled */
-bool qeth_l2_vnicc_is_in_use(struct qeth_card *card)
+static bool _qeth_l2_vnicc_is_in_use(struct qeth_card *card)
{
if (!card->options.vnicc.sup_chars)
return false;
@@ -2050,6 +2147,21 @@ bool qeth_l2_vnicc_is_in_use(struct qeth_card *card)
return true;
}

+/**
+ * qeth_bridgeport_allowed - are any qeth_bridgeport functions allowed?
+ * @card: qeth_card structure pointer
+ *
+ * qeth_bridgeport functionality is mutually exclusive with usage of the
+ * VNIC Characteristics and dev2br address notifications
+ */
+bool qeth_bridgeport_allowed(struct qeth_card *card)
+{
+ struct qeth_priv *priv = netdev_priv(card->dev);
+
+ return (!_qeth_l2_vnicc_is_in_use(card) &&
+ !(priv->brport_features & BR_LEARNING_SYNC));
+}
+
/* recover user timeout setting */
static bool qeth_l2_vnicc_recover_timeout(struct qeth_card *card, u32 vnicc,
u32 *timeout)
diff --git a/drivers/s390/net/qeth_l2_sys.c b/drivers/s390/net/qeth_l2_sys.c
index 4695d25e54f2..4ba3bc57263f 100644
--- a/drivers/s390/net/qeth_l2_sys.c
+++ b/drivers/s390/net/qeth_l2_sys.c
@@ -18,7 +18,7 @@ static ssize_t qeth_bridge_port_role_state_show(struct device *dev,
int rc = 0;
char *word;

- if (qeth_l2_vnicc_is_in_use(card))
+ if (!qeth_bridgeport_allowed(card))
return sprintf(buf, "n/a (VNIC characteristics)\n");

mutex_lock(&card->sbp_lock);
@@ -65,7 +65,7 @@ static ssize_t qeth_bridge_port_role_show(struct device *dev,
{
struct qeth_card *card = dev_get_drvdata(dev);

- if (qeth_l2_vnicc_is_in_use(card))
+ if (!qeth_bridgeport_allowed(card))
return sprintf(buf, "n/a (VNIC characteristics)\n");

return qeth_bridge_port_role_state_show(dev, attr, buf, 0);
@@ -90,7 +90,7 @@ static ssize_t qeth_bridge_port_role_store(struct device *dev,
mutex_lock(&card->conf_mutex);
mutex_lock(&card->sbp_lock);

- if (qeth_l2_vnicc_is_in_use(card))
+ if (!qeth_bridgeport_allowed(card))
rc = -EBUSY;
else if (card->options.sbp.reflect_promisc)
/* Forbid direct manipulation */
@@ -116,7 +116,7 @@ static ssize_t qeth_bridge_port_state_show(struct device *dev,
{
struct qeth_card *card = dev_get_drvdata(dev);

- if (qeth_l2_vnicc_is_in_use(card))
+ if (!qeth_bridgeport_allowed(card))
return sprintf(buf, "n/a (VNIC characteristics)\n");

return qeth_bridge_port_role_state_show(dev, attr, buf, 1);
@@ -131,7 +131,7 @@ static ssize_t qeth_bridgeport_hostnotification_show(struct device *dev,
struct qeth_card *card = dev_get_drvdata(dev);
int enabled;

- if (qeth_l2_vnicc_is_in_use(card))
+ if (!qeth_bridgeport_allowed(card))
return sprintf(buf, "n/a (VNIC characteristics)\n");

enabled = card->options.sbp.hostnotification;
@@ -153,7 +153,7 @@ static ssize_t qeth_bridgeport_hostnotification_store(struct device *dev,
mutex_lock(&card->conf_mutex);
mutex_lock(&card->sbp_lock);

- if (qeth_l2_vnicc_is_in_use(card))
+ if (!qeth_bridgeport_allowed(card))
rc = -EBUSY;
else if (qeth_card_hw_is_reachable(card)) {
rc = qeth_bridgeport_an_set(card, enable);
@@ -179,7 +179,7 @@ static ssize_t qeth_bridgeport_reflect_show(struct device *dev,
struct qeth_card *card = dev_get_drvdata(dev);
char *state;

- if (qeth_l2_vnicc_is_in_use(card))
+ if (!qeth_bridgeport_allowed(card))
return sprintf(buf, "n/a (VNIC characteristics)\n");

if (card->options.sbp.reflect_promisc) {
@@ -215,7 +215,7 @@ static ssize_t qeth_bridgeport_reflect_store(struct device *dev,
mutex_lock(&card->conf_mutex);
mutex_lock(&card->sbp_lock);

- if (qeth_l2_vnicc_is_in_use(card))
+ if (!qeth_bridgeport_allowed(card))
rc = -EBUSY;
else if (card->options.sbp.role != QETH_SBP_ROLE_NONE)
rc = -EPERM;
--
2.17.1