Skip to content

Commit 653bbf1

Browse files
davem330davem330
committed
Merge branch 'xen-netback'
Zoltan Kiss says: ==================== xen-netback: Fixing up xenvif_tx_check_gop This series fixes a lot of bugs on the error path around this function, which were introduced with my grant mapping series in 3.15. They apply to the latest net tree, but probably to net-next as well without any modification. I'll post an another series which applies to 3.15 stable, as the problem was first discovered there. The only difference is that the "queue" variable name is replaced to "vif". ==================== Signed-off-by: Zoltan Kiss <zoltan.kiss@citrix.com> Reported-by: Armin Zentai <armin.zentai@ezit.hu> Signed-off-by: David S. Miller <davem@davemloft.net>
2 parents 7801db8 + d8cfbfc commit 653bbf1

File tree

1 file changed

+63
-23
lines changed

1 file changed

+63
-23
lines changed

‎drivers/net/xen-netback/netback.c

Lines changed: 63 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -1030,23 +1030,34 @@ static int xenvif_tx_check_gop(struct xenvif_queue *queue,
10301030
{
10311031
struct gnttab_map_grant_ref *gop_map = *gopp_map;
10321032
u16 pending_idx = XENVIF_TX_CB(skb)->pending_idx;
1033+
/* This always points to the shinfo of the skb being checked, which
1034+
* could be either the first or the one on the frag_list
1035+
*/
10331036
struct skb_shared_info *shinfo = skb_shinfo(skb);
1037+
/* If this is non-NULL, we are currently checking the frag_list skb, and
1038+
* this points to the shinfo of the first one
1039+
*/
1040+
struct skb_shared_info *first_shinfo = NULL;
10341041
int nr_frags = shinfo->nr_frags;
1042+
const bool sharedslot = nr_frags &&
1043+
frag_get_pending_idx(&shinfo->frags[0]) == pending_idx;
10351044
int i, err;
1036-
struct sk_buff *first_skb = NULL;
10371045

10381046
/* Check status of header. */
10391047
err = (*gopp_copy)->status;
1040-
(*gopp_copy)++;
10411048
if (unlikely(err)) {
10421049
if (net_ratelimit())
10431050
netdev_dbg(queue->vif->dev,
10441051
"Grant copy of header failed! status: %d pending_idx: %u ref: %u\n",
10451052
(*gopp_copy)->status,
10461053
pending_idx,
10471054
(*gopp_copy)->source.u.ref);
1048-
xenvif_idx_release(queue, pending_idx, XEN_NETIF_RSP_ERROR);
1055+
/* The first frag might still have this slot mapped */
1056+
if (!sharedslot)
1057+
xenvif_idx_release(queue, pending_idx,
1058+
XEN_NETIF_RSP_ERROR);
10491059
}
1060+
(*gopp_copy)++;
10501061

10511062
check_frags:
10521063
for (i = 0; i < nr_frags; i++, gop_map++) {
@@ -1062,8 +1073,19 @@ static int xenvif_tx_check_gop(struct xenvif_queue *queue,
10621073
pending_idx,
10631074
gop_map->handle);
10641075
/* Had a previous error? Invalidate this fragment. */
1065-
if (unlikely(err))
1076+
if (unlikely(err)) {
10661077
xenvif_idx_unmap(queue, pending_idx);
1078+
/* If the mapping of the first frag was OK, but
1079+
* the header's copy failed, and they are
1080+
* sharing a slot, send an error
1081+
*/
1082+
if (i == 0 && sharedslot)
1083+
xenvif_idx_release(queue, pending_idx,
1084+
XEN_NETIF_RSP_ERROR);
1085+
else
1086+
xenvif_idx_release(queue, pending_idx,
1087+
XEN_NETIF_RSP_OKAY);
1088+
}
10671089
continue;
10681090
}
10691091

@@ -1075,42 +1097,53 @@ static int xenvif_tx_check_gop(struct xenvif_queue *queue,
10751097
gop_map->status,
10761098
pending_idx,
10771099
gop_map->ref);
1100+
10781101
xenvif_idx_release(queue, pending_idx, XEN_NETIF_RSP_ERROR);
10791102

10801103
/* Not the first error? Preceding frags already invalidated. */
10811104
if (err)
10821105
continue;
1083-
/* First error: invalidate preceding fragments. */
1106+
1107+
/* First error: if the header haven't shared a slot with the
1108+
* first frag, release it as well.
1109+
*/
1110+
if (!sharedslot)
1111+
xenvif_idx_release(queue,
1112+
XENVIF_TX_CB(skb)->pending_idx,
1113+
XEN_NETIF_RSP_OKAY);
1114+
1115+
/* Invalidate preceding fragments of this skb. */
10841116
for (j = 0; j < i; j++) {
10851117
pending_idx = frag_get_pending_idx(&shinfo->frags[j]);
10861118
xenvif_idx_unmap(queue, pending_idx);
1119+
xenvif_idx_release(queue, pending_idx,
1120+
XEN_NETIF_RSP_OKAY);
1121+
}
1122+
1123+
/* And if we found the error while checking the frag_list, unmap
1124+
* the first skb's frags
1125+
*/
1126+
if (first_shinfo) {
1127+
for (j = 0; j < first_shinfo->nr_frags; j++) {
1128+
pending_idx = frag_get_pending_idx(&first_shinfo->frags[j]);
1129+
xenvif_idx_unmap(queue, pending_idx);
1130+
xenvif_idx_release(queue, pending_idx,
1131+
XEN_NETIF_RSP_OKAY);
1132+
}
10871133
}
10881134

10891135
/* Remember the error: invalidate all subsequent fragments. */
10901136
err = newerr;
10911137
}
10921138

1093-
if (skb_has_frag_list(skb)) {
1094-
first_skb = skb;
1095-
skb = shinfo->frag_list;
1096-
shinfo = skb_shinfo(skb);
1139+
if (skb_has_frag_list(skb) && !first_shinfo) {
1140+
first_shinfo = skb_shinfo(skb);
1141+
shinfo = skb_shinfo(skb_shinfo(skb)->frag_list);
10971142
nr_frags = shinfo->nr_frags;
10981143

10991144
goto check_frags;
11001145
}
11011146

1102-
/* There was a mapping error in the frag_list skb. We have to unmap
1103-
* the first skb's frags
1104-
*/
1105-
if (first_skb && err) {
1106-
int j;
1107-
shinfo = skb_shinfo(first_skb);
1108-
for (j = 0; j < shinfo->nr_frags; j++) {
1109-
pending_idx = frag_get_pending_idx(&shinfo->frags[j]);
1110-
xenvif_idx_unmap(queue, pending_idx);
1111-
}
1112-
}
1113-
11141147
*gopp_map = gop_map;
11151148
return err;
11161149
}
@@ -1518,7 +1551,16 @@ static int xenvif_tx_submit(struct xenvif_queue *queue)
15181551

15191552
/* Check the remap error code. */
15201553
if (unlikely(xenvif_tx_check_gop(queue, skb, &gop_map, &gop_copy))) {
1554+
/* If there was an error, xenvif_tx_check_gop is
1555+
* expected to release all the frags which were mapped,
1556+
* so kfree_skb shouldn't do it again
1557+
*/
15211558
skb_shinfo(skb)->nr_frags = 0;
1559+
if (skb_has_frag_list(skb)) {
1560+
struct sk_buff *nskb =
1561+
skb_shinfo(skb)->frag_list;
1562+
skb_shinfo(nskb)->nr_frags = 0;
1563+
}
15221564
kfree_skb(skb);
15231565
continue;
15241566
}
@@ -1822,8 +1864,6 @@ void xenvif_idx_unmap(struct xenvif_queue *queue, u16 pending_idx)
18221864
tx_unmap_op.status);
18231865
BUG();
18241866
}
1825-
1826-
xenvif_idx_release(queue, pending_idx, XEN_NETIF_RSP_OKAY);
18271867
}
18281868

18291869
static inline int rx_work_todo(struct xenvif_queue *queue)

0 commit comments

Comments
 (0)