Skip to content

Commit d456015

Browse files
julianwiedmanndavem330
julianwiedmann
authored and
davem330
committed
s390/qeth: call dev_close() during recovery
When resetting an interface ("recovery"), qeth currently attempts to elide the call to dev_close(). We initially only call .ndo_close to quiesce the data path, and then offline & online the ccwgroup device. If the reset succeeded, a call to .ndo_open then resumes the data path along with some internal setup (dev_addr validation, RX modeset) that dev_open() would have usually triggered. dev_close() only gets called (via the close_dev worker) if the reset action fails. It's unclear whether this was initially done due to locking concerns, or rather to execute the reset transparently. Either way, temporarily closing the interface without dev_close() is fragile, and means we're susceptible to various races and unexpected behaviour. For instance: - Bypassing dev_deactivate_many() means that the qdiscs are not set to __QDISC_STATE_DEACTIVATED. Consequently any intermittent TX completion can wake up the txq, resulting in calls to .ndo_start_xmit while the data path is down. We have custom state checking to detect this case and drop such packets. - Because the IFF_UP flag doesn't reflect the interface's actual state during a reset, we have custom state checking in .ndo_open and .ndo_close to guard against invalid calls. - Considering that the reset might take a considerable amount of time (in particular if an IO fails and we end up waiting for its timeout), we _do_ want NETDEV_GOING_DOWN and NETDEV_DOWN events so that components like bonding, team, bridge, macvlan, vlan, ... can take appropriate action. Signed-off-by: Julian Wiedmann <[email protected]> Signed-off-by: David S. Miller <[email protected]>
1 parent 7bd2275 commit d456015

File tree

3 files changed

+20
-75
lines changed

3 files changed

+20
-75
lines changed

‎drivers/s390/net/qeth_core_main.c

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -89,9 +89,6 @@ static void qeth_close_dev_handler(struct work_struct *work)
8989

9090
card = container_of(work, struct qeth_card, close_dev_work);
9191
QETH_CARD_TEXT(card, 2, "cldevhdl");
92-
rtnl_lock();
93-
dev_close(card->dev);
94-
rtnl_unlock();
9592
ccwgroup_set_offline(card->gdev);
9693
}
9794

‎drivers/s390/net/qeth_l2_main.c

Lines changed: 10 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -285,24 +285,16 @@ static int qeth_l2_vlan_rx_kill_vid(struct net_device *dev,
285285
return qeth_l2_send_setdelvlan(card, vid, IPA_CMD_DELVLAN);
286286
}
287287

288-
static void qeth_l2_stop_card(struct qeth_card *card, int recovery_mode)
288+
static void qeth_l2_stop_card(struct qeth_card *card)
289289
{
290290
QETH_DBF_TEXT(SETUP , 2, "stopcard");
291291
QETH_DBF_HEX(SETUP, 2, &card, sizeof(void *));
292292

293293
qeth_set_allowed_threads(card, 0, 1);
294294
if (card->read.state == CH_STATE_UP &&
295295
card->write.state == CH_STATE_UP &&
296-
(card->state == CARD_STATE_UP)) {
297-
if (recovery_mode && !IS_OSN(card)) {
298-
qeth_stop(card->dev);
299-
} else {
300-
rtnl_lock();
301-
dev_close(card->dev);
302-
rtnl_unlock();
303-
}
296+
card->state == CARD_STATE_UP)
304297
card->state = CARD_STATE_SOFTSETUP;
305-
}
306298
if (card->state == CARD_STATE_SOFTSETUP) {
307299
qeth_l2_del_all_macs(card);
308300
qeth_clear_ipacmd_list(card);
@@ -802,7 +794,7 @@ static void qeth_l2_trace_features(struct qeth_card *card)
802794
sizeof(card->options.vnicc.sup_chars));
803795
}
804796

805-
static int __qeth_l2_set_online(struct ccwgroup_device *gdev, int recovery_mode)
797+
static int qeth_l2_set_online(struct ccwgroup_device *gdev)
806798
{
807799
struct qeth_card *card = dev_get_drvdata(&gdev->dev);
808800
struct net_device *dev = card->dev;
@@ -882,14 +874,7 @@ static int __qeth_l2_set_online(struct ccwgroup_device *gdev, int recovery_mode)
882874

883875
if (card->info.open_when_online) {
884876
card->info.open_when_online = 0;
885-
if (recovery_mode && !IS_OSN(card)) {
886-
if (!qeth_l2_validate_addr(dev)) {
887-
qeth_open(dev);
888-
qeth_l2_set_rx_mode(dev);
889-
}
890-
} else {
891-
dev_open(dev, NULL);
892-
}
877+
dev_open(dev, NULL);
893878
}
894879
rtnl_unlock();
895880
}
@@ -900,7 +885,7 @@ static int __qeth_l2_set_online(struct ccwgroup_device *gdev, int recovery_mode)
900885
return 0;
901886

