netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] net/hyperv: fix the issue that large packets be dropped by bridge
@ 2012-01-30  8:12 Wei Yongjun
  2012-02-01 19:28 ` David Miller
  0 siblings, 1 reply; 4+ messages in thread
From: Wei Yongjun @ 2012-01-30  8:12 UTC (permalink / raw)
  To: davem; +Cc: haiyangz, kys, netdev

From: Wei Yongjun <yongjun_wei@trendmicro.com.cn>

The packets with size larger than 1452 will be dropped by bridge
which with two hyperv netdevice ports. This cause by hyperv netvsc
driver always copy the trailer padding to the data packet, and then
the skb received from netdevice may include wrong skb->len (20 bytes
larger than the real size normally). The captured packet may like
this:

  Ethernet II, Src: Microsof_00:00:07 (00:15:5d:00:00:07),
               Dst: HewlettP_00:00:4e (00:1f:29:00:00:4e)
    Destination: HewlettP_e6:00:4e (00:1f:29:00:00:4e)
    Source: Microsof_f6:6d:07 (00:15:5d:f6:6d:07)
    Type: IP (0x0800)
    Trailer: 1415161718191A1B1C1D1E1F20212223
    Frame check sequence: 0x24252627 [incorrect, should be 0x7c2e5a5e]

The following command help to reproduction it, and the ping ICMP
packets will be dropped by bridge.
  $ ping ip -s 1453

This patch fixed it by removing the trailer padding from the data
packet.

Signed-off-by: Wei Yongjun <yongjun_wei@trendmicro.com.cn>
---
 drivers/net/hyperv/rndis_filter.c |   19 +++++++++++++++++++
 1 files changed, 19 insertions(+), 0 deletions(-)

diff --git a/drivers/net/hyperv/rndis_filter.c b/drivers/net/hyperv/rndis_filter.c
index da181f9..7568984 100644
--- a/drivers/net/hyperv/rndis_filter.c
+++ b/drivers/net/hyperv/rndis_filter.c
@@ -321,6 +321,25 @@ static void rndis_filter_receive_data(struct rndis_device *dev,
 	data_offset = RNDIS_HEADER_SIZE + rndis_pkt->data_offset;
 
 	pkt->total_data_buflen -= data_offset;
+
+	/*
+	 * Make sure we got a valid packet message, now total_data_buflen
+	 * should be the data packet size plus the trailer padding size
+	 */
+	if (pkt->total_data_buflen < rndis_pkt->data_len) {
+		netdev_err(dev->net_dev->ndev, "incoming packet message "
+			   "buffer overflow detected (got %u, min %u)"
+			   "...dropping this message!\n",
+			   pkt->total_data_buflen, rndis_pkt->data_len);
+		return;
+	}
+
+	/*
+	 * Remove the rndis trailer padding from packet message
+	 * rndis_pkt->data_len tell us the real data length, we only copy
+	 * the data packet to the stack, without the rndis trailer padding
+	 */
+	pkt->total_data_buflen = rndis_pkt->data_len;
 	pkt->data = (void *)((unsigned long)pkt->data + data_offset);
 
 	pkt->is_data_pkt = true;
-- 
1.7.1

^ permalink raw reply related	[flat|nested] 4+ messages in thread
* Re: [PATCH] net/hyperv: fix the issue that large packets be dropped by bridge
@ 2012-02-02  2:43 Wei Yongjun
  2012-02-02  4:07 ` David Miller
  0 siblings, 1 reply; 4+ messages in thread
From: Wei Yongjun @ 2012-02-02  2:43 UTC (permalink / raw)
  To: davem; +Cc: haiyangz, kys, netdev

On 02/02/2012 03:28 AM, David Miller wrote:
> From: Wei Yongjun <weiyj.lk@gmail.com>
> Date: Mon, 30 Jan 2012 16:12:45 +0800
>
>> +	/*
>> +	 * Make sure we got a valid packet message, now total_data_buflen
>> +	 * should be the data packet size plus the trailer padding size
>> +	 */
>> +	if (pkt->total_data_buflen < rndis_pkt->data_len) {
>> +		netdev_err(dev->net_dev->ndev, "incoming packet message "
>> +			   "buffer overflow detected (got %u, min %u)"
>> +			   "...dropping this message!\n",
>> +			   pkt->total_data_buflen, rndis_pkt->data_len);
>> +		return;
>> +	}
> This is inappropriate.  Any logging message that is potentially remotely
> triggerable must at a minimum be rate limited.  But to be honest I'd
> much prefer a statistic counter for this kind of event, rather then
> a log message.
Maybe the badly described error message confuse you, this error can not be
triggered remotely, it is the RNDIS control messages, which is sent
between the host and the RNDIS device.

Remain this error message may help us to detect the host device driver
bugs or the netvsc ringbuffer bugs.

So how about to change the error message?

+	/*
+	 * Make sure we got a valid RNDIS message, now total_data_buflen
+	 * should be the data packet size plus the trailer padding size
+	 */
+	if (pkt->total_data_buflen < rndis_pkt->data_len) {
+		netdev_err(dev->net_dev->ndev, "rndis message buffer "
+			   "overflow detected (got %u, min %u)"
+			   "...dropping this message!\n",
+			   pkt->total_data_buflen, rndis_pkt->data_len);
+		return;
+	}


Regards

^ permalink raw reply	[flat|nested] 4+ messages in thread
end of thread, other threads:[~2012-02-02  4:07 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-01-30  8:12 [PATCH] net/hyperv: fix the issue that large packets be dropped by bridge Wei Yongjun
2012-02-01 19:28 ` David Miller
  -- strict thread matches above, loose matches on Subject: below --
2012-02-02  2:43 Wei Yongjun
2012-02-02  4:07 ` David Miller

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).