Commit 9d6a569a authored by Julian Wiedmann's avatar Julian Wiedmann Committed by David S. Miller

s390/qeth: strictly order bridge address events

The current code for bridge address events has two shortcomings in its
control sequence:

1. after disabling address events via PNSO, we don't flush the remaining
   events from the event_wq. So if the feature is re-enabled fast
   enough, stale events could leak over.
2. PNSO and the events' arrival via the READ ccw device are unordered.
   So even if we flushed the workqueue, it's difficult to say whether
   the READ device might produce more events onto the workqueue
   afterwards.

Fix this by
1. explicitly fencing off the events when we no longer care, in the
   READ device's event handler. This ensures that once we flush the
   workqueue, it doesn't get additional address events.
2. Flush the workqueue after disabling the events & fencing them off.
   As the code that triggers the flush will typically hold the sbp_lock,
   we need to rework the worker code to avoid a deadlock here in case
   of a 'notifications-stopped' event. In case of lock contention,
   requeue such an event with a delay. We'll eventually aquire the lock,
   or spot that the feature has been disabled and the event can thus be
   discarded.

This leaves the theoretical race that a stale event could arrive
_after_ we re-enabled ourselves to receive events again. Such an event
would be impossible to distinguish from a 'good' event, nothing we can
do about it.
Signed-off-by: default avatarJulian Wiedmann <jwi@linux.ibm.com>
Reviewed-by: default avatarAlexandra Winter <wintera@linux.ibm.com>
Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
parent 65b0494e
...@@ -674,6 +674,11 @@ struct qeth_card_blkt { ...@@ -674,6 +674,11 @@ struct qeth_card_blkt {
int inter_packet_jumbo; int inter_packet_jumbo;
}; };
enum qeth_pnso_mode {
QETH_PNSO_NONE,
QETH_PNSO_BRIDGEPORT,
};
#define QETH_BROADCAST_WITH_ECHO 0x01 #define QETH_BROADCAST_WITH_ECHO 0x01
#define QETH_BROADCAST_WITHOUT_ECHO 0x02 #define QETH_BROADCAST_WITHOUT_ECHO 0x02
struct qeth_card_info { struct qeth_card_info {
...@@ -690,6 +695,7 @@ struct qeth_card_info { ...@@ -690,6 +695,7 @@ struct qeth_card_info {
/* no bitfield, we take a pointer on these two: */ /* no bitfield, we take a pointer on these two: */
u8 has_lp2lp_cso_v6; u8 has_lp2lp_cso_v6;
u8 has_lp2lp_cso_v4; u8 has_lp2lp_cso_v4;
enum qeth_pnso_mode pnso_mode;
enum qeth_card_types type; enum qeth_card_types type;
enum qeth_link_types link_type; enum qeth_link_types link_type;
int broadcast_capable; int broadcast_capable;
......
...@@ -273,6 +273,17 @@ static int qeth_l2_vlan_rx_kill_vid(struct net_device *dev, ...@@ -273,6 +273,17 @@ static int qeth_l2_vlan_rx_kill_vid(struct net_device *dev,
return qeth_l2_send_setdelvlan(card, vid, IPA_CMD_DELVLAN); return qeth_l2_send_setdelvlan(card, vid, IPA_CMD_DELVLAN);
} }
static void qeth_l2_set_pnso_mode(struct qeth_card *card,
enum qeth_pnso_mode mode)
{
spin_lock_irq(get_ccwdev_lock(CARD_RDEV(card)));
WRITE_ONCE(card->info.pnso_mode, mode);
spin_unlock_irq(get_ccwdev_lock(CARD_RDEV(card)));
if (mode == QETH_PNSO_NONE)
drain_workqueue(card->event_wq);
}
static void qeth_l2_stop_card(struct qeth_card *card) static void qeth_l2_stop_card(struct qeth_card *card)
{ {
QETH_CARD_TEXT(card, 2, "stopcard"); QETH_CARD_TEXT(card, 2, "stopcard");
...@@ -290,7 +301,7 @@ static void qeth_l2_stop_card(struct qeth_card *card) ...@@ -290,7 +301,7 @@ static void qeth_l2_stop_card(struct qeth_card *card)
qeth_qdio_clear_card(card, 0); qeth_qdio_clear_card(card, 0);
qeth_clear_working_pool_list(card); qeth_clear_working_pool_list(card);
flush_workqueue(card->event_wq); qeth_l2_set_pnso_mode(card, QETH_PNSO_NONE);
qeth_flush_local_addrs(card); qeth_flush_local_addrs(card);
card->info.promisc_mode = 0; card->info.promisc_mode = 0;
} }
...@@ -1153,19 +1164,34 @@ static void qeth_bridge_state_change(struct qeth_card *card, ...@@ -1153,19 +1164,34 @@ static void qeth_bridge_state_change(struct qeth_card *card,
} }
struct qeth_addr_change_data { struct qeth_addr_change_data {
struct work_struct worker; struct delayed_work dwork;
struct qeth_card *card; struct qeth_card *card;
struct qeth_ipacmd_addr_change ac_event; struct qeth_ipacmd_addr_change ac_event;
}; };
static void qeth_addr_change_event_worker(struct work_struct *work) static void qeth_addr_change_event_worker(struct work_struct *work)
{ {
struct qeth_addr_change_data *data = struct delayed_work *dwork = to_delayed_work(work);
container_of(work, struct qeth_addr_change_data, worker); struct qeth_addr_change_data *data;
struct qeth_card *card;
int i; int i;
data = container_of(dwork, struct qeth_addr_change_data, dwork);
card = data->card;
QETH_CARD_TEXT(data->card, 4, "adrchgew"); QETH_CARD_TEXT(data->card, 4, "adrchgew");
if (READ_ONCE(card->info.pnso_mode) == QETH_PNSO_NONE)
goto free;
if (data->ac_event.lost_event_mask) { if (data->ac_event.lost_event_mask) {
/* Potential re-config in progress, try again later: */
if (!mutex_trylock(&card->sbp_lock)) {
queue_delayed_work(card->event_wq, dwork,
msecs_to_jiffies(100));
return;
}
dev_info(&data->card->gdev->dev, dev_info(&data->card->gdev->dev,
"Address change notification stopped on %s (%s)\n", "Address change notification stopped on %s (%s)\n",
data->card->dev->name, data->card->dev->name,
...@@ -1174,8 +1200,9 @@ static void qeth_addr_change_event_worker(struct work_struct *work) ...@@ -1174,8 +1200,9 @@ static void qeth_addr_change_event_worker(struct work_struct *work)
: (data->ac_event.lost_event_mask == 0x02) : (data->ac_event.lost_event_mask == 0x02)
? "Bridge port state change" ? "Bridge port state change"
: "Unknown reason"); : "Unknown reason");
mutex_lock(&data->card->sbp_lock);
data->card->options.sbp.hostnotification = 0; data->card->options.sbp.hostnotification = 0;
card->info.pnso_mode = QETH_PNSO_NONE;
mutex_unlock(&data->card->sbp_lock); mutex_unlock(&data->card->sbp_lock);
qeth_bridge_emit_host_event(data->card, anev_abort, qeth_bridge_emit_host_event(data->card, anev_abort,
0, NULL, NULL); 0, NULL, NULL);
...@@ -1189,6 +1216,8 @@ static void qeth_addr_change_event_worker(struct work_struct *work) ...@@ -1189,6 +1216,8 @@ static void qeth_addr_change_event_worker(struct work_struct *work)
&entry->token, &entry->token,
&entry->addr_lnid); &entry->addr_lnid);
} }
free:
kfree(data); kfree(data);
} }
...@@ -1200,6 +1229,9 @@ static void qeth_addr_change_event(struct qeth_card *card, ...@@ -1200,6 +1229,9 @@ static void qeth_addr_change_event(struct qeth_card *card,
struct qeth_addr_change_data *data; struct qeth_addr_change_data *data;
int extrasize; int extrasize;
if (card->info.pnso_mode == QETH_PNSO_NONE)
return;
QETH_CARD_TEXT(card, 4, "adrchgev"); QETH_CARD_TEXT(card, 4, "adrchgev");
if (cmd->hdr.return_code != 0x0000) { if (cmd->hdr.return_code != 0x0000) {
if (cmd->hdr.return_code == 0x0010) { if (cmd->hdr.return_code == 0x0010) {
...@@ -1219,11 +1251,11 @@ static void qeth_addr_change_event(struct qeth_card *card, ...@@ -1219,11 +1251,11 @@ static void qeth_addr_change_event(struct qeth_card *card,
QETH_CARD_TEXT(card, 2, "ACNalloc"); QETH_CARD_TEXT(card, 2, "ACNalloc");
return; return;
} }
INIT_WORK(&data->worker, qeth_addr_change_event_worker); INIT_DELAYED_WORK(&data->dwork, qeth_addr_change_event_worker);
data->card = card; data->card = card;
memcpy(&data->ac_event, hostevs, memcpy(&data->ac_event, hostevs,
sizeof(struct qeth_ipacmd_addr_change) + extrasize); sizeof(struct qeth_ipacmd_addr_change) + extrasize);
queue_work(card->event_wq, &data->worker); queue_delayed_work(card->event_wq, &data->dwork, 0);
} }
/* SETBRIDGEPORT support; sending commands */ /* SETBRIDGEPORT support; sending commands */
...@@ -1545,9 +1577,14 @@ int qeth_bridgeport_an_set(struct qeth_card *card, int enable) ...@@ -1545,9 +1577,14 @@ int qeth_bridgeport_an_set(struct qeth_card *card, int enable)
if (enable) { if (enable) {
qeth_bridge_emit_host_event(card, anev_reset, 0, NULL, NULL); qeth_bridge_emit_host_event(card, anev_reset, 0, NULL, NULL);
qeth_l2_set_pnso_mode(card, QETH_PNSO_BRIDGEPORT);
rc = qeth_l2_pnso(card, 1, qeth_bridgeport_an_set_cb, card); rc = qeth_l2_pnso(card, 1, qeth_bridgeport_an_set_cb, card);
} else if (rc)
qeth_l2_set_pnso_mode(card, QETH_PNSO_NONE);
} else {
rc = qeth_l2_pnso(card, 0, NULL, NULL); rc = qeth_l2_pnso(card, 0, NULL, NULL);
qeth_l2_set_pnso_mode(card, QETH_PNSO_NONE);
}
return rc; return rc;
} }
......
...@@ -157,6 +157,7 @@ static ssize_t qeth_bridgeport_hostnotification_store(struct device *dev, ...@@ -157,6 +157,7 @@ static ssize_t qeth_bridgeport_hostnotification_store(struct device *dev,
rc = -EBUSY; rc = -EBUSY;
else if (qeth_card_hw_is_reachable(card)) { else if (qeth_card_hw_is_reachable(card)) {
rc = qeth_bridgeport_an_set(card, enable); rc = qeth_bridgeport_an_set(card, enable);
/* sbp_lock ensures ordering vs notifications-stopped events */
if (!rc) if (!rc)
card->options.sbp.hostnotification = enable; card->options.sbp.hostnotification = enable;
} else } else
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment