xen-netfront possibly rides the rocket too often
Subsystems (3)
We had reports about this message being seen on EC2 for a while but finally a reporter did notice some details about the guests and was able to provide a simple way to reproduce[1].
For my local experiments I use a Xen-4.2.2 based host (though I would say the host versions are not important). The host has one NIC which is used as the outgoing port of a Linux based (not openvswitch) bridge. And the PV guests use that bridge. I set the mtu to 9001 (which was seen on affected instance types) and also inside the guests. As described in the report one guests runs redis-server and the other nodejs through two scripts (for me I had to do the two sub.js calls in separate shells). After a bit the error messages appear on the guest running the redis-server.
I added some debug printk's to show a bit more detail about the skb and got the following (<length>@<offset (after masking off complete pages)>):
[ 698.108119] xen_netfront: xennet: skb rides the rocket: 19 slots [ 698.108134] header 1490@238 -> 1 slots [ 698.108139] frag #0 1614@2164 -> + 1 pages [ 698.108143] frag #1 3038@1296 -> + 2 pages [ 698.108147] frag #2 6076@1852 -> + 2 pages [ 698.108151] frag #3 6076@292 -> + 2 pages [ 698.108156] frag #4 6076@2828 -> + 3 pages [ 698.108160] frag #5 3038@1268 -> + 2 pages [ 698.108164] frag #6 2272@1824 -> + 1 pages [ 698.108168] frag #7 3804@0 -> + 1 pages [ 698.108172] frag #8 6076@264 -> + 2 pages [ 698.108177] frag #9 3946@2800 -> + 2 pages [ 698.108180] frags adding 18 slots
Since I am not deeply familiar with the networking code, I wonder about two things:
- is there something that should limit the skb data length from all frags to stay below the 64K which the definition of MAX_SKB_FRAGS hints?
- is multiple frags having offsets expected?
The latter is the problem here. If I did the maths right, the overall data size is around 41K. But since frags 1,4,5, and 9 have an offset big enough to require an additional page, the overall slot count goes up to 19.
If such a layout is valid, maybe the xen-netfront driver needs to reduce its XEN_NETIF_MAX_TX_SIZE which currently is set to 64K? Or something else...
-Stefan
[1] https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1317811
Quoted 30 lines · Stefan Bader
We had reports about this message being seen on EC2 for a while but finally a reporter did notice some details about the guests and was able to provide a simple way to reproduce[1].
For my local experiments I use a Xen-4.2.2 based host (though I would say the host versions are not important). The host has one NIC which is used as the outgoing port of a Linux based (not openvswitch) bridge. And the PV guests use that bridge. I set the mtu to 9001 (which was seen on affected instance types) and also inside the guests. As described in the report one guests runs redis-server and the other nodejs through two scripts (for me I had to do the two sub.js calls in separate shells). After a bit the error messages appear on the guest running the redis-server.
I added some debug printk's to show a bit more detail about the skb and got the following (<length>@<offset (after masking off complete pages)>):
[ 698.108119] xen_netfront: xennet: skb rides the rocket: 19 slots [ 698.108134] header 1490@238 -> 1 slots [ 698.108139] frag #0 1614@2164 -> + 1 pages [ 698.108143] frag #1 3038@1296 -> + 2 pages [ 698.108147] frag #2 6076@1852 -> + 2 pages [ 698.108151] frag #3 6076@292 -> + 2 pages [ 698.108156] frag #4 6076@2828 -> + 3 pages [ 698.108160] frag #5 3038@1268 -> + 2 pages [ 698.108164] frag #6 2272@1824 -> + 1 pages [ 698.108168] frag #7 3804@0 -> + 1 pages [ 698.108172] frag #8 6076@264 -> + 2 pages [ 698.108177] frag #9 3946@2800 -> + 2 pages [ 698.108180] frags adding 18 slots
Quoted 3 lines
Since I am not deeply familiar with the networking code, I wonder about two things:
- is there something that should limit the skb data length from all frags to stay below the 64K which the definition ofMAX_SKB_FRAGShints?
I think netfront should be able to handle 64K packets at most.
Quoted 1 line
- is multiple frags having offsets expected?
Yes, since compound pages a frag could over the 4K page boundary. The problem is, that in the netback/front protocol the assumption is that every slot is a single page, because grant operations could be done only on a 4K page. And every slot ends up as a frag (expect maybe the first, it can happen it is grant copied straight to the linear buffer), therefore the frontend cannot send an skb which occupies more than MAX_SKB_FRAGS individual 4k page. The problem is known for a while, the solution is not, unfortunately.
Quoted 12 lines
The latter is the problem here. If I did the maths right, the overall data size is around 41K. But since frags 1,4,5, and 9 have an offset big enough to require an additional page, the overall slot count goes up to 19.
If such a layout is valid, maybe the xen-netfront driver needs to reduce its
XEN_NETIF_MAX_TX_SIZEwhich currently is set to 64K? Or something else...-Stefan
[1] https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1317811
Quoted 7 lines · Zoltan Kiss
Quoted 4 lines · Stefan Bader
Since I am not deeply familiar with the networking code, I wonder about two things:
- is there something that should limit the skb data length from all frags to stay below the 64K which the definition ofMAX_SKB_FRAGShints?I think netfront should be able to handle 64K packets at most.
Quoted 1 line
- is multiple frags having offsets expected?
Quoted 8 lines
Yes, since compound pages a frag could over the 4K page boundary. The problem is, that in the netback/front protocol the assumption is that every slot is a single page, because grant operations could be done only on a 4K page. And every slot ends up as a frag (expect maybe the first, it can happen it is grant copied straight to the linear buffer), therefore the frontend cannot send an skb which occupies more than
MAX_SKB_FRAGSindividual 4k page.
The problem is known for a while, the solution is not, unfortunately.
I think the worst case scenario is when every frag and the linear buffer contains 2 bytes, which are overlapping a page boundary (that's (17+1)*2=36 so far), plus 15 of them have a 4k page in the middle of them, so, a 1+4096+1 byte buffer can span over 3 page. That's 51 individual pages. With the previous grant copy implementation there would be the option to modify backend and coalesce everything into a well formed skb. That would be a minor change there. But with grant mapping it's harder.
Slots of compound pages could be mapped to adjacent pages to Dom0, maybe somehow you can present them as compound pages in Dom0 as well. But in MFN space they wouldn't be contiguous, you need SWIOTLB or use IOMMU to hide that from the devices. Plus, what happens when you can't find adjacent pending slots? I think we would be better off at the moment with trying to compact these skbs a bit. Usually they overflow the limit by one or two, which means we should reallocate one or two frag, or the linear buffer to decrease the number of 4K pages used.
Zoli
Quoted 26 lines · you
Quoted 7 lines · Zoltan Kiss
Quoted 4 lines · Stefan Bader
Since I am not deeply familiar with the networking code, I wonder about two things:
- is there something that should limit the skb data length from all frags to stay below the 64K which the definition ofMAX_SKB_FRAGShints?I think netfront should be able to handle 64K packets at most.
Quoted 1 line
- is multiple frags having offsets expected?
Quoted 8 lines
Yes, since compound pages a frag could over the 4K page boundary. The problem is, that in the netback/front protocol the assumption is that every slot is a single page, because grant operations could be done only on a 4K page. And every slot ends up as a frag (expect maybe the first, it can happen it is grant copied straight to the linear buffer), therefore the frontend cannot send an skb which occupies more than
MAX_SKB_FRAGSindividual 4k page.
The problem is known for a while, the solution is not, unfortunately.I think the worst case scenario is when every frag and the linear buffer contains 2 bytes, which are overlapping a page boundary (that's (17+1)*2=36 so far), plus 15 of them have a 4k page in the middle of them, so, a 1+4096+1 byte buffer can span over 3 page. That's 51 individual pages.
With the previous grant copy implementation there would be the option to modify backend and coalesce everything into a well formed skb. That would be a minor change there. But with grant mapping it's harder. Slots of compound pages could be mapped to adjacent pages to Dom0, maybe
Quoted 8 lines
somehow you can present them as compound pages in Dom0 as well. But in MFN space they wouldn't be contiguous, you need SWIOTLB or use IOMMU to hide that from the devices. Plus, what happens when you can't find adjacent pending slots?
I think we would be better off at the moment with trying to compact these skbs a bit. Usually they overflow the limit by one or two, which means we should reallocate one or two frag, or the linear buffer to decrease the number of 4K pages used.
How does virtio-net handle this, the would probably have ran into the same problems ?
--
Sander
Quoted 1 line
Zoli
Quoted 38 lines · Sander Eikelenboom
Quoted 26 lines · you
Quoted 7 lines · Zoltan Kiss
Quoted 4 lines · Stefan Bader
Since I am not deeply familiar with the networking code, I wonder about two things:
- is there something that should limit the skb data length from all frags to stay below the 64K which the definition ofMAX_SKB_FRAGShints?I think netfront should be able to handle 64K packets at most.
Quoted 1 line
- is multiple frags having offsets expected?
Quoted 8 lines
Yes, since compound pages a frag could over the 4K page boundary. The problem is, that in the netback/front protocol the assumption is that every slot is a single page, because grant operations could be done only on a 4K page. And every slot ends up as a frag (expect maybe the first, it can happen it is grant copied straight to the linear buffer), therefore the frontend cannot send an skb which occupies more than
MAX_SKB_FRAGSindividual 4k page.
The problem is known for a while, the solution is not, unfortunately.I think the worst case scenario is when every frag and the linear buffer contains 2 bytes, which are overlapping a page boundary (that's (17+1)*2=36 so far), plus 15 of them have a 4k page in the middle of them, so, a 1+4096+1 byte buffer can span over 3 page. That's 51 individual pages.
With the previous grant copy implementation there would be the option to modify backend and coalesce everything into a well formed skb. That would be a minor change there. But with grant mapping it's harder. Slots of compound pages could be mapped to adjacent pages to Dom0, maybeQuoted 8 lines
somehow you can present them as compound pages in Dom0 as well. But in MFN space they wouldn't be contiguous, you need SWIOTLB or use IOMMU to hide that from the devices. Plus, what happens when you can't find adjacent pending slots?
I think we would be better off at the moment with trying to compact these skbs a bit. Usually they overflow the limit by one or two, which means we should reallocate one or two frag, or the linear buffer to decrease the number of 4K pages used.
Quoted 1 line
How does virtio-net handle this, the would probably have ran into the same problems ?
Maybe, though it seems not too many things cause this kind of traffic and then I could not say whether they have to handle a limited set of ring pages, too. But its something to keep in mind when digging deeper into it.
-Stefan
Quoted 14 lines
--
SanderQuoted 1 line
Zoli
_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xen.org/xen-devel
Quoted 38 lines · Sander Eikelenboom
Quoted 26 lines · you
Quoted 7 lines · Zoltan Kiss
Quoted 4 lines · Stefan Bader
Since I am not deeply familiar with the networking code, I wonder about two things:
- is there something that should limit the skb data length from all frags to stay below the 64K which the definition ofMAX_SKB_FRAGShints?I think netfront should be able to handle 64K packets at most.
Quoted 1 line
- is multiple frags having offsets expected?
Quoted 8 lines
Yes, since compound pages a frag could over the 4K page boundary. The problem is, that in the netback/front protocol the assumption is that every slot is a single page, because grant operations could be done only on a 4K page. And every slot ends up as a frag (expect maybe the first, it can happen it is grant copied straight to the linear buffer), therefore the frontend cannot send an skb which occupies more than
MAX_SKB_FRAGSindividual 4k page.
The problem is known for a while, the solution is not, unfortunately.I think the worst case scenario is when every frag and the linear buffer contains 2 bytes, which are overlapping a page boundary (that's (17+1)*2=36 so far), plus 15 of them have a 4k page in the middle of them, so, a 1+4096+1 byte buffer can span over 3 page. That's 51 individual pages.
With the previous grant copy implementation there would be the option to modify backend and coalesce everything into a well formed skb. That would be a minor change there. But with grant mapping it's harder. Slots of compound pages could be mapped to adjacent pages to Dom0, maybeQuoted 8 lines
somehow you can present them as compound pages in Dom0 as well. But in MFN space they wouldn't be contiguous, you need SWIOTLB or use IOMMU to hide that from the devices. Plus, what happens when you can't find adjacent pending slots?
I think we would be better off at the moment with trying to compact these skbs a bit. Usually they overflow the limit by one or two, which means we should reallocate one or two frag, or the linear buffer to decrease the number of 4K pages used.
Quoted 1 line
How does virtio-net handle this, the would probably have ran into the same problems ?
Maybe, though it seems not too many things cause this kind of traffic and then I could not say whether they have to handle a limited set of ring pages, too. But its something to keep in mind when digging deeper into it.
-Stefan
Quoted 9 lines
--
SanderQuoted 1 line
Zoli
Quoted 5 lines
_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xen.org/xen-devel
_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xen.org/xen-devel
Quoted 26 lines · you
Quoted 7 lines · Zoltan Kiss
Quoted 4 lines · Stefan Bader
Since I am not deeply familiar with the networking code, I wonder about two things:
- is there something that should limit the skb data length from all frags to stay below the 64K which the definition ofMAX_SKB_FRAGShints?I think netfront should be able to handle 64K packets at most.
Quoted 1 line
- is multiple frags having offsets expected?
Quoted 8 lines
Yes, since compound pages a frag could over the 4K page boundary. The problem is, that in the netback/front protocol the assumption is that every slot is a single page, because grant operations could be done only on a 4K page. And every slot ends up as a frag (expect maybe the first, it can happen it is grant copied straight to the linear buffer), therefore the frontend cannot send an skb which occupies more than
MAX_SKB_FRAGSindividual 4k page.
The problem is known for a while, the solution is not, unfortunately.I think the worst case scenario is when every frag and the linear buffer contains 2 bytes, which are overlapping a page boundary (that's (17+1)*2=36 so far), plus 15 of them have a 4k page in the middle of them, so, a 1+4096+1 byte buffer can span over 3 page. That's 51 individual pages.
With the previous grant copy implementation there would be the option to modify backend and coalesce everything into a well formed skb. That would be a minor change there. But with grant mapping it's harder. Slots of compound pages could be mapped to adjacent pages to Dom0, maybe
Quoted 8 lines
somehow you can present them as compound pages in Dom0 as well. But in MFN space they wouldn't be contiguous, you need SWIOTLB or use IOMMU to hide that from the devices. Plus, what happens when you can't find adjacent pending slots?
I think we would be better off at the moment with trying to compact these skbs a bit. Usually they overflow the limit by one or two, which means we should reallocate one or two frag, or the linear buffer to decrease the number of 4K pages used.
How does virtio-net handle this, the would probably have ran into the same problems ?
--
Sander
Quoted 1 line
Zoli
Quoted 7 lines · Zoltan Kiss
Quoted 4 lines · Stefan Bader
Since I am not deeply familiar with the networking code, I wonder about two things:
- is there something that should limit the skb data length from all frags to stay below the 64K which the definition ofMAX_SKB_FRAGShints?I think netfront should be able to handle 64K packets at most.
Quoted 1 line
- is multiple frags having offsets expected?
Quoted 8 lines
Yes, since compound pages a frag could over the 4K page boundary. The problem is, that in the netback/front protocol the assumption is that every slot is a single page, because grant operations could be done only on a 4K page. And every slot ends up as a frag (expect maybe the first, it can happen it is grant copied straight to the linear buffer), therefore the frontend cannot send an skb which occupies more than
MAX_SKB_FRAGSindividual 4k page.
The problem is known for a while, the solution is not, unfortunately.
I think the worst case scenario is when every frag and the linear buffer contains 2 bytes, which are overlapping a page boundary (that's (17+1)*2=36 so far), plus 15 of them have a 4k page in the middle of them, so, a 1+4096+1 byte buffer can span over 3 page. That's 51 individual pages. With the previous grant copy implementation there would be the option to modify backend and coalesce everything into a well formed skb. That would be a minor change there. But with grant mapping it's harder.
Slots of compound pages could be mapped to adjacent pages to Dom0, maybe somehow you can present them as compound pages in Dom0 as well. But in MFN space they wouldn't be contiguous, you need SWIOTLB or use IOMMU to hide that from the devices. Plus, what happens when you can't find adjacent pending slots? I think we would be better off at the moment with trying to compact these skbs a bit. Usually they overflow the limit by one or two, which means we should reallocate one or two frag, or the linear buffer to decrease the number of 4K pages used.
Zoli
Ian Campbell
Acked-byQuoted 31 lines · Zoltan Kiss
Quoted 30 lines · Stefan Bader
We had reports about this message being seen on EC2 for a while but finally a reporter did notice some details about the guests and was able to provide a simple way to reproduce[1].
For my local experiments I use a Xen-4.2.2 based host (though I would say the host versions are not important). The host has one NIC which is used as the outgoing port of a Linux based (not openvswitch) bridge. And the PV guests use that bridge. I set the mtu to 9001 (which was seen on affected instance types) and also inside the guests. As described in the report one guests runs redis-server and the other nodejs through two scripts (for me I had to do the two sub.js calls in separate shells). After a bit the error messages appear on the guest running the redis-server.
I added some debug printk's to show a bit more detail about the skb and got the following (<length>@<offset (after masking off complete pages)>):
[ 698.108119] xen_netfront: xennet: skb rides the rocket: 19 slots [ 698.108134] header 1490@238 -> 1 slots [ 698.108139] frag #0 1614@2164 -> + 1 pages [ 698.108143] frag #1 3038@1296 -> + 2 pages [ 698.108147] frag #2 6076@1852 -> + 2 pages [ 698.108151] frag #3 6076@292 -> + 2 pages [ 698.108156] frag #4 6076@2828 -> + 3 pages [ 698.108160] frag #5 3038@1268 -> + 2 pages [ 698.108164] frag #6 2272@1824 -> + 1 pages [ 698.108168] frag #7 3804@0 -> + 1 pages [ 698.108172] frag #8 6076@264 -> + 2 pages [ 698.108177] frag #9 3946@2800 -> + 2 pages [ 698.108180] frags adding 18 slots
Quoted 4 lines
Quoted 3 lines
Since I am not deeply familiar with the networking code, I wonder about two things:
- is there something that should limit the skb data length from all frags to stay below the 64K which the definition ofMAX_SKB_FRAGShints?I think netfront should be able to handle 64K packets at most.
Ah, maybe this relates to this fix from Wei?
commit 9ecd1a75d977e2e8c48139c7d3efed183f898d94
Author: Wei Liu <[email protected]> Date: Mon Apr 22 02:20:41 2013 +0000
xen-netfront: reduce gso_max_size to account for max TCP header
The maximum packet including header that can be handled by netfront / netback wire format is 65535. Reduce gso_max_size accordingly.
Drop skb and print warning when skb->len > 65535. This can 1) save the effort to send malformed packet to netback, 2) help spotting misconfiguration of netfront in the future.
drivers/net/xen-netfront.c +15 -2
@@ -36,7 +36,7 @@#include <linux/skbuff.h> #include <linux/ethtool.h> #include <linux/if_ether.h> -#include <linux/tcp.h> +#include <net/tcp.h> #include <linux/udp.h> #include <linux/moduleparam.h> #include <linux/mm.h>
@@ -547,6 +547,16 @@ static int xennet_start_xmit(struct sk_buff *skb, struct net_device *dev)unsigned int len =skb_headlen (skb);unsigned long flags; +/* If skb->len is too big for wire format, drop skb and alert +* user about misconfiguration. +*/ +if (unlikely (skb->len >XEN_NETIF_MAX_TX_SIZE )) { +net_alert_ratelimited ( +"xennet: skb->len = %u, too big for wire format\n" , + skb->len); + goto drop; + } + slots =DIV_ROUND_UP (offset + len,PAGE_SIZE ) +xennet_count_skb_frag_slots (skb);if (unlikely (slots >MAX_SKB_FRAGS +1 )) {
@@ -1058,7 +1068,8 @@ err:static int xennet_change_mtu (struct net_device *dev,int mtu) { -int max =xennet_can_sg (dev) ?65535 -ETH_HLEN :ETH_DATA_LEN ; +int max =xennet_can_sg (dev) ? +XEN_NETIF_MAX_TX_SIZE -MAX_TCP_HEADER :ETH_DATA_LEN ;if (mtu > max)return -EINVAL ;
@@ -1362,6 +1373,8 @@ static struct net_device *xennet_create_dev(struct xenbus_device *dev)SET_ETHTOOL_OPS (netdev, &xennet_ethtool_ops);SET_NETDEV_DEV (netdev, &dev->dev); +netif_set_gso_max_size (netdev,XEN_NETIF_MAX_TX_SIZE -MAX_TCP_HEADER ); + np->netdev = netdev;netif_carrier_off (netdev);
include/xen/interface/io/netif.h +1 -0
@@ -47,6 +47,7 @@#define _XEN_NETTXF_extra_info (3)#define XEN_NETTXF_extra_info (1U<<_XEN_NETTXF_extra_info) +#define XEN_NETIF_MAX_TX_SIZE 0xFFFFstruct xen_netif_tx_request {grant_ref_t gref;/* Reference to buffer page */ uint16_t offset;/* Offset within buffer page */
Quoted 32 lines · Ian Campbell
Quoted 31 lines · Zoltan Kiss
Quoted 30 lines · Stefan Bader
We had reports about this message being seen on EC2 for a while but finally a reporter did notice some details about the guests and was able to provide a simple way to reproduce[1].
For my local experiments I use a Xen-4.2.2 based host (though I would say the host versions are not important). The host has one NIC which is used as the outgoing port of a Linux based (not openvswitch) bridge. And the PV guests use that bridge. I set the mtu to 9001 (which was seen on affected instance types) and also inside the guests. As described in the report one guests runs redis-server and the other nodejs through two scripts (for me I had to do the two sub.js calls in separate shells). After a bit the error messages appear on the guest running the redis-server.
I added some debug printk's to show a bit more detail about the skb and got the following (<length>@<offset (after masking off complete pages)>):
[ 698.108119] xen_netfront: xennet: skb rides the rocket: 19 slots [ 698.108134] header 1490@238 -> 1 slots [ 698.108139] frag #0 1614@2164 -> + 1 pages [ 698.108143] frag #1 3038@1296 -> + 2 pages [ 698.108147] frag #2 6076@1852 -> + 2 pages [ 698.108151] frag #3 6076@292 -> + 2 pages [ 698.108156] frag #4 6076@2828 -> + 3 pages [ 698.108160] frag #5 3038@1268 -> + 2 pages [ 698.108164] frag #6 2272@1824 -> + 1 pages [ 698.108168] frag #7 3804@0 -> + 1 pages [ 698.108172] frag #8 6076@264 -> + 2 pages [ 698.108177] frag #9 3946@2800 -> + 2 pages [ 698.108180] frags adding 18 slots
Quoted 6 lines
Quoted 4 lines
Quoted 3 lines
Since I am not deeply familiar with the networking code, I wonder about two things:
- is there something that should limit the skb data length from all frags to stay below the 64K which the definition ofMAX_SKB_FRAGShints?I think netfront should be able to handle 64K packets at most.
Ah, maybe this relates to this fix from Wei?
That indeed should (imo) limit the overall size. And if that would happen still, we would see the different message. The problem I see is not the overall size but the layout of the frags. Since multiple start at an offset high enough, the count of slots goes too high.
Now I have to read that code in more detail, but there also has been some changes which introduce a make frags function. I wonder whether maybe there is already some kind of shifting (or copying) going on and whether its just the check that needs to improve. But right now that is speculation.
For the fun I actually found an even simpler way to trigger it and (need to confirm it yet without mucking around with mtu before). It looked like mtu actually did not make a difference. One only needed the redis server on one guest and run redis-benchmark from the other with (I think -d 1000, or whatever it is that sets the datasize). Then on the range tests this happens quite often.
-Stefan
Quoted 87 lines
commit
9ecd1a75d977e2e8c48139c7d3efed183f898d94Author: Wei Liu <[email protected]> Date: Mon Apr 22 02:20:41 2013 +0000xen-netfront: reduce
gso_max_sizeto account for max TCP headerThe maximum packet including header that can be handled by netfront / netback wire format is 65535. Reduce
gso_max_sizeaccordingly.Drop skb and print warning when
skb->len> 65535. This can 1) save the effort to send malformed packet to netback, 2) help spotting misconfiguration of netfront in the future.Signed-off-by Wei Liu <[email protected]>Acked-by Ian Campbell <[email protected]>Signed-off-by David S. Miller <[email protected]>drivers/net/xen-netfront.c +15
-2@@ -36,7 +36,7 @@#include <linux/skbuff.h> #include <linux/ethtool.h> #include <linux/if_ether.h> -#include <linux/tcp.h> +#include <net/tcp.h> #include <linux/udp.h> #include <linux/moduleparam.h> #include <linux/mm.h> @@ -547,6 +547,16 @@ static int xennet_start_xmit(struct sk_buff *skb, struct net_device *dev)unsigned int len =skb_headlen (skb);unsigned long flags; +/* If skb->len is too big for wire format, drop skb and alert +* user about misconfiguration. +*/ +if (unlikely (skb->len >XEN_NETIF_MAX_TX_SIZE )) { +net_alert_ratelimited ( +"xennet: skb->len = %u, too big for wire format\n" , + skb->len); + goto drop; + } + slots =DIV_ROUND_UP (offset + len,PAGE_SIZE ) +xennet_count_skb_frag_slots (skb);if (unlikely (slots >MAX_SKB_FRAGS +1 )) {@@ -1058,7 +1068,8 @@ err:static int xennet_change_mtu (struct net_device *dev,int mtu) { -int max =xennet_can_sg (dev) ?65535 -ETH_HLEN :ETH_DATA_LEN ; +int max =xennet_can_sg (dev) ? +XEN_NETIF_MAX_TX_SIZE -MAX_TCP_HEADER :ETH_DATA_LEN ;if (mtu > max)return -EINVAL ;@@ -1362,6 +1373,8 @@ static struct net_device *xennet_create_dev(struct xenbus_device *dev)SET_ETHTOOL_OPS (netdev, &xennet_ethtool_ops);SET_NETDEV_DEV (netdev, &dev->dev); +netif_set_gso_max_size (netdev,XEN_NETIF_MAX_TX_SIZE -MAX_TCP_HEADER ); + np->netdev = netdev;netif_carrier_off (netdev);include/xen/interface/io/netif.h +1
-0@@ -47,6 +47,7 @@#define _XEN_NETTXF_extra_info (3)#define XEN_NETTXF_extra_info (1U<<_XEN_NETTXF_extra_info) +#define XEN_NETIF_MAX_TX_SIZE 0xFFFFstruct xen_netif_tx_request {grant_ref_t gref;/* Reference to buffer page */ uint16_t offset;/* Offset within buffer page */ _______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xen.org/xen-devel
Quoted 45 lines · you
Quoted 32 lines · Ian Campbell
Quoted 31 lines · Zoltan Kiss
Quoted 30 lines · Stefan Bader
We had reports about this message being seen on EC2 for a while but finally a reporter did notice some details about the guests and was able to provide a simple way to reproduce[1].
For my local experiments I use a Xen-4.2.2 based host (though I would say the host versions are not important). The host has one NIC which is used as the outgoing port of a Linux based (not openvswitch) bridge. And the PV guests use that bridge. I set the mtu to 9001 (which was seen on affected instance types) and also inside the guests. As described in the report one guests runs redis-server and the other nodejs through two scripts (for me I had to do the two sub.js calls in separate shells). After a bit the error messages appear on the guest running the redis-server.
I added some debug printk's to show a bit more detail about the skb and got the following (<length>@<offset (after masking off complete pages)>):
[ 698.108119] xen_netfront: xennet: skb rides the rocket: 19 slots [ 698.108134] header 1490@238 -> 1 slots [ 698.108139] frag #0 1614@2164 -> + 1 pages [ 698.108143] frag #1 3038@1296 -> + 2 pages [ 698.108147] frag #2 6076@1852 -> + 2 pages [ 698.108151] frag #3 6076@292 -> + 2 pages [ 698.108156] frag #4 6076@2828 -> + 3 pages [ 698.108160] frag #5 3038@1268 -> + 2 pages [ 698.108164] frag #6 2272@1824 -> + 1 pages [ 698.108168] frag #7 3804@0 -> + 1 pages [ 698.108172] frag #8 6076@264 -> + 2 pages [ 698.108177] frag #9 3946@2800 -> + 2 pages [ 698.108180] frags adding 18 slotsQuoted 6 lines
Quoted 4 lines
Quoted 3 lines
Since I am not deeply familiar with the networking code, I wonder about two things:
- is there something that should limit the skb data length from all frags to stay below the 64K which the definition ofMAX_SKB_FRAGShints?I think netfront should be able to handle 64K packets at most.
Ah, maybe this relates to this fix from Wei?
That indeed should (imo) limit the overall size. And if that would happen still, we would see the different message. The problem I see is not the overall size but the layout of the frags. Since multiple start at an offset high enough, the count of slots goes too high.
Quoted 4 lines
Now I have to read that code in more detail, but there also has been some changes which introduce a make frags function. I wonder whether maybe there is already some kind of shifting (or copying) going on and whether its just the check that needs to improve. But right now that is speculation.
Netback seems to be able to cope with it, but properly calculating and checking the needed slots proved difficult (see discussion with Paul Durrant for 3.14).
Quoted 1 line
From my tests there it seemed that max slots was capped to
MAX_SKB_FRAGS.
I assume that could be explained by packets having a max size, so if there are frags with offsets that need more pages per frags, that is counterbalanced .. since the total packet size is already limited .. so it would have to have less frags.
That's at least what the netback code is using now as a calculation method for the required slots, pessimistic with a cap on MAX_SKB_FRAGS.
But i'm not 100% sure the assumption is valid (by backing it theoretically).
Quoted 94 lines
For the fun I actually found an even simpler way to trigger it and (need to confirm it yet without mucking around with mtu before). It looked like mtu actually did not make a difference. One only needed the redis server on one guest and run redis-benchmark from the other with (I think -d 1000, or whatever it is that sets the datasize). Then on the range tests this happens quite often.
-Stefan
Quoted 87 lines
commit
9ecd1a75d977e2e8c48139c7d3efed183f898d94Author: Wei Liu <[email protected]> Date: Mon Apr 22 02:20:41 2013 +0000xen-netfront: reduce
gso_max_sizeto account for max TCP headerThe maximum packet including header that can be handled by netfront / netback wire format is 65535. Reduce
gso_max_sizeaccordingly.Drop skb and print warning when
skb->len> 65535. This can 1) save the effort to send malformed packet to netback, 2) help spotting misconfiguration of netfront in the future.Signed-off-by Wei Liu <[email protected]>Acked-by Ian Campbell <[email protected]>Signed-off-by David S. Miller <[email protected]>drivers/net/xen-netfront.c +15
-2@@ -36,7 +36,7 @@#include <linux/skbuff.h> #include <linux/ethtool.h> #include <linux/if_ether.h> -#include <linux/tcp.h> +#include <net/tcp.h> #include <linux/udp.h> #include <linux/moduleparam.h> #include <linux/mm.h> @@ -547,6 +547,16 @@ static int xennet_start_xmit(struct sk_buff *skb, struct net_device *dev)unsigned int len =skb_headlen (skb);unsigned long flags; +/* If skb->len is too big for wire format, drop skb and alert +* user about misconfiguration. +*/ +if (unlikely (skb->len >XEN_NETIF_MAX_TX_SIZE )) { +net_alert_ratelimited ( +"xennet: skb->len = %u, too big for wire format\n" , + skb->len); + goto drop; + } + slots =DIV_ROUND_UP (offset + len,PAGE_SIZE ) +xennet_count_skb_frag_slots (skb);if (unlikely (slots >MAX_SKB_FRAGS +1 )) {@@ -1058,7 +1068,8 @@ err:static int xennet_change_mtu (struct net_device *dev,int mtu) { -int max =xennet_can_sg (dev) ?65535 -ETH_HLEN :ETH_DATA_LEN ; +int max =xennet_can_sg (dev) ? +XEN_NETIF_MAX_TX_SIZE -MAX_TCP_HEADER :ETH_DATA_LEN ;if (mtu > max)return -EINVAL ;@@ -1362,6 +1373,8 @@ static struct net_device *xennet_create_dev(struct xenbus_device *dev)SET_ETHTOOL_OPS (netdev, &xennet_ethtool_ops);SET_NETDEV_DEV (netdev, &dev->dev); +netif_set_gso_max_size (netdev,XEN_NETIF_MAX_TX_SIZE -MAX_TCP_HEADER ); + np->netdev = netdev;netif_carrier_off (netdev);include/xen/interface/io/netif.h +1
-0@@ -47,6 +47,7 @@#define _XEN_NETTXF_extra_info (3)#define XEN_NETTXF_extra_info (1U<<_XEN_NETTXF_extra_info) +#define XEN_NETIF_MAX_TX_SIZE 0xFFFFstruct xen_netif_tx_request {grant_ref_t gref;/* Reference to buffer page */ uint16_t offset;/* Offset within buffer page */ _______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xen.org/xen-devel
Quoted 45 lines · you
Quoted 32 lines · Ian Campbell
Quoted 31 lines · Zoltan Kiss
Quoted 30 lines · Stefan Bader
We had reports about this message being seen on EC2 for a while but finally a reporter did notice some details about the guests and was able to provide a simple way to reproduce[1].
For my local experiments I use a Xen-4.2.2 based host (though I would say the host versions are not important). The host has one NIC which is used as the outgoing port of a Linux based (not openvswitch) bridge. And the PV guests use that bridge. I set the mtu to 9001 (which was seen on affected instance types) and also inside the guests. As described in the report one guests runs redis-server and the other nodejs through two scripts (for me I had to do the two sub.js calls in separate shells). After a bit the error messages appear on the guest running the redis-server.
I added some debug printk's to show a bit more detail about the skb and got the following (<length>@<offset (after masking off complete pages)>):
[ 698.108119] xen_netfront: xennet: skb rides the rocket: 19 slots [ 698.108134] header 1490@238 -> 1 slots [ 698.108139] frag #0 1614@2164 -> + 1 pages [ 698.108143] frag #1 3038@1296 -> + 2 pages [ 698.108147] frag #2 6076@1852 -> + 2 pages [ 698.108151] frag #3 6076@292 -> + 2 pages [ 698.108156] frag #4 6076@2828 -> + 3 pages [ 698.108160] frag #5 3038@1268 -> + 2 pages [ 698.108164] frag #6 2272@1824 -> + 1 pages [ 698.108168] frag #7 3804@0 -> + 1 pages [ 698.108172] frag #8 6076@264 -> + 2 pages [ 698.108177] frag #9 3946@2800 -> + 2 pages [ 698.108180] frags adding 18 slotsQuoted 6 lines
Quoted 4 lines
Quoted 3 lines
Since I am not deeply familiar with the networking code, I wonder about two things:
- is there something that should limit the skb data length from all frags to stay below the 64K which the definition ofMAX_SKB_FRAGShints?I think netfront should be able to handle 64K packets at most.
Ah, maybe this relates to this fix from Wei?
That indeed should (imo) limit the overall size. And if that would happen still, we would see the different message. The problem I see is not the overall size but the layout of the frags. Since multiple start at an offset high enough, the count of slots goes too high.
Quoted 4 lines
Now I have to read that code in more detail, but there also has been some changes which introduce a make frags function. I wonder whether maybe there is already some kind of shifting (or copying) going on and whether its just the check that needs to improve. But right now that is speculation.
Netback seems to be able to cope with it, but properly calculating and checking the needed slots proved difficult (see discussion with Paul Durrant for 3.14).
Quoted 1 line
From my tests there it seemed that max slots was capped to
MAX_SKB_FRAGS.
I assume that could be explained by packets having a max size, so if there are frags with offsets that need more pages per frags, that is counterbalanced .. since the total packet size is already limited .. so it would have to have less frags.
That's at least what the netback code is using now as a calculation method for the required slots, pessimistic with a cap on MAX_SKB_FRAGS.
But i'm not 100% sure the assumption is valid (by backing it theoretically).
Quoted 94 lines
For the fun I actually found an even simpler way to trigger it and (need to confirm it yet without mucking around with mtu before). It looked like mtu actually did not make a difference. One only needed the redis server on one guest and run redis-benchmark from the other with (I think -d 1000, or whatever it is that sets the datasize). Then on the range tests this happens quite often.
-Stefan
Quoted 87 lines
commit
9ecd1a75d977e2e8c48139c7d3efed183f898d94Author: Wei Liu <[email protected]> Date: Mon Apr 22 02:20:41 2013 +0000xen-netfront: reduce
gso_max_sizeto account for max TCP headerThe maximum packet including header that can be handled by netfront / netback wire format is 65535. Reduce
gso_max_sizeaccordingly.Drop skb and print warning when
skb->len> 65535. This can 1) save the effort to send malformed packet to netback, 2) help spotting misconfiguration of netfront in the future.Signed-off-by Wei Liu <[email protected]>Acked-by Ian Campbell <[email protected]>Signed-off-by David S. Miller <[email protected]>drivers/net/xen-netfront.c +15
-2@@ -36,7 +36,7 @@#include <linux/skbuff.h> #include <linux/ethtool.h> #include <linux/if_ether.h> -#include <linux/tcp.h> +#include <net/tcp.h> #include <linux/udp.h> #include <linux/moduleparam.h> #include <linux/mm.h> @@ -547,6 +547,16 @@ static int xennet_start_xmit(struct sk_buff *skb, struct net_device *dev)unsigned int len =skb_headlen (skb);unsigned long flags; +/* If skb->len is too big for wire format, drop skb and alert +* user about misconfiguration. +*/ +if (unlikely (skb->len >XEN_NETIF_MAX_TX_SIZE )) { +net_alert_ratelimited ( +"xennet: skb->len = %u, too big for wire format\n" , + skb->len); + goto drop; + } + slots =DIV_ROUND_UP (offset + len,PAGE_SIZE ) +xennet_count_skb_frag_slots (skb);if (unlikely (slots >MAX_SKB_FRAGS +1 )) {@@ -1058,7 +1068,8 @@ err:static int xennet_change_mtu (struct net_device *dev,int mtu) { -int max =xennet_can_sg (dev) ?65535 -ETH_HLEN :ETH_DATA_LEN ; +int max =xennet_can_sg (dev) ? +XEN_NETIF_MAX_TX_SIZE -MAX_TCP_HEADER :ETH_DATA_LEN ;if (mtu > max)return -EINVAL ;@@ -1362,6 +1373,8 @@ static struct net_device *xennet_create_dev(struct xenbus_device *dev)SET_ETHTOOL_OPS (netdev, &xennet_ethtool_ops);SET_NETDEV_DEV (netdev, &dev->dev); +netif_set_gso_max_size (netdev,XEN_NETIF_MAX_TX_SIZE -MAX_TCP_HEADER ); + np->netdev = netdev;netif_carrier_off (netdev);include/xen/interface/io/netif.h +1
-0@@ -47,6 +47,7 @@#define _XEN_NETTXF_extra_info (3)#define XEN_NETTXF_extra_info (1U<<_XEN_NETTXF_extra_info) +#define XEN_NETIF_MAX_TX_SIZE 0xFFFFstruct xen_netif_tx_request {grant_ref_t gref;/* Reference to buffer page */ uint16_t offset;/* Offset within buffer page */ _______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xen.org/xen-devel
Quoted 32 lines · Ian Campbell
Quoted 31 lines · Zoltan Kiss
Quoted 30 lines · Stefan Bader
We had reports about this message being seen on EC2 for a while but finally a reporter did notice some details about the guests and was able to provide a simple way to reproduce[1].
For my local experiments I use a Xen-4.2.2 based host (though I would say the host versions are not important). The host has one NIC which is used as the outgoing port of a Linux based (not openvswitch) bridge. And the PV guests use that bridge. I set the mtu to 9001 (which was seen on affected instance types) and also inside the guests. As described in the report one guests runs redis-server and the other nodejs through two scripts (for me I had to do the two sub.js calls in separate shells). After a bit the error messages appear on the guest running the redis-server.
I added some debug printk's to show a bit more detail about the skb and got the following (<length>@<offset (after masking off complete pages)>):
[ 698.108119] xen_netfront: xennet: skb rides the rocket: 19 slots [ 698.108134] header 1490@238 -> 1 slots [ 698.108139] frag #0 1614@2164 -> + 1 pages [ 698.108143] frag #1 3038@1296 -> + 2 pages [ 698.108147] frag #2 6076@1852 -> + 2 pages [ 698.108151] frag #3 6076@292 -> + 2 pages [ 698.108156] frag #4 6076@2828 -> + 3 pages [ 698.108160] frag #5 3038@1268 -> + 2 pages [ 698.108164] frag #6 2272@1824 -> + 1 pages [ 698.108168] frag #7 3804@0 -> + 1 pages [ 698.108172] frag #8 6076@264 -> + 2 pages [ 698.108177] frag #9 3946@2800 -> + 2 pages [ 698.108180] frags adding 18 slots
Quoted 6 lines
Quoted 4 lines
Quoted 3 lines
Since I am not deeply familiar with the networking code, I wonder about two things:
- is there something that should limit the skb data length from all frags to stay below the 64K which the definition ofMAX_SKB_FRAGShints?I think netfront should be able to handle 64K packets at most.
Ah, maybe this relates to this fix from Wei?
That indeed should (imo) limit the overall size. And if that would happen still, we would see the different message. The problem I see is not the overall size but the layout of the frags. Since multiple start at an offset high enough, the count of slots goes too high.
Now I have to read that code in more detail, but there also has been some changes which introduce a make frags function. I wonder whether maybe there is already some kind of shifting (or copying) going on and whether its just the check that needs to improve. But right now that is speculation.
For the fun I actually found an even simpler way to trigger it and (need to confirm it yet without mucking around with mtu before). It looked like mtu actually did not make a difference. One only needed the redis server on one guest and run redis-benchmark from the other with (I think -d 1000, or whatever it is that sets the datasize). Then on the range tests this happens quite often.
-Stefan
Quoted 82 lines
commit
9ecd1a75d977e2e8c48139c7d3efed183f898d94Author: Wei Liu <[email protected]> Date: Mon Apr 22 02:20:41 2013 +0000xen-netfront: reduce
gso_max_sizeto account for max TCP headerThe maximum packet including header that can be handled by netfront / netback wire format is 65535. Reduce
gso_max_sizeaccordingly.Drop skb and print warning when
skb->len> 65535. This can 1) save the effort to send malformed packet to netback, 2) help spotting misconfiguration of netfront in the future.Signed-off-by Wei Liu <[email protected]>Acked-by Ian Campbell <[email protected]>Signed-off-by David S. Miller <[email protected]>drivers/net/xen-netfront.c +15
-2@@ -36,7 +36,7 @@#include <linux/skbuff.h> #include <linux/ethtool.h> #include <linux/if_ether.h> -#include <linux/tcp.h> +#include <net/tcp.h> #include <linux/udp.h> #include <linux/moduleparam.h> #include <linux/mm.h> @@ -547,6 +547,16 @@ static int xennet_start_xmit(struct sk_buff *skb, struct net_device *dev)unsigned int len =skb_headlen (skb);unsigned long flags; +/* If skb->len is too big for wire format, drop skb and alert +* user about misconfiguration. +*/ +if (unlikely (skb->len >XEN_NETIF_MAX_TX_SIZE )) { +net_alert_ratelimited ( +"xennet: skb->len = %u, too big for wire format\n" , + skb->len); + goto drop; + } + slots =DIV_ROUND_UP (offset + len,PAGE_SIZE ) +xennet_count_skb_frag_slots (skb);if (unlikely (slots >MAX_SKB_FRAGS +1 )) {@@ -1058,7 +1068,8 @@ err:static int xennet_change_mtu (struct net_device *dev,int mtu) { -int max =xennet_can_sg (dev) ?65535 -ETH_HLEN :ETH_DATA_LEN ; +int max =xennet_can_sg (dev) ? +XEN_NETIF_MAX_TX_SIZE -MAX_TCP_HEADER :ETH_DATA_LEN ;if (mtu > max)return -EINVAL ;@@ -1362,6 +1373,8 @@ static struct net_device *xennet_create_dev(struct xenbus_device *dev)SET_ETHTOOL_OPS (netdev, &xennet_ethtool_ops);SET_NETDEV_DEV (netdev, &dev->dev); +netif_set_gso_max_size (netdev,XEN_NETIF_MAX_TX_SIZE -MAX_TCP_HEADER ); + np->netdev = netdev;netif_carrier_off (netdev);include/xen/interface/io/netif.h +1
-0@@ -47,6 +47,7 @@#define _XEN_NETTXF_extra_info (3)#define XEN_NETTXF_extra_info (1U<<_XEN_NETTXF_extra_info) +#define XEN_NETIF_MAX_TX_SIZE 0xFFFFstruct xen_netif_tx_request {grant_ref_t gref;/* Reference to buffer page */ uint16_t offset;/* Offset within buffer page */
Quoted 5 lines
_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xen.org/xen-devel
_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xen.org/xen-devel
Quoted 32 lines · Ian Campbell
Quoted 31 lines · Zoltan Kiss
Quoted 30 lines · Stefan Bader
We had reports about this message being seen on EC2 for a while but finally a reporter did notice some details about the guests and was able to provide a simple way to reproduce[1].
For my local experiments I use a Xen-4.2.2 based host (though I would say the host versions are not important). The host has one NIC which is used as the outgoing port of a Linux based (not openvswitch) bridge. And the PV guests use that bridge. I set the mtu to 9001 (which was seen on affected instance types) and also inside the guests. As described in the report one guests runs redis-server and the other nodejs through two scripts (for me I had to do the two sub.js calls in separate shells). After a bit the error messages appear on the guest running the redis-server.
I added some debug printk's to show a bit more detail about the skb and got the following (<length>@<offset (after masking off complete pages)>):
[ 698.108119] xen_netfront: xennet: skb rides the rocket: 19 slots [ 698.108134] header 1490@238 -> 1 slots [ 698.108139] frag #0 1614@2164 -> + 1 pages [ 698.108143] frag #1 3038@1296 -> + 2 pages [ 698.108147] frag #2 6076@1852 -> + 2 pages [ 698.108151] frag #3 6076@292 -> + 2 pages [ 698.108156] frag #4 6076@2828 -> + 3 pages [ 698.108160] frag #5 3038@1268 -> + 2 pages [ 698.108164] frag #6 2272@1824 -> + 1 pages [ 698.108168] frag #7 3804@0 -> + 1 pages [ 698.108172] frag #8 6076@264 -> + 2 pages [ 698.108177] frag #9 3946@2800 -> + 2 pages [ 698.108180] frags adding 18 slots
Quoted 7 lines
Quoted 4 lines
Quoted 3 lines
Since I am not deeply familiar with the networking code, I wonder about two things:
- is there something that should limit the skb data length from all frags to stay below the 64K which the definition ofMAX_SKB_FRAGShints?I think netfront should be able to handle 64K packets at most.
Ah, maybe this relates to this fix from Wei?
Yes, below patch limits SKB size to 64KB. However the problem here is not SKB exceeding 64KB. The said SKB is acutally 43KB in size. The problem is that guest kernel is using compound page so a frag which can be fit into one 4K page spans two 4K pages. The fix seems to be coalescing SKB in frontend, but it will degrade performance.
Wei.
From: Wei Liu
Quoted 38 lines · Ian Campbell
Quoted 37 lines · Zoltan Kiss
Quoted 35 lines · Stefan Bader
Quoted 30 lines
We had reports about this message being seen on EC2 for a while but finally a reporter did notice some details about the guests and was able to provide a simple way to reproduce[1].
For my local experiments I use a Xen-4.2.2 based host (though I would say the host versions are not important). The host has one NIC which is used as the outgoing port of a Linux based (not openvswitch) bridge. And the PV guests use that bridge. I set the mtu to 9001 (which was seen on affected instance types) and also inside the guests. As described in the report one guests runs redis-server and the other nodejs through two scripts (for me I had to do the two sub.js calls in separate shells). After a bit the error messages appear on the guest running the redis-server.
I added some debug printk's to show a bit more detail about the skb and got the following (<length>@<offset (after masking off complete pages)>):
[ 698.108119] xen_netfront: xennet: skb rides the rocket: 19 slots [ 698.108134] header 1490@238 -> 1 slots [ 698.108139] frag #0 1614@2164 -> + 1 pages [ 698.108143] frag #1 3038@1296 -> + 2 pages [ 698.108147] frag #2 6076@1852 -> + 2 pages [ 698.108151] frag #3 6076@292 -> + 2 pages [ 698.108156] frag #4 6076@2828 -> + 3 pages [ 698.108160] frag #5 3038@1268 -> + 2 pages [ 698.108164] frag #6 2272@1824 -> + 1 pages [ 698.108168] frag #7 3804@0 -> + 1 pages [ 698.108172] frag #8 6076@264 -> + 2 pages [ 698.108177] frag #9 3946@2800 -> + 2 pages [ 698.108180] frags adding 18 slotsQuoted 3 lines
Since I am not deeply familiar with the networking code, I wonder about two things:
- is there something that should limit the skb data length from all frags to stay below the 64K which the definition ofMAX_SKB_FRAGShints?I think netfront should be able to handle 64K packets at most.
Quoted 8 lines
Quoted 2 lines
Ah, maybe this relates to this fix from Wei?
Yes, below patch limits SKB size to 64KB. However the problem here is not SKB exceeding 64KB. The said SKB is acutally 43KB in size. The problem is that guest kernel is using compound page so a frag which can be fit into one 4K page spans two 4K pages. The fix seems to be coalescing SKB in frontend, but it will degrade performance.
Presumably the thing to do is copy the frag list into a locally allocated list of 4k fragments.
Unless you can support having some fragments mapped and some local there probably isn't much point in trying to maintain the fragment boundaries.
David
From: Wei Liu
Quoted 38 lines · Ian Campbell
Quoted 37 lines · Zoltan Kiss
Quoted 35 lines · Stefan Bader
Quoted 30 lines
We had reports about this message being seen on EC2 for a while but finally a reporter did notice some details about the guests and was able to provide a simple way to reproduce[1].
For my local experiments I use a Xen-4.2.2 based host (though I would say the host versions are not important). The host has one NIC which is used as the outgoing port of a Linux based (not openvswitch) bridge. And the PV guests use that bridge. I set the mtu to 9001 (which was seen on affected instance types) and also inside the guests. As described in the report one guests runs redis-server and the other nodejs through two scripts (for me I had to do the two sub.js calls in separate shells). After a bit the error messages appear on the guest running the redis-server.
I added some debug printk's to show a bit more detail about the skb and got the following (<length>@<offset (after masking off complete pages)>):
[ 698.108119] xen_netfront: xennet: skb rides the rocket: 19 slots [ 698.108134] header 1490@238 -> 1 slots [ 698.108139] frag #0 1614@2164 -> + 1 pages [ 698.108143] frag #1 3038@1296 -> + 2 pages [ 698.108147] frag #2 6076@1852 -> + 2 pages [ 698.108151] frag #3 6076@292 -> + 2 pages [ 698.108156] frag #4 6076@2828 -> + 3 pages [ 698.108160] frag #5 3038@1268 -> + 2 pages [ 698.108164] frag #6 2272@1824 -> + 1 pages [ 698.108168] frag #7 3804@0 -> + 1 pages [ 698.108172] frag #8 6076@264 -> + 2 pages [ 698.108177] frag #9 3946@2800 -> + 2 pages [ 698.108180] frags adding 18 slotsQuoted 3 lines
Since I am not deeply familiar with the networking code, I wonder about two things:
- is there something that should limit the skb data length from all frags to stay below the 64K which the definition ofMAX_SKB_FRAGShints?I think netfront should be able to handle 64K packets at most.
Quoted 8 lines
Quoted 2 lines
Ah, maybe this relates to this fix from Wei?
Yes, below patch limits SKB size to 64KB. However the problem here is not SKB exceeding 64KB. The said SKB is acutally 43KB in size. The problem is that guest kernel is using compound page so a frag which can be fit into one 4K page spans two 4K pages. The fix seems to be coalescing SKB in frontend, but it will degrade performance.
Presumably the thing to do is copy the frag list into a locally allocated list of 4k fragments.
Unless you can support having some fragments mapped and some local there probably isn't much point in trying to maintain the fragment boundaries.
David
Quoted 38 lines · Wei Liu
Quoted 37 lines · Ian Campbell
Quoted 35 lines · Zoltan Kiss
Quoted 30 lines · Stefan Bader
We had reports about this message being seen on EC2 for a while but finally a reporter did notice some details about the guests and was able to provide a simple way to reproduce[1].
For my local experiments I use a Xen-4.2.2 based host (though I would say the host versions are not important). The host has one NIC which is used as the outgoing port of a Linux based (not openvswitch) bridge. And the PV guests use that bridge. I set the mtu to 9001 (which was seen on affected instance types) and also inside the guests. As described in the report one guests runs redis-server and the other nodejs through two scripts (for me I had to do the two sub.js calls in separate shells). After a bit the error messages appear on the guest running the redis-server.
I added some debug printk's to show a bit more detail about the skb and got the following (<length>@<offset (after masking off complete pages)>):
[ 698.108119] xen_netfront: xennet: skb rides the rocket: 19 slots [ 698.108134] header 1490@238 -> 1 slots [ 698.108139] frag #0 1614@2164 -> + 1 pages [ 698.108143] frag #1 3038@1296 -> + 2 pages [ 698.108147] frag #2 6076@1852 -> + 2 pages [ 698.108151] frag #3 6076@292 -> + 2 pages [ 698.108156] frag #4 6076@2828 -> + 3 pages [ 698.108160] frag #5 3038@1268 -> + 2 pages [ 698.108164] frag #6 2272@1824 -> + 1 pages [ 698.108168] frag #7 3804@0 -> + 1 pages [ 698.108172] frag #8 6076@264 -> + 2 pages [ 698.108177] frag #9 3946@2800 -> + 2 pages [ 698.108180] frags adding 18 slotsQuoted 3 lines
Since I am not deeply familiar with the networking code, I wonder about two things:
- is there something that should limit the skb data length from all frags to stay below the 64K which the definition ofMAX_SKB_FRAGShints?I think netfront should be able to handle 64K packets at most.
Quoted 8 lines
Quoted 2 lines
Ah, maybe this relates to this fix from Wei?
Yes, below patch limits SKB size to 64KB. However the problem here is not SKB exceeding 64KB. The said SKB is acutally 43KB in size. The problem is that guest kernel is using compound page so a frag which can be fit into one 4K page spans two 4K pages. The fix seems to be coalescing SKB in frontend, but it will degrade performance.
So long as it only happens when this scenario occurs a performance degradation would seem preferable to dropping the skb altogether.
Ian.
Quoted 38 lines · Wei Liu
Quoted 37 lines · Ian Campbell
Quoted 35 lines · Zoltan Kiss
Quoted 30 lines · Stefan Bader
We had reports about this message being seen on EC2 for a while but finally a reporter did notice some details about the guests and was able to provide a simple way to reproduce[1].
For my local experiments I use a Xen-4.2.2 based host (though I would say the host versions are not important). The host has one NIC which is used as the outgoing port of a Linux based (not openvswitch) bridge. And the PV guests use that bridge. I set the mtu to 9001 (which was seen on affected instance types) and also inside the guests. As described in the report one guests runs redis-server and the other nodejs through two scripts (for me I had to do the two sub.js calls in separate shells). After a bit the error messages appear on the guest running the redis-server.
I added some debug printk's to show a bit more detail about the skb and got the following (<length>@<offset (after masking off complete pages)>):
[ 698.108119] xen_netfront: xennet: skb rides the rocket: 19 slots [ 698.108134] header 1490@238 -> 1 slots [ 698.108139] frag #0 1614@2164 -> + 1 pages [ 698.108143] frag #1 3038@1296 -> + 2 pages [ 698.108147] frag #2 6076@1852 -> + 2 pages [ 698.108151] frag #3 6076@292 -> + 2 pages [ 698.108156] frag #4 6076@2828 -> + 3 pages [ 698.108160] frag #5 3038@1268 -> + 2 pages [ 698.108164] frag #6 2272@1824 -> + 1 pages [ 698.108168] frag #7 3804@0 -> + 1 pages [ 698.108172] frag #8 6076@264 -> + 2 pages [ 698.108177] frag #9 3946@2800 -> + 2 pages [ 698.108180] frags adding 18 slotsQuoted 3 lines
Since I am not deeply familiar with the networking code, I wonder about two things:
- is there something that should limit the skb data length from all frags to stay below the 64K which the definition ofMAX_SKB_FRAGShints?I think netfront should be able to handle 64K packets at most.
Quoted 8 lines
Quoted 2 lines
Ah, maybe this relates to this fix from Wei?
Yes, below patch limits SKB size to 64KB. However the problem here is not SKB exceeding 64KB. The said SKB is acutally 43KB in size. The problem is that guest kernel is using compound page so a frag which can be fit into one 4K page spans two 4K pages. The fix seems to be coalescing SKB in frontend, but it will degrade performance.
So long as it only happens when this scenario occurs a performance degradation would seem preferable to dropping the skb altogether.
Ian.
Quoted 41 lines · Wei Liu
Quoted 32 lines · Ian Campbell
Quoted 31 lines · Zoltan Kiss
Quoted 30 lines · Stefan Bader
We had reports about this message being seen on EC2 for a while but finally a reporter did notice some details about the guests and was able to provide a simple way to reproduce[1].
For my local experiments I use a Xen-4.2.2 based host (though I would say the host versions are not important). The host has one NIC which is used as the outgoing port of a Linux based (not openvswitch) bridge. And the PV guests use that bridge. I set the mtu to 9001 (which was seen on affected instance types) and also inside the guests. As described in the report one guests runs redis-server and the other nodejs through two scripts (for me I had to do the two sub.js calls in separate shells). After a bit the error messages appear on the guest running the redis-server.
I added some debug printk's to show a bit more detail about the skb and got the following (<length>@<offset (after masking off complete pages)>):
[ 698.108119] xen_netfront: xennet: skb rides the rocket: 19 slots [ 698.108134] header 1490@238 -> 1 slots [ 698.108139] frag #0 1614@2164 -> + 1 pages [ 698.108143] frag #1 3038@1296 -> + 2 pages [ 698.108147] frag #2 6076@1852 -> + 2 pages [ 698.108151] frag #3 6076@292 -> + 2 pages [ 698.108156] frag #4 6076@2828 -> + 3 pages [ 698.108160] frag #5 3038@1268 -> + 2 pages [ 698.108164] frag #6 2272@1824 -> + 1 pages [ 698.108168] frag #7 3804@0 -> + 1 pages [ 698.108172] frag #8 6076@264 -> + 2 pages [ 698.108177] frag #9 3946@2800 -> + 2 pages [ 698.108180] frags adding 18 slotsQuoted 7 lines
Quoted 4 lines
Quoted 3 lines
Since I am not deeply familiar with the networking code, I wonder about two things:
- is there something that should limit the skb data length from all frags to stay below the 64K which the definition ofMAX_SKB_FRAGShints?I think netfront should be able to handle 64K packets at most.
Ah, maybe this relates to this fix from Wei?
Quoted 8 lines
Yes, below patch limits SKB size to 64KB. However the problem here is not SKB exceeding 64KB. The said SKB is acutally 43KB in size. The problem is that guest kernel is using compound page so a frag which can be fit into one 4K page spans two 4K pages. The fix seems to be coalescing SKB in frontend, but it will degrade performance.
Wei.
Reading more of the code I would agree. The definition of MAX_SKB_FRAGS (at least now with compound pages) cannot be used in any way to derive the number of 4k slots a transfer will require.
Zoltan already commented on worst cases. Not sure it would get as bad as that or "just" 16*4k frags all in the middle of compound pages. That would then end in around 33 or 34 slots, depending on the header.
Quoted 2 lines · Zoltan
I think the worst case scenario is when every frag and the linear buffer contains 2 bytes, which are overlapping a page boundary (that's (17+1)*2=36 so far), plus 15 of
them have a 4k
Quoted 2 lines
page in the middle of them, so, a 1+4096+1 byte buffer can span over 3 page. That's 51 individual pages.
I cannot claim to really know what to expect worst case. Somewhat I was thinking of a
worst case of (16+1)*2, which would be inconvenient enough.
So without knowing exactly how to do it, but as Ian said it sounds best to come up with some sort of exception coalescing in cases the slot count goes over 18 and we know the data size is below 64K.
-Stefan
_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xen.org/xen-devel
Quoted 41 lines · Wei Liu
Quoted 32 lines · Ian Campbell
Quoted 31 lines · Zoltan Kiss
Quoted 30 lines · Stefan Bader
We had reports about this message being seen on EC2 for a while but finally a reporter did notice some details about the guests and was able to provide a simple way to reproduce[1].
For my local experiments I use a Xen-4.2.2 based host (though I would say the host versions are not important). The host has one NIC which is used as the outgoing port of a Linux based (not openvswitch) bridge. And the PV guests use that bridge. I set the mtu to 9001 (which was seen on affected instance types) and also inside the guests. As described in the report one guests runs redis-server and the other nodejs through two scripts (for me I had to do the two sub.js calls in separate shells). After a bit the error messages appear on the guest running the redis-server.
I added some debug printk's to show a bit more detail about the skb and got the following (<length>@<offset (after masking off complete pages)>):
[ 698.108119] xen_netfront: xennet: skb rides the rocket: 19 slots [ 698.108134] header 1490@238 -> 1 slots [ 698.108139] frag #0 1614@2164 -> + 1 pages [ 698.108143] frag #1 3038@1296 -> + 2 pages [ 698.108147] frag #2 6076@1852 -> + 2 pages [ 698.108151] frag #3 6076@292 -> + 2 pages [ 698.108156] frag #4 6076@2828 -> + 3 pages [ 698.108160] frag #5 3038@1268 -> + 2 pages [ 698.108164] frag #6 2272@1824 -> + 1 pages [ 698.108168] frag #7 3804@0 -> + 1 pages [ 698.108172] frag #8 6076@264 -> + 2 pages [ 698.108177] frag #9 3946@2800 -> + 2 pages [ 698.108180] frags adding 18 slotsQuoted 7 lines
Quoted 4 lines
Quoted 3 lines
Since I am not deeply familiar with the networking code, I wonder about two things:
- is there something that should limit the skb data length from all frags to stay below the 64K which the definition ofMAX_SKB_FRAGShints?I think netfront should be able to handle 64K packets at most.
Ah, maybe this relates to this fix from Wei?
Quoted 8 lines
Yes, below patch limits SKB size to 64KB. However the problem here is not SKB exceeding 64KB. The said SKB is acutally 43KB in size. The problem is that guest kernel is using compound page so a frag which can be fit into one 4K page spans two 4K pages. The fix seems to be coalescing SKB in frontend, but it will degrade performance.
Wei.
Reading more of the code I would agree. The definition of MAX_SKB_FRAGS (at least now with compound pages) cannot be used in any way to derive the number of 4k slots a transfer will require.
Zoltan already commented on worst cases. Not sure it would get as bad as that or "just" 16*4k frags all in the middle of compound pages. That would then end in around 33 or 34 slots, depending on the header.
Quoted 2 lines · Zoltan
I think the worst case scenario is when every frag and the linear buffer contains 2 bytes, which are overlapping a page boundary (that's (17+1)*2=36 so far), plus 15 of
them have a 4k
Quoted 2 lines
page in the middle of them, so, a 1+4096+1 byte buffer can span over 3 page. That's 51 individual pages.
I cannot claim to really know what to expect worst case. Somewhat I was thinking of a
worst case of (16+1)*2, which would be inconvenient enough.
So without knowing exactly how to do it, but as Ian said it sounds best to come up with some sort of exception coalescing in cases the slot count goes over 18 and we know the data size is below 64K.
-Stefan
On Thu, May 15, 2014 at 02:14:00PM +0200, Stefan Bader wrote:
[...]
Quoted 17 lines
Quoted 2 lines
Wei.
Reading more of the code I would agree. The definition of
MAX_SKB_FRAGS(at least now with compound pages) cannot be used in any way to derive the number of 4k slots a transfer will require.Zoltan already commented on worst cases. Not sure it would get as bad as that or "just" 16*4k frags all in the middle of compound pages. That would then end in around 33 or 34 slots, depending on the header.
Quoted 2 lines · Zoltan
I think the worst case scenario is when every frag and the linear buffer contains 2 bytes, which are overlapping a page boundary (that's (17+1)*2=36 so far), plus 15 of
them have a 4k
Quoted 2 lines
page in the middle of them, so, a 1+4096+1 byte buffer can span over 3 page. That's 51 individual pages.
Quoted 8 lines
I cannot claim to really know what to expect worst case. Somewhat I was thinking of a
worst case of (16+1)*2, which would be inconvenient enough.So without knowing exactly how to do it, but as Ian said it sounds best to come up with some sort of exception coalescing in cases the slot count goes over 18 and we know the data size is below 64K.
I took a stab at it this morning and came up with this patch. Ran redis-benchmark, it seemed to fix that for me -- only saw one "failed to linearize skb" during
redis-benchmark -h XXX -d 1000 -t lrange
And before this change, a lot of "rides rocket" were triggered.
Thought?
---8<---
Quoted 1 line
From
743495a2b2d338fc6cfe9bfd4b6e840392b87f4aMon Sep 17 00:00:00 2001
From: Wei Liu <[email protected]>
Date: Fri, 16 May 2014 10:39:01 +0100
Subject: [PATCH] xen-netfront: linearize SKB if it occupies too many slots
Some workload, such as Redis can generate SKBs which make use of compound pages. Netfront doesn't quite like that because it doesn't want to send exessive slots to the backend as backend might deem it malicious. On the flip side these packets are actually legit, the size check at the beginning of xennet_start_xmit ensures that packet size is below 64K.
So we linearize SKB if it occupies too many slots. If the linearization fails then the SKB is dropped.
--- drivers/net/xen-netfront.c | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-)
drivers/net/xen-netfront.c +15 -3
@@ -573,9 +573,21 @@ static int xennet_start_xmit(struct sk_buff *skb, struct net_device *dev) slots =DIV_ROUND_UP (offset + len,PAGE_SIZE ) +xennet_count_skb_frag_slots (skb);if (unlikely (slots >MAX_SKB_FRAGS +1 )) { -net_alert_ratelimited ( -"xennet: skb rides the rocket: %d slots\n" , slots); - goto drop; +if (skb_linearize (skb)) { +net_alert_ratelimited ( +"xennet: failed to linearize skb, skb dropped\n" ); + goto drop; + } + data = skb->data; + offset =offset_in_page (data); + len =skb_headlen (skb); + slots =DIV_ROUND_UP (offset + len,PAGE_SIZE ) + +xennet_count_skb_frag_slots (skb); +if (unlikely (slots >MAX_SKB_FRAGS +1 )) { +net_alert_ratelimited ( +"xennet: still too many slots after linerization: %d" , slots); + goto drop; + } }spin_lock_irqsave (&np->tx_lock, flags);
Signature · 1 line
1.7.10.4
Quoted 3 lines
-Stefan
Quoted 32 lines · Wei Liu
On Thu, May 15, 2014 at 02:14:00PM +0200, Stefan Bader wrote:
[...]Quoted 17 lines
Quoted 2 lines
Wei.
Reading more of the code I would agree. The definition of
MAX_SKB_FRAGS(at least now with compound pages) cannot be used in any way to derive the number of 4k slots a transfer will require.Zoltan already commented on worst cases. Not sure it would get as bad as that or "just" 16*4k frags all in the middle of compound pages. That would then end in around 33 or 34 slots, depending on the header.
Quoted 2 lines · Zoltan
I think the worst case scenario is when every frag and the linear buffer contains 2 bytes, which are overlapping a page boundary (that's (17+1)*2=36 so far), plus 15 of
them have a 4k
Quoted 2 lines
page in the middle of them, so, a 1+4096+1 byte buffer can span over 3 page. That's 51 individual pages.
Quoted 8 lines
I cannot claim to really know what to expect worst case. Somewhat I was thinking of a
worst case of (16+1)*2, which would be inconvenient enough.So without knowing exactly how to do it, but as Ian said it sounds best to come up with some sort of exception coalescing in cases the slot count goes over 18 and we know the data size is below 64K.
I took a stab at it this morning and came up with this patch. Ran redis-benchmark, it seemed to fix that for me -- only saw one "failed to linearize skb" during
Quoted 6 lines
redis-benchmark -h XXX -d 1000 -t lrange
And before this change, a lot of "rides rocket" were triggered.
Thought?
And don't take this as a formal patch because I haven't tested it thoroughly. It's just an idea to coalesce SKBs. We can certainly code up a loop to do so by ourself.
Wei.
Quoted 32 lines · Wei Liu
On Thu, May 15, 2014 at 02:14:00PM +0200, Stefan Bader wrote:
[...]Quoted 17 lines
Quoted 2 lines
Wei.
Reading more of the code I would agree. The definition of
MAX_SKB_FRAGS(at least now with compound pages) cannot be used in any way to derive the number of 4k slots a transfer will require.Zoltan already commented on worst cases. Not sure it would get as bad as that or "just" 16*4k frags all in the middle of compound pages. That would then end in around 33 or 34 slots, depending on the header.
Quoted 2 lines · Zoltan
I think the worst case scenario is when every frag and the linear buffer contains 2 bytes, which are overlapping a page boundary (that's (17+1)*2=36 so far), plus 15 of
them have a 4k
Quoted 2 lines
page in the middle of them, so, a 1+4096+1 byte buffer can span over 3 page. That's 51 individual pages.
Quoted 8 lines
I cannot claim to really know what to expect worst case. Somewhat I was thinking of a
worst case of (16+1)*2, which would be inconvenient enough.So without knowing exactly how to do it, but as Ian said it sounds best to come up with some sort of exception coalescing in cases the slot count goes over 18 and we know the data size is below 64K.
I took a stab at it this morning and came up with this patch. Ran redis-benchmark, it seemed to fix that for me -- only saw one "failed to linearize skb" during
Quoted 6 lines
redis-benchmark -h XXX -d 1000 -t lrange
And before this change, a lot of "rides rocket" were triggered.
Thought?
And don't take this as a formal patch because I haven't tested it thoroughly. It's just an idea to coalesce SKBs. We can certainly code up a loop to do so by ourself.
Wei.
From: Wei Liu
Quoted 62 lines · Stefan Bader
[...]
Quoted 17 lines
Quoted 2 lines
Wei.
Reading more of the code I would agree. The definition of
MAX_SKB_FRAGS(at least now with compound pages) cannot be used in any way to derive the number of 4k slots a transfer will require.Zoltan already commented on worst cases. Not sure it would get as bad as that or "just" 16*4k frags all in the middle of compound pages. That would then end in around 33 or 34 slots, depending on the header.
Quoted 2 lines · Zoltan
I think the worst case scenario is when every frag and the linear buffer contains 2 bytes, which are overlapping a page boundary (that's (17+1)*2=36 so far), plus 15 of
them have a 4k
Quoted 2 lines
page in the middle of them, so, a 1+4096+1 byte buffer can span over 3 page. That's 51 individual pages.
Quoted 7 lines
I cannot claim to really know what to expect worst case. Somewhat I was thinking of a worst case of (16+1)*2, which would be inconvenient enough.
So without knowing exactly how to do it, but as Ian said it sounds best to come up with some sort of exception coalescing in cases the slot count goes over 18 and we know the data size is below 64K.
I took a stab at it this morning and came up with this patch. Ran redis-benchmark, it seemed to fix that for me -- only saw one "failed to linearize skb" during
redis-benchmark -h XXX -d 1000 -t lrange
And before this change, a lot of "rides rocket" were triggered.
Thought?
---8<---
From743495a2b2d338fc6cfe9bfd4b6e840392b87f4aMon Sep 17 00:00:00 2001
From: Wei Liu <[email protected]>
Date: Fri, 16 May 2014 10:39:01 +0100
Subject: [PATCH] xen-netfront: linearize SKB if it occupies too many slotsSome workload, such as Redis can generate SKBs which make use of compound pages. Netfront doesn't quite like that because it doesn't want to send exessive slots to the backend as backend might deem it malicious. On the flip side these packets are actually legit, the size check at the beginning of
xennet_start_xmitensures that packet size is below 64K.So we linearize SKB if it occupies too many slots. If the linearization fails then the SKB is dropped.
Signed-off-by Wei Liu <[email protected]>--- drivers/net/xen-netfront.c | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-)drivers/net/xen-netfront.c +0
-0
drivers/net/xen-netfront.c static int xennet_start_xmit(struct sk_buff *skb, struct net_device *dev) · line 576
Quoted 7 lines
drivers/net/xen-netfront.c +1
-3@@ -573,9 +573,21 @@ static int xennet_start_xmit(struct sk_buff *skb, struct net_device *dev) slots =DIV_ROUND_UP (offset + len,PAGE_SIZE ) +xennet_count_skb_frag_slots (skb);if (unlikely (slots >MAX_SKB_FRAGS +1 )) { -net_alert_ratelimited ( -"xennet: skb rides the rocket: %d slots\n" , slots); - goto drop; +if (skb_linearize (skb)) {
You don't need to actually linearize the skb here.
One with multiple fragments is fine.
I'm not sure there is a standard function to 'copy and refragment' the skb data though.
drivers/net/xen-netfront.c static int xennet_start_xmit(struct sk_buff *skb, struct net_device *dev) · line 585
Quoted 9 lines
drivers/net/xen-netfront.c +9
-0+net_alert_ratelimited ( +"xennet: failed to linearize skb, skb dropped\n" ); + goto drop; + } + data = skb->data; + offset =offset_in_page (data); + len =skb_headlen (skb); + slots =DIV_ROUND_UP (offset + len,PAGE_SIZE ) + +xennet_count_skb_frag_slots (skb);
IIRC If you have called skb_linearize then there shouldn't be any fragments.
Quoted 10 lines
drivers/net/xen-netfront.c +5
-1+if (unlikely (slots >MAX_SKB_FRAGS +1 )) { +net_alert_ratelimited ( +"xennet: still too many slots after linerization: %d" , slots); + goto drop; + } }spin_lock_irqsave (&np->tx_lock, flags); --1.7.10.4
David
On Fri, May 16, 2014 at 10:05:46AM +0000, David Laight wrote:
[...]
Quoted 20 lines
Quoted 20 lines
---8<---
From743495a2b2d338fc6cfe9bfd4b6e840392b87f4aMon Sep 17 00:00:00 2001
From: Wei Liu <[email protected]>
Date: Fri, 16 May 2014 10:39:01 +0100
Subject: [PATCH] xen-netfront: linearize SKB if it occupies too many slotsSome workload, such as Redis can generate SKBs which make use of compound pages. Netfront doesn't quite like that because it doesn't want to send exessive slots to the backend as backend might deem it malicious. On the flip side these packets are actually legit, the size check at the beginning of
xennet_start_xmitensures that packet size is below 64K.So we linearize SKB if it occupies too many slots. If the linearization fails then the SKB is dropped.
Signed-off-by Wei Liu <[email protected]>--- drivers/net/xen-netfront.c | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-)
Quoted 15 lines
drivers/net/xen-netfront.c static int xennet_start_xmit(struct sk_buff *skb, struct net_device *dev) · line 576
Quoted 12 lines
drivers/net/xen-netfront.c +1
-3@@ -573,9 +573,21 @@ static int xennet_start_xmit(struct sk_buff *skb, struct net_device *dev) slots =DIV_ROUND_UP (offset + len,PAGE_SIZE ) +xennet_count_skb_frag_slots (skb);if (unlikely (slots >MAX_SKB_FRAGS +1 )) { -net_alert_ratelimited ( -"xennet: skb rides the rocket: %d slots\n" , slots); - goto drop; +if (skb_linearize (skb)) {You don't need to actually linearize the skb here. One with multiple fragments is fine.
Yes I know; but my idea is to get a SKB that doesn't use up too many slots -- a linearized SKB will not use too many slots.
Quoted 3 lines
I'm not sure there is a standard function to 'copy and refragment' the skb data though.
That can only help if we can control how the fragment page is allocated.
Quoted 12 lines
drivers/net/xen-netfront.c static int xennet_start_xmit(struct sk_buff *skb, struct net_device *dev) · line 585
Quoted 9 lines
drivers/net/xen-netfront.c +9
-0+net_alert_ratelimited ( +"xennet: failed to linearize skb, skb dropped\n" ); + goto drop; + } + data = skb->data; + offset =offset_in_page (data); + len =skb_headlen (skb); + slots =DIV_ROUND_UP (offset + len,PAGE_SIZE ) + +xennet_count_skb_frag_slots (skb);IIRC If you have called
skb_linearizethen there shouldn't be any fragments.
Plain copy-n-paste from code above. I will pay more attention if it's a formal patch. ;-)
Wei.
On Fri, May 16, 2014 at 10:05:46AM +0000, David Laight wrote:
[...]
Quoted 20 lines
Quoted 20 lines
---8<---
From743495a2b2d338fc6cfe9bfd4b6e840392b87f4aMon Sep 17 00:00:00 2001
From: Wei Liu <[email protected]>
Date: Fri, 16 May 2014 10:39:01 +0100
Subject: [PATCH] xen-netfront: linearize SKB if it occupies too many slotsSome workload, such as Redis can generate SKBs which make use of compound pages. Netfront doesn't quite like that because it doesn't want to send exessive slots to the backend as backend might deem it malicious. On the flip side these packets are actually legit, the size check at the beginning of
xennet_start_xmitensures that packet size is below 64K.So we linearize SKB if it occupies too many slots. If the linearization fails then the SKB is dropped.
Signed-off-by Wei Liu <[email protected]>--- drivers/net/xen-netfront.c | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-)
Quoted 15 lines
drivers/net/xen-netfront.c static int xennet_start_xmit(struct sk_buff *skb, struct net_device *dev) · line 576
Quoted 12 lines
drivers/net/xen-netfront.c +1
-3@@ -573,9 +573,21 @@ static int xennet_start_xmit(struct sk_buff *skb, struct net_device *dev) slots =DIV_ROUND_UP (offset + len,PAGE_SIZE ) +xennet_count_skb_frag_slots (skb);if (unlikely (slots >MAX_SKB_FRAGS +1 )) { -net_alert_ratelimited ( -"xennet: skb rides the rocket: %d slots\n" , slots); - goto drop; +if (skb_linearize (skb)) {You don't need to actually linearize the skb here. One with multiple fragments is fine.
Yes I know; but my idea is to get a SKB that doesn't use up too many slots -- a linearized SKB will not use too many slots.
Quoted 3 lines
I'm not sure there is a standard function to 'copy and refragment' the skb data though.
That can only help if we can control how the fragment page is allocated.
Quoted 12 lines
drivers/net/xen-netfront.c static int xennet_start_xmit(struct sk_buff *skb, struct net_device *dev) · line 585
Quoted 9 lines
drivers/net/xen-netfront.c +9
-0+net_alert_ratelimited ( +"xennet: failed to linearize skb, skb dropped\n" ); + goto drop; + } + data = skb->data; + offset =offset_in_page (data); + len =skb_headlen (skb); + slots =DIV_ROUND_UP (offset + len,PAGE_SIZE ) + +xennet_count_skb_frag_slots (skb);IIRC If you have called
skb_linearizethen there shouldn't be any fragments.
Plain copy-n-paste from code above. I will pay more attention if it's a formal patch. ;-)
Wei.
From: Wei Liu
Quoted 62 lines · Stefan Bader
[...]
Quoted 17 lines
Quoted 2 lines
Wei.
Reading more of the code I would agree. The definition of
MAX_SKB_FRAGS(at least now with compound pages) cannot be used in any way to derive the number of 4k slots a transfer will require.Zoltan already commented on worst cases. Not sure it would get as bad as that or "just" 16*4k frags all in the middle of compound pages. That would then end in around 33 or 34 slots, depending on the header.
Quoted 2 lines · Zoltan
I think the worst case scenario is when every frag and the linear buffer contains 2 bytes, which are overlapping a page boundary (that's (17+1)*2=36 so far), plus 15 of
them have a 4k
Quoted 2 lines
page in the middle of them, so, a 1+4096+1 byte buffer can span over 3 page. That's 51 individual pages.
Quoted 7 lines
I cannot claim to really know what to expect worst case. Somewhat I was thinking of a worst case of (16+1)*2, which would be inconvenient enough.
So without knowing exactly how to do it, but as Ian said it sounds best to come up with some sort of exception coalescing in cases the slot count goes over 18 and we know the data size is below 64K.
I took a stab at it this morning and came up with this patch. Ran redis-benchmark, it seemed to fix that for me -- only saw one "failed to linearize skb" during
redis-benchmark -h XXX -d 1000 -t lrange
And before this change, a lot of "rides rocket" were triggered.
Thought?
---8<---
From743495a2b2d338fc6cfe9bfd4b6e840392b87f4aMon Sep 17 00:00:00 2001
From: Wei Liu <[email protected]>
Date: Fri, 16 May 2014 10:39:01 +0100
Subject: [PATCH] xen-netfront: linearize SKB if it occupies too many slotsSome workload, such as Redis can generate SKBs which make use of compound pages. Netfront doesn't quite like that because it doesn't want to send exessive slots to the backend as backend might deem it malicious. On the flip side these packets are actually legit, the size check at the beginning of
xennet_start_xmitensures that packet size is below 64K.So we linearize SKB if it occupies too many slots. If the linearization fails then the SKB is dropped.
Signed-off-by Wei Liu <[email protected]>--- drivers/net/xen-netfront.c | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-)drivers/net/xen-netfront.c +0
-0
drivers/net/xen-netfront.c static int xennet_start_xmit(struct sk_buff *skb, struct net_device *dev) · line 576
Quoted 7 lines
drivers/net/xen-netfront.c +1
-3@@ -573,9 +573,21 @@ static int xennet_start_xmit(struct sk_buff *skb, struct net_device *dev) slots =DIV_ROUND_UP (offset + len,PAGE_SIZE ) +xennet_count_skb_frag_slots (skb);if (unlikely (slots >MAX_SKB_FRAGS +1 )) { -net_alert_ratelimited ( -"xennet: skb rides the rocket: %d slots\n" , slots); - goto drop; +if (skb_linearize (skb)) {
You don't need to actually linearize the skb here.
One with multiple fragments is fine.
I'm not sure there is a standard function to 'copy and refragment' the skb data though.
drivers/net/xen-netfront.c static int xennet_start_xmit(struct sk_buff *skb, struct net_device *dev) · line 585
Quoted 9 lines
drivers/net/xen-netfront.c +9
-0+net_alert_ratelimited ( +"xennet: failed to linearize skb, skb dropped\n" ); + goto drop; + } + data = skb->data; + offset =offset_in_page (data); + len =skb_headlen (skb); + slots =DIV_ROUND_UP (offset + len,PAGE_SIZE ) + +xennet_count_skb_frag_slots (skb);
IIRC If you have called skb_linearize then there shouldn't be any fragments.
Quoted 10 lines
drivers/net/xen-netfront.c +5
-1+if (unlikely (slots >MAX_SKB_FRAGS +1 )) { +net_alert_ratelimited ( +"xennet: still too many slots after linerization: %d" , slots); + goto drop; + } }spin_lock_irqsave (&np->tx_lock, flags); --1.7.10.4
David
Quoted 32 lines · Wei Liu
On Thu, May 15, 2014 at 02:14:00PM +0200, Stefan Bader wrote:
[...]Quoted 17 lines
Quoted 2 lines
Wei.
Reading more of the code I would agree. The definition of
MAX_SKB_FRAGS(at least now with compound pages) cannot be used in any way to derive the number of 4k slots a transfer will require.Zoltan already commented on worst cases. Not sure it would get as bad as that or "just" 16*4k frags all in the middle of compound pages. That would then end in around 33 or 34 slots, depending on the header.
Quoted 2 lines · Zoltan
I think the worst case scenario is when every frag and the linear buffer contains 2 bytes, which are overlapping a page boundary (that's (17+1)*2=36 so far), plus 15 of
them have a 4k
Quoted 2 lines
page in the middle of them, so, a 1+4096+1 byte buffer can span over 3 page. That's 51 individual pages.
Quoted 8 lines
I cannot claim to really know what to expect worst case. Somewhat I was thinking of a
worst case of (16+1)*2, which would be inconvenient enough.So without knowing exactly how to do it, but as Ian said it sounds best to come up with some sort of exception coalescing in cases the slot count goes over 18 and we know the data size is below 64K.
I took a stab at it this morning and came up with this patch. Ran redis-benchmark, it seemed to fix that for me -- only saw one "failed to linearize skb" during
Quoted 5 lines
redis-benchmark -h XXX -d 1000 -t lrange
And before this change, a lot of "rides rocket" were triggered.
Thought?
It appears at least to me as something that nicely makes use of existing code. I was wondering about what could or could not be used. Trying to get ones head around the whole thing is kind of a lot to look at.
The change at least looks straight forward enough.
-Stefan
Quoted 51 lines
---8<---
From743495a2b2d338fc6cfe9bfd4b6e840392b87f4aMon Sep 17 00:00:00 2001
From: Wei Liu <[email protected]>
Date: Fri, 16 May 2014 10:39:01 +0100
Subject: [PATCH] xen-netfront: linearize SKB if it occupies too many slotsSome workload, such as Redis can generate SKBs which make use of compound pages. Netfront doesn't quite like that because it doesn't want to send exessive slots to the backend as backend might deem it malicious. On the flip side these packets are actually legit, the size check at the beginning of
xennet_start_xmitensures that packet size is below 64K.So we linearize SKB if it occupies too many slots. If the linearization fails then the SKB is dropped.
Signed-off-by Wei Liu <[email protected]>--- drivers/net/xen-netfront.c | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-)drivers/net/xen-netfront.c +15
-3@@ -573,9 +573,21 @@ static int xennet_start_xmit(struct sk_buff *skb, struct net_device *dev) slots =DIV_ROUND_UP (offset + len,PAGE_SIZE ) +xennet_count_skb_frag_slots (skb);if (unlikely (slots >MAX_SKB_FRAGS +1 )) { -net_alert_ratelimited ( -"xennet: skb rides the rocket: %d slots\n" , slots); - goto drop; +if (skb_linearize (skb)) { +net_alert_ratelimited ( +"xennet: failed to linearize skb, skb dropped\n" ); + goto drop; + } + data = skb->data; + offset =offset_in_page (data); + len =skb_headlen (skb); + slots =DIV_ROUND_UP (offset + len,PAGE_SIZE ) + +xennet_count_skb_frag_slots (skb); +if (unlikely (slots >MAX_SKB_FRAGS +1 )) { +net_alert_ratelimited ( +"xennet: still too many slots after linerization: %d" , slots); + goto drop; + } }spin_lock_irqsave (&np->tx_lock, flags);
Quoted 37 lines · Stefan Bader
Quoted 36 lines · Wei Liu
On Thu, May 15, 2014 at 02:14:00PM +0200, Stefan Bader wrote:
[...]Quoted 17 lines
Quoted 2 lines
Wei.
Reading more of the code I would agree. The definition of
MAX_SKB_FRAGS(at least now with compound pages) cannot be used in any way to derive the number of 4k slots a transfer will require.Zoltan already commented on worst cases. Not sure it would get as bad as that or "just" 16*4k frags all in the middle of compound pages. That would then end in around 33 or 34 slots, depending on the header.
Quoted 2 lines · Zoltan
I think the worst case scenario is when every frag and the linear buffer contains 2 bytes, which are overlapping a page boundary (that's (17+1)*2=36 so far), plus 15 of
them have a 4k
Quoted 2 lines
page in the middle of them, so, a 1+4096+1 byte buffer can span over 3 page. That's 51 individual pages.
Quoted 8 lines
I cannot claim to really know what to expect worst case. Somewhat I was thinking of a
worst case of (16+1)*2, which would be inconvenient enough.So without knowing exactly how to do it, but as Ian said it sounds best to come up with some sort of exception coalescing in cases the slot count goes over 18 and we know the data size is below 64K.
I took a stab at it this morning and came up with this patch. Ran redis-benchmark, it seemed to fix that for me -- only saw one "failed to linearize skb" during
redis-benchmark -h XXX -d 1000 -t lrange
And before this change, a lot of "rides rocket" were triggered.
Quoted 7 lines
Quoted 1 line
Thought?
It appears at least to me as something that nicely makes use of existing code. I was wondering about what could or could not be used. Trying to get ones head around the whole thing is kind of a lot to look at.
The change at least looks straight forward enough.
The only woe for me is that I am looking puzzled at the implementation of skb_linearize(). Somehow the data_len element decides whether a skb can be linearized and basically how much it tries to pull from the tail. It probably makes sense ... just not to me with not deep experience here.
-Stefan
Quoted 44 lines · Stefan Bader
Quoted 43 lines · Stefan Bader
Quoted 32 lines · Wei Liu
On Thu, May 15, 2014 at 02:14:00PM +0200, Stefan Bader wrote:
[...]Quoted 17 lines
Quoted 2 lines
Wei.
Reading more of the code I would agree. The definition of
MAX_SKB_FRAGS(at least now with compound pages) cannot be used in any way to derive the number of 4k slots a transfer will require.Zoltan already commented on worst cases. Not sure it would get as bad as that or "just" 16*4k frags all in the middle of compound pages. That would then end in around 33 or 34 slots, depending on the header.
Quoted 2 lines · Zoltan
I think the worst case scenario is when every frag and the linear buffer contains 2 bytes, which are overlapping a page boundary (that's (17+1)*2=36 so far), plus 15 of
them have a 4k
Quoted 2 lines
page in the middle of them, so, a 1+4096+1 byte buffer can span over 3 page. That's 51 individual pages.
Quoted 8 lines
I cannot claim to really know what to expect worst case. Somewhat I was thinking of a
worst case of (16+1)*2, which would be inconvenient enough.So without knowing exactly how to do it, but as Ian said it sounds best to come up with some sort of exception coalescing in cases the slot count goes over 18 and we know the data size is below 64K.
I took a stab at it this morning and came up with this patch. Ran redis-benchmark, it seemed to fix that for me -- only saw one "failed to linearize skb" during
Quoted 5 lines
redis-benchmark -h XXX -d 1000 -t lrange
And before this change, a lot of "rides rocket" were triggered.
Thought?
It appears at least to me as something that nicely makes use of existing code. I was wondering about what could or could not be used. Trying to get ones head around the whole thing is kind of a lot to look at.
Quoted 7 lines
Quoted 1 line
The change at least looks straight forward enough.
The only woe for me is that I am looking puzzled at the implementation of
skb_linearize(). Somehow thedata_lenelement decides whether a skb can be linearized and basically how much it tries to pull from the tail. It probably makes sense ... just not to me with not deep experience here.
Data_len is the size of paged data (that's the size of data in frags). If it pulls everything from frags to linear area then SKB is linearized.
Wei.
Quoted 2 lines
-Stefan
Quoted 44 lines · Stefan Bader
Quoted 43 lines · Stefan Bader
Quoted 32 lines · Wei Liu
On Thu, May 15, 2014 at 02:14:00PM +0200, Stefan Bader wrote:
[...]Quoted 17 lines
Quoted 2 lines
Wei.
Reading more of the code I would agree. The definition of
MAX_SKB_FRAGS(at least now with compound pages) cannot be used in any way to derive the number of 4k slots a transfer will require.Zoltan already commented on worst cases. Not sure it would get as bad as that or "just" 16*4k frags all in the middle of compound pages. That would then end in around 33 or 34 slots, depending on the header.
Quoted 2 lines · Zoltan
I think the worst case scenario is when every frag and the linear buffer contains 2 bytes, which are overlapping a page boundary (that's (17+1)*2=36 so far), plus 15 of
them have a 4k
Quoted 2 lines
page in the middle of them, so, a 1+4096+1 byte buffer can span over 3 page. That's 51 individual pages.
Quoted 8 lines
I cannot claim to really know what to expect worst case. Somewhat I was thinking of a
worst case of (16+1)*2, which would be inconvenient enough.So without knowing exactly how to do it, but as Ian said it sounds best to come up with some sort of exception coalescing in cases the slot count goes over 18 and we know the data size is below 64K.
I took a stab at it this morning and came up with this patch. Ran redis-benchmark, it seemed to fix that for me -- only saw one "failed to linearize skb" during
Quoted 5 lines
redis-benchmark -h XXX -d 1000 -t lrange
And before this change, a lot of "rides rocket" were triggered.
Thought?
It appears at least to me as something that nicely makes use of existing code. I was wondering about what could or could not be used. Trying to get ones head around the whole thing is kind of a lot to look at.
Quoted 7 lines
Quoted 1 line
The change at least looks straight forward enough.
The only woe for me is that I am looking puzzled at the implementation of
skb_linearize(). Somehow thedata_lenelement decides whether a skb can be linearized and basically how much it tries to pull from the tail. It probably makes sense ... just not to me with not deep experience here.
Data_len is the size of paged data (that's the size of data in frags). If it pulls everything from frags to linear area then SKB is linearized.
Wei.
Quoted 2 lines
-Stefan
Quoted 37 lines · Stefan Bader
Quoted 36 lines · Wei Liu
On Thu, May 15, 2014 at 02:14:00PM +0200, Stefan Bader wrote:
[...]Quoted 17 lines
Quoted 2 lines
Wei.
Reading more of the code I would agree. The definition of
MAX_SKB_FRAGS(at least now with compound pages) cannot be used in any way to derive the number of 4k slots a transfer will require.Zoltan already commented on worst cases. Not sure it would get as bad as that or "just" 16*4k frags all in the middle of compound pages. That would then end in around 33 or 34 slots, depending on the header.
Quoted 2 lines · Zoltan
I think the worst case scenario is when every frag and the linear buffer contains 2 bytes, which are overlapping a page boundary (that's (17+1)*2=36 so far), plus 15 of
them have a 4k
Quoted 2 lines
page in the middle of them, so, a 1+4096+1 byte buffer can span over 3 page. That's 51 individual pages.
Quoted 8 lines
I cannot claim to really know what to expect worst case. Somewhat I was thinking of a
worst case of (16+1)*2, which would be inconvenient enough.So without knowing exactly how to do it, but as Ian said it sounds best to come up with some sort of exception coalescing in cases the slot count goes over 18 and we know the data size is below 64K.
I took a stab at it this morning and came up with this patch. Ran redis-benchmark, it seemed to fix that for me -- only saw one "failed to linearize skb" during
redis-benchmark -h XXX -d 1000 -t lrange
And before this change, a lot of "rides rocket" were triggered.
Quoted 7 lines
Quoted 1 line
Thought?
It appears at least to me as something that nicely makes use of existing code. I was wondering about what could or could not be used. Trying to get ones head around the whole thing is kind of a lot to look at.
The change at least looks straight forward enough.
The only woe for me is that I am looking puzzled at the implementation of skb_linearize(). Somehow the data_len element decides whether a skb can be linearized and basically how much it tries to pull from the tail. It probably makes sense ... just not to me with not deep experience here.
-Stefan
_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xen.org/xen-devel
Quoted 32 lines · Wei Liu
On Thu, May 15, 2014 at 02:14:00PM +0200, Stefan Bader wrote:
[...]Quoted 17 lines
Quoted 2 lines
Wei.
Reading more of the code I would agree. The definition of
MAX_SKB_FRAGS(at least now with compound pages) cannot be used in any way to derive the number of 4k slots a transfer will require.Zoltan already commented on worst cases. Not sure it would get as bad as that or "just" 16*4k frags all in the middle of compound pages. That would then end in around 33 or 34 slots, depending on the header.
Quoted 2 lines · Zoltan
I think the worst case scenario is when every frag and the linear buffer contains 2 bytes, which are overlapping a page boundary (that's (17+1)*2=36 so far), plus 15 of
them have a 4k
Quoted 2 lines
page in the middle of them, so, a 1+4096+1 byte buffer can span over 3 page. That's 51 individual pages.
Quoted 8 lines
I cannot claim to really know what to expect worst case. Somewhat I was thinking of a
worst case of (16+1)*2, which would be inconvenient enough.So without knowing exactly how to do it, but as Ian said it sounds best to come up with some sort of exception coalescing in cases the slot count goes over 18 and we know the data size is below 64K.
I took a stab at it this morning and came up with this patch. Ran redis-benchmark, it seemed to fix that for me -- only saw one "failed to linearize skb" during
Quoted 5 lines
redis-benchmark -h XXX -d 1000 -t lrange
And before this change, a lot of "rides rocket" were triggered.
Thought?
It appears at least to me as something that nicely makes use of existing code. I was wondering about what could or could not be used. Trying to get ones head around the whole thing is kind of a lot to look at.
The change at least looks straight forward enough.
-Stefan
Quoted 41 lines
---8<---
From743495a2b2d338fc6cfe9bfd4b6e840392b87f4aMon Sep 17 00:00:00 2001
From: Wei Liu <[email protected]>
Date: Fri, 16 May 2014 10:39:01 +0100
Subject: [PATCH] xen-netfront: linearize SKB if it occupies too many slotsSome workload, such as Redis can generate SKBs which make use of compound pages. Netfront doesn't quite like that because it doesn't want to send exessive slots to the backend as backend might deem it malicious. On the flip side these packets are actually legit, the size check at the beginning of
xennet_start_xmitensures that packet size is below 64K.So we linearize SKB if it occupies too many slots. If the linearization fails then the SKB is dropped.
Signed-off-by Wei Liu <[email protected]>--- drivers/net/xen-netfront.c | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-)drivers/net/xen-netfront.c +8
-3@@ -573,9 +573,21 @@ static int xennet_start_xmit(struct sk_buff *skb, struct net_device *dev) slots =DIV_ROUND_UP (offset + len,PAGE_SIZE ) +xennet_count_skb_frag_slots (skb);if (unlikely (slots >MAX_SKB_FRAGS +1 )) { -net_alert_ratelimited ( -"xennet: skb rides the rocket: %d slots\n" , slots); - goto drop; +if (skb_linearize (skb)) { +net_alert_ratelimited ( +"xennet: failed to linearize skb, skb dropped\n" ); + goto drop; + } + data = skb->data; + offset =offset_in_page (data); + len =skb_headlen (skb);
drivers/net/xen-netfront.c static int xennet_start_xmit(struct sk_buff *skb, struct net_device *dev) · line 593
Quoted 10 lines
drivers/net/xen-netfront.c +7
-0+ slots =DIV_ROUND_UP (offset + len,PAGE_SIZE ) + +xennet_count_skb_frag_slots (skb); +if (unlikely (slots >MAX_SKB_FRAGS +1 )) { +net_alert_ratelimited ( +"xennet: still too many slots after linerization: %d" , slots); + goto drop; + } }spin_lock_irqsave (&np->tx_lock, flags);
_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xen.org/xen-devel
On Thu, May 15, 2014 at 02:14:00PM +0200, Stefan Bader wrote:
[...]
Quoted 17 lines
Quoted 2 lines
Wei.
Reading more of the code I would agree. The definition of
MAX_SKB_FRAGS(at least now with compound pages) cannot be used in any way to derive the number of 4k slots a transfer will require.Zoltan already commented on worst cases. Not sure it would get as bad as that or "just" 16*4k frags all in the middle of compound pages. That would then end in around 33 or 34 slots, depending on the header.
Quoted 2 lines · Zoltan
I think the worst case scenario is when every frag and the linear buffer contains 2 bytes, which are overlapping a page boundary (that's (17+1)*2=36 so far), plus 15 of
them have a 4k
Quoted 2 lines
page in the middle of them, so, a 1+4096+1 byte buffer can span over 3 page. That's 51 individual pages.
Quoted 8 lines
I cannot claim to really know what to expect worst case. Somewhat I was thinking of a
worst case of (16+1)*2, which would be inconvenient enough.So without knowing exactly how to do it, but as Ian said it sounds best to come up with some sort of exception coalescing in cases the slot count goes over 18 and we know the data size is below 64K.
I took a stab at it this morning and came up with this patch. Ran redis-benchmark, it seemed to fix that for me -- only saw one "failed to linearize skb" during
redis-benchmark -h XXX -d 1000 -t lrange
And before this change, a lot of "rides rocket" were triggered.
Thought?
---8<---
Quoted 1 line
From
743495a2b2d338fc6cfe9bfd4b6e840392b87f4aMon Sep 17 00:00:00 2001
From: Wei Liu <[email protected]>
Date: Fri, 16 May 2014 10:39:01 +0100
Subject: [PATCH] xen-netfront: linearize SKB if it occupies too many slots
Some workload, such as Redis can generate SKBs which make use of compound pages. Netfront doesn't quite like that because it doesn't want to send exessive slots to the backend as backend might deem it malicious. On the flip side these packets are actually legit, the size check at the beginning of xennet_start_xmit ensures that packet size is below 64K.
So we linearize SKB if it occupies too many slots. If the linearization fails then the SKB is dropped.
--- drivers/net/xen-netfront.c | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-)
drivers/net/xen-netfront.c +15 -3
@@ -573,9 +573,21 @@ static int xennet_start_xmit(struct sk_buff *skb, struct net_device *dev) slots =DIV_ROUND_UP (offset + len,PAGE_SIZE ) +xennet_count_skb_frag_slots (skb);if (unlikely (slots >MAX_SKB_FRAGS +1 )) { -net_alert_ratelimited ( -"xennet: skb rides the rocket: %d slots\n" , slots); - goto drop; +if (skb_linearize (skb)) { +net_alert_ratelimited ( +"xennet: failed to linearize skb, skb dropped\n" ); + goto drop; + } + data = skb->data; + offset =offset_in_page (data); + len =skb_headlen (skb); + slots =DIV_ROUND_UP (offset + len,PAGE_SIZE ) + +xennet_count_skb_frag_slots (skb); +if (unlikely (slots >MAX_SKB_FRAGS +1 )) { +net_alert_ratelimited ( +"xennet: still too many slots after linerization: %d" , slots); + goto drop; + } }spin_lock_irqsave (&np->tx_lock, flags);
Signature · 1 line
1.7.10.4
Quoted 3 lines
-Stefan
Quoted 32 lines · Ian Campbell
Quoted 31 lines · Zoltan Kiss
Quoted 30 lines · Stefan Bader
We had reports about this message being seen on EC2 for a while but finally a reporter did notice some details about the guests and was able to provide a simple way to reproduce[1].
For my local experiments I use a Xen-4.2.2 based host (though I would say the host versions are not important). The host has one NIC which is used as the outgoing port of a Linux based (not openvswitch) bridge. And the PV guests use that bridge. I set the mtu to 9001 (which was seen on affected instance types) and also inside the guests. As described in the report one guests runs redis-server and the other nodejs through two scripts (for me I had to do the two sub.js calls in separate shells). After a bit the error messages appear on the guest running the redis-server.
I added some debug printk's to show a bit more detail about the skb and got the following (<length>@<offset (after masking off complete pages)>):
[ 698.108119] xen_netfront: xennet: skb rides the rocket: 19 slots [ 698.108134] header 1490@238 -> 1 slots [ 698.108139] frag #0 1614@2164 -> + 1 pages [ 698.108143] frag #1 3038@1296 -> + 2 pages [ 698.108147] frag #2 6076@1852 -> + 2 pages [ 698.108151] frag #3 6076@292 -> + 2 pages [ 698.108156] frag #4 6076@2828 -> + 3 pages [ 698.108160] frag #5 3038@1268 -> + 2 pages [ 698.108164] frag #6 2272@1824 -> + 1 pages [ 698.108168] frag #7 3804@0 -> + 1 pages [ 698.108172] frag #8 6076@264 -> + 2 pages [ 698.108177] frag #9 3946@2800 -> + 2 pages [ 698.108180] frags adding 18 slots
Quoted 7 lines
Quoted 4 lines
Quoted 3 lines
Since I am not deeply familiar with the networking code, I wonder about two things:
- is there something that should limit the skb data length from all frags to stay below the 64K which the definition ofMAX_SKB_FRAGShints?I think netfront should be able to handle 64K packets at most.
Ah, maybe this relates to this fix from Wei?
Yes, below patch limits SKB size to 64KB. However the problem here is not SKB exceeding 64KB. The said SKB is acutally 43KB in size. The problem is that guest kernel is using compound page so a frag which can be fit into one 4K page spans two 4K pages. The fix seems to be coalescing SKB in frontend, but it will degrade performance.
Wei.
Ian Campbell
Acked-byQuoted 31 lines · Zoltan Kiss
Quoted 30 lines · Stefan Bader
We had reports about this message being seen on EC2 for a while but finally a reporter did notice some details about the guests and was able to provide a simple way to reproduce[1].
For my local experiments I use a Xen-4.2.2 based host (though I would say the host versions are not important). The host has one NIC which is used as the outgoing port of a Linux based (not openvswitch) bridge. And the PV guests use that bridge. I set the mtu to 9001 (which was seen on affected instance types) and also inside the guests. As described in the report one guests runs redis-server and the other nodejs through two scripts (for me I had to do the two sub.js calls in separate shells). After a bit the error messages appear on the guest running the redis-server.
I added some debug printk's to show a bit more detail about the skb and got the following (<length>@<offset (after masking off complete pages)>):
[ 698.108119] xen_netfront: xennet: skb rides the rocket: 19 slots [ 698.108134] header 1490@238 -> 1 slots [ 698.108139] frag #0 1614@2164 -> + 1 pages [ 698.108143] frag #1 3038@1296 -> + 2 pages [ 698.108147] frag #2 6076@1852 -> + 2 pages [ 698.108151] frag #3 6076@292 -> + 2 pages [ 698.108156] frag #4 6076@2828 -> + 3 pages [ 698.108160] frag #5 3038@1268 -> + 2 pages [ 698.108164] frag #6 2272@1824 -> + 1 pages [ 698.108168] frag #7 3804@0 -> + 1 pages [ 698.108172] frag #8 6076@264 -> + 2 pages [ 698.108177] frag #9 3946@2800 -> + 2 pages [ 698.108180] frags adding 18 slots
Quoted 4 lines
Quoted 3 lines
Since I am not deeply familiar with the networking code, I wonder about two things:
- is there something that should limit the skb data length from all frags to stay below the 64K which the definition ofMAX_SKB_FRAGShints?I think netfront should be able to handle 64K packets at most.
Ah, maybe this relates to this fix from Wei?
commit 9ecd1a75d977e2e8c48139c7d3efed183f898d94
Author: Wei Liu <[email protected]> Date: Mon Apr 22 02:20:41 2013 +0000
xen-netfront: reduce gso_max_size to account for max TCP header
The maximum packet including header that can be handled by netfront / netback wire format is 65535. Reduce gso_max_size accordingly.
Drop skb and print warning when skb->len > 65535. This can 1) save the effort to send malformed packet to netback, 2) help spotting misconfiguration of netfront in the future.
drivers/net/xen-netfront.c +15 -2
@@ -36,7 +36,7 @@#include <linux/skbuff.h> #include <linux/ethtool.h> #include <linux/if_ether.h> -#include <linux/tcp.h> +#include <net/tcp.h> #include <linux/udp.h> #include <linux/moduleparam.h> #include <linux/mm.h>
@@ -547,6 +547,16 @@ static int xennet_start_xmit(struct sk_buff *skb, struct net_device *dev)unsigned int len =skb_headlen (skb);unsigned long flags; +/* If skb->len is too big for wire format, drop skb and alert +* user about misconfiguration. +*/ +if (unlikely (skb->len >XEN_NETIF_MAX_TX_SIZE )) { +net_alert_ratelimited ( +"xennet: skb->len = %u, too big for wire format\n" , + skb->len); + goto drop; + } + slots =DIV_ROUND_UP (offset + len,PAGE_SIZE ) +xennet_count_skb_frag_slots (skb);if (unlikely (slots >MAX_SKB_FRAGS +1 )) {
@@ -1058,7 +1068,8 @@ err:static int xennet_change_mtu (struct net_device *dev,int mtu) { -int max =xennet_can_sg (dev) ?65535 -ETH_HLEN :ETH_DATA_LEN ; +int max =xennet_can_sg (dev) ? +XEN_NETIF_MAX_TX_SIZE -MAX_TCP_HEADER :ETH_DATA_LEN ;if (mtu > max)return -EINVAL ;
@@ -1362,6 +1373,8 @@ static struct net_device *xennet_create_dev(struct xenbus_device *dev)SET_ETHTOOL_OPS (netdev, &xennet_ethtool_ops);SET_NETDEV_DEV (netdev, &dev->dev); +netif_set_gso_max_size (netdev,XEN_NETIF_MAX_TX_SIZE -MAX_TCP_HEADER ); + np->netdev = netdev;netif_carrier_off (netdev);
include/xen/interface/io/netif.h +1 -0
@@ -47,6 +47,7 @@#define _XEN_NETTXF_extra_info (3)#define XEN_NETTXF_extra_info (1U<<_XEN_NETTXF_extra_info) +#define XEN_NETIF_MAX_TX_SIZE 0xFFFFstruct xen_netif_tx_request {grant_ref_t gref;/* Reference to buffer page */ uint16_t offset;/* Offset within buffer page */
Quoted 30 lines · Stefan Bader
We had reports about this message being seen on EC2 for a while but finally a reporter did notice some details about the guests and was able to provide a simple way to reproduce[1].
For my local experiments I use a Xen-4.2.2 based host (though I would say the host versions are not important). The host has one NIC which is used as the outgoing port of a Linux based (not openvswitch) bridge. And the PV guests use that bridge. I set the mtu to 9001 (which was seen on affected instance types) and also inside the guests. As described in the report one guests runs redis-server and the other nodejs through two scripts (for me I had to do the two sub.js calls in separate shells). After a bit the error messages appear on the guest running the redis-server.
I added some debug printk's to show a bit more detail about the skb and got the following (<length>@<offset (after masking off complete pages)>):
[ 698.108119] xen_netfront: xennet: skb rides the rocket: 19 slots [ 698.108134] header 1490@238 -> 1 slots [ 698.108139] frag #0 1614@2164 -> + 1 pages [ 698.108143] frag #1 3038@1296 -> + 2 pages [ 698.108147] frag #2 6076@1852 -> + 2 pages [ 698.108151] frag #3 6076@292 -> + 2 pages [ 698.108156] frag #4 6076@2828 -> + 3 pages [ 698.108160] frag #5 3038@1268 -> + 2 pages [ 698.108164] frag #6 2272@1824 -> + 1 pages [ 698.108168] frag #7 3804@0 -> + 1 pages [ 698.108172] frag #8 6076@264 -> + 2 pages [ 698.108177] frag #9 3946@2800 -> + 2 pages [ 698.108180] frags adding 18 slots
Quoted 3 lines
Since I am not deeply familiar with the networking code, I wonder about two things:
- is there something that should limit the skb data length from all frags to stay below the 64K which the definition ofMAX_SKB_FRAGShints?
I think netfront should be able to handle 64K packets at most.
Quoted 1 line
- is multiple frags having offsets expected?
Yes, since compound pages a frag could over the 4K page boundary. The problem is, that in the netback/front protocol the assumption is that every slot is a single page, because grant operations could be done only on a 4K page. And every slot ends up as a frag (expect maybe the first, it can happen it is grant copied straight to the linear buffer), therefore the frontend cannot send an skb which occupies more than MAX_SKB_FRAGS individual 4k page. The problem is known for a while, the solution is not, unfortunately.
Quoted 12 lines
The latter is the problem here. If I did the maths right, the overall data size is around 41K. But since frags 1,4,5, and 9 have an offset big enough to require an additional page, the overall slot count goes up to 19.
If such a layout is valid, maybe the xen-netfront driver needs to reduce its
XEN_NETIF_MAX_TX_SIZEwhich currently is set to 64K? Or something else...-Stefan
[1] https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1317811