Message ID | 20150512013424.GA7960@oracle.com |
---|---|
State | Not Applicable, archived |
Delegated to: | David Miller |
Headers | show |
On Mon, 2015-05-11 at 18:34 -0700, Venkat Venkatsubra wrote: > In ed1f50c3a ("net: add skb_checksum_setup") some checksum functions > were introduced in core. Subsequent change b5cf66cd1 ("xen-netfront: > use new skb_checksum_setup function") made use of those functions to > replace its own implementation. With that change ip_hdr() and tcp_hdr() > were not pointing at the right place. d554f73df ("xen-netfront: reset > skb network header before checksum") fixed the ip_hdr(). > > This patch fixes tcp_hdr(). > > We saw this problem when LRO was enabled on the host and it results in > netfront setting skb->ip_summed to CHECKSUM_UNNECESSARY. > > Signed-off-by: Venkat Venkatsubra <venkat.x.venkatsubra@oracle.com> > --- > drivers/net/xen-netfront.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/net/xen-netfront.c b/drivers/net/xen-netfront.c > index 3f45afd..ed5c242 100644 > --- a/drivers/net/xen-netfront.c > +++ b/drivers/net/xen-netfront.c > @@ -889,6 +889,7 @@ static int handle_incoming_queue(struct netfront_queue *queue, > /* Ethernet work: Delayed to here as it peeks the header. */ > skb->protocol = eth_type_trans(skb, queue->info->netdev); > skb_reset_network_header(skb); > + skb_set_transport_header(skb, ip_hdrlen(skb)); > > if (checksum_setup(queue->info->netdev, skb)) { > kfree_skb(skb); This looks bogus for IPv6. I would rather fix skb_checksum_setup_ip() and skb_checksum_setup_ipv6(), since you already use correctly skb_maybe_pull_tail() there. In general, setting network and transport header at this point seems extra work. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> > This looks bogus for IPv6. > > I would rather fix skb_checksum_setup_ip() and > skb_checksum_setup_ipv6(), since you already use correctly > skb_maybe_pull_tail() there. > > In general, setting network and transport header at this point seems > extra work. > Yes, I missed the IPv6 case. ;-) Thanks Eric for pointing that out. Looking further the below commit might have already resolved the issue. commit e5d5decaedf41f5b03e6819789cc3ba8744d9f54 Author: Jason Wang <jasowang@redhat.com> Date: Tue Mar 26 23:11:20 2013 +0000 net: core: let skb_partial_csum_set() set transport header For untrusted packets with partial checksum, we need to set the transport he for precise packet length estimation. We can just let skb_pratial_csum_set() do this to avoid extra call to skb_flow_dissect() and simplify the caller. Cc: Eric Dumazet <edumazet@google.com> Signed-off-by: Jason Wang <jasowang@redhat.com> Acked-by: Eric Dumazet <edumazet@google.com> Signed-off-by: David S. Miller <davem@davemloft.net> The guest we were running was older to this and didn't have the above commit. Thanks. Venkat -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/net/xen-netfront.c b/drivers/net/xen-netfront.c index 3f45afd..ed5c242 100644 --- a/drivers/net/xen-netfront.c +++ b/drivers/net/xen-netfront.c @@ -889,6 +889,7 @@ static int handle_incoming_queue(struct netfront_queue *queue, /* Ethernet work: Delayed to here as it peeks the header. */ skb->protocol = eth_type_trans(skb, queue->info->netdev); skb_reset_network_header(skb); + skb_set_transport_header(skb, ip_hdrlen(skb)); if (checksum_setup(queue->info->netdev, skb)) { kfree_skb(skb);
In ed1f50c3a ("net: add skb_checksum_setup") some checksum functions were introduced in core. Subsequent change b5cf66cd1 ("xen-netfront: use new skb_checksum_setup function") made use of those functions to replace its own implementation. With that change ip_hdr() and tcp_hdr() were not pointing at the right place. d554f73df ("xen-netfront: reset skb network header before checksum") fixed the ip_hdr(). This patch fixes tcp_hdr(). We saw this problem when LRO was enabled on the host and it results in netfront setting skb->ip_summed to CHECKSUM_UNNECESSARY. Signed-off-by: Venkat Venkatsubra <venkat.x.venkatsubra@oracle.com> --- drivers/net/xen-netfront.c | 1 + 1 file changed, 1 insertion(+)