902887
out_remove:
903-
qeth_l2_stop_card(card, 0);
888+
qeth_l2_stop_card(card);
904889
ccw_device_set_offline(CARD_DDEV(card));
905890
ccw_device_set_offline(CARD_WDEV(card));
906891
ccw_device_set_offline(CARD_RDEV(card));
@@ -912,11 +897,6 @@ static int __qeth_l2_set_online(struct ccwgroup_device *gdev, int recovery_mode)
912897
return rc;
913898
}
914899

915-
static int qeth_l2_set_online(struct ccwgroup_device *gdev)
916-
{
917-
return __qeth_l2_set_online(gdev, 0);
918-
}
919-
920900
static int __qeth_l2_set_offline(struct ccwgroup_device *cgdev,
921901
int recovery_mode)
922902
{
@@ -935,11 +915,12 @@ static int __qeth_l2_set_offline(struct ccwgroup_device *cgdev,
935915

936916
rtnl_lock();
937917
card->info.open_when_online = card->dev->flags & IFF_UP;
918+
dev_close(card->dev);
938919
netif_device_detach(card->dev);
939920
netif_carrier_off(card->dev);
940921
rtnl_unlock();
941922

942-
qeth_l2_stop_card(card, recovery_mode);
923+
qeth_l2_stop_card(card);
943924
rc = ccw_device_set_offline(CARD_DDEV(card));
944925
rc2 = ccw_device_set_offline(CARD_WDEV(card));
945926
rc3 = ccw_device_set_offline(CARD_RDEV(card));
@@ -974,7 +955,7 @@ static int qeth_l2_recover(void *ptr)
974955
dev_warn(&card->gdev->dev,
975956
"A recovery process has been started for the device\n");
976957
__qeth_l2_set_offline(card->gdev, 1);
977-
rc = __qeth_l2_set_online(card->gdev, 1);
958+
rc = qeth_l2_set_online(card->gdev);
978959
if (!rc)
979960
dev_info(&card->gdev->dev,
980961
"Device successfully recovered!\n");
@@ -1019,17 +1000,9 @@ static int qeth_l2_pm_suspend(struct ccwgroup_device *gdev)
10191000
static int qeth_l2_pm_resume(struct ccwgroup_device *gdev)
10201001
{
10211002
struct qeth_card *card = dev_get_drvdata(&gdev->dev);
1022-
int rc = 0;
1003+
int rc;
10231004

1024-
if (card->info.open_when_online) {
1025-
rc = __qeth_l2_set_online(card->gdev, 1);
1026-
if (rc) {
1027-
rtnl_lock();
1028-
dev_close(card->dev);
1029-
rtnl_unlock();
1030-
}
1031-
} else
1032-
rc = __qeth_l2_set_online(card->gdev, 0);
1005+
rc = qeth_l2_set_online(gdev);
10331006

10341007
qeth_set_allowed_threads(card, 0xffffffff, 0);
10351008
if (rc)

‎drivers/s390/net/qeth_l3_main.c

Lines changed: 10 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -1406,7 +1406,7 @@ static int qeth_l3_process_inbound_buffer(struct qeth_card *card,
14061406
return work_done;
14071407
}
14081408

1409-
static void qeth_l3_stop_card(struct qeth_card *card, int recovery_mode)
1409+
static void qeth_l3_stop_card(struct qeth_card *card)
14101410
{
14111411
QETH_DBF_TEXT(SETUP, 2, "stopcard");
14121412
QETH_DBF_HEX(SETUP, 2, &card, sizeof(void *));
@@ -1417,16 +1417,8 @@ static void qeth_l3_stop_card(struct qeth_card *card, int recovery_mode)
14171417
qeth_diags_trace(card, QETH_DIAGS_CMD_TRACE_DISABLE);
14181418
if (card->read.state == CH_STATE_UP &&
14191419
card->write.state == CH_STATE_UP &&
1420-
(card->state == CARD_STATE_UP)) {
1421-
if (recovery_mode)
1422-
qeth_stop(card->dev);
1423-
else {
1424-
rtnl_lock();
1425-
dev_close(card->dev);
1426-
rtnl_unlock();
1427-
}
1420+
card->state == CARD_STATE_UP)
14281421
card->state = CARD_STATE_SOFTSETUP;
1429-
}
14301422
if (card->state == CARD_STATE_SOFTSETUP) {
14311423
qeth_l3_clear_ip_htable(card, 1);
14321424
qeth_clear_ipacmd_list(card);
@@ -2299,7 +2291,7 @@ static void qeth_l3_remove_device(struct ccwgroup_device *cgdev)
22992291
qeth_l3_clear_ipato_list(card);
23002292
}
23012293

2302-
static int __qeth_l3_set_online(struct ccwgroup_device *gdev, int recovery_mode)
2294+
static int qeth_l3_set_online(struct ccwgroup_device *gdev)
23032295
{
23042296
struct qeth_card *card = dev_get_drvdata(&gdev->dev);
23052297
struct net_device *dev = card->dev;
@@ -2375,12 +2367,7 @@ static int __qeth_l3_set_online(struct ccwgroup_device *gdev, int recovery_mode)
23752367

23762368
if (card->info.open_when_online) {
23772369
card->info.open_when_online = 0;
2378-
if (recovery_mode) {
2379-
qeth_open(dev);
2380-
qeth_l3_set_rx_mode(dev);
2381-
} else {
2382-
dev_open(dev, NULL);
2383-
}
2370+
dev_open(dev, NULL);
23842371
}
23852372
rtnl_unlock();
23862373
}
@@ -2391,7 +2378,7 @@ static int __qeth_l3_set_online(struct ccwgroup_device *gdev, int recovery_mode)
23912378
mutex_unlock(&card->discipline_mutex);
23922379
return 0;
23932380
out_remove:
2394-
qeth_l3_stop_card(card, 0);
2381+
qeth_l3_stop_card(card);
23952382
ccw_device_set_offline(CARD_DDEV(card));
23962383
ccw_device_set_offline(CARD_WDEV(card));
23972384
ccw_device_set_offline(CARD_RDEV(card));
@@ -2403,11 +2390,6 @@ static int __qeth_l3_set_online(struct ccwgroup_device *gdev, int recovery_mode)
24032390
return rc;
24042391
}
24052392

2406-
static int qeth_l3_set_online(struct ccwgroup_device *gdev)
2407-
{
2408-
return __qeth_l3_set_online(gdev, 0);
2409-
}
2410-
24112393
static int __qeth_l3_set_offline(struct ccwgroup_device *cgdev,
24122394
int recovery_mode)
24132395
{
@@ -2426,11 +2408,12 @@ static int __qeth_l3_set_offline(struct ccwgroup_device *cgdev,
24262408

24272409
rtnl_lock();
24282410
card->info.open_when_online = card->dev->flags & IFF_UP;
2411+
dev_close(card->dev);
24292412
netif_device_detach(card->dev);
24302413
netif_carrier_off(card->dev);
24312414
rtnl_unlock();
24322415

2433-
qeth_l3_stop_card(card, recovery_mode);
2416+
qeth_l3_stop_card(card);
24342417
if ((card->options.cq == QETH_CQ_ENABLED) && card->dev) {
24352418
rtnl_lock();
24362419
call_netdevice_notifiers(NETDEV_REBOOT, card->dev);
@@ -2471,7 +2454,7 @@ static int qeth_l3_recover(void *ptr)
24712454
dev_warn(&card->gdev->dev,
24722455
"A recovery process has been started for the device\n");
24732456
__qeth_l3_set_offline(card->gdev, 1);
2474-
rc = __qeth_l3_set_online(card->gdev, 1);
2457+
rc = qeth_l3_set_online(card->gdev);
24752458
if (!rc)
24762459
dev_info(&card->gdev->dev,
24772460
"Device successfully recovered!\n");
@@ -2505,17 +2488,9 @@ static int qeth_l3_pm_suspend(struct ccwgroup_device *gdev)
25052488
static int qeth_l3_pm_resume(struct ccwgroup_device *gdev)
25062489
{
25072490
struct qeth_card *card = dev_get_drvdata(&gdev->dev);
2508-
int rc = 0;
2491+
int rc;
25092492

2510-
if (card->info.open_when_online) {
2511-
rc = __qeth_l3_set_online(card->gdev, 1);
2512-
if (rc) {
2513-
rtnl_lock();
2514-
dev_close(card->dev);
2515-
rtnl_unlock();
2516-
}
2517-
} else
2518-
rc = __qeth_l3_set_online(card->gdev, 0);
2493+
rc = qeth_l3_set_online(gdev);
25192494

25202495
qeth_set_allowed_threads(card, 0xffffffff, 0);
25212496
if (rc)

0 commit comments

Comments
 (0)