[ovs-dev] [PATCH] lib/netdev-linux.c: Prevent receiving of sent packets

Murphy McCauley murphy.mccauley at gmail.com
Thu Jun 13 22:05:34 UTC 2013


On Jun 13, 2013, at 2:42 PM, Ben Pfaff wrote:

> On Thu, Jun 13, 2013 at 04:41:37PM +0900, Simon Horman wrote:
>> On Wed, Jun 12, 2013 at 07:30:44PM -0700, Murphy McCauley wrote:
>>> Commit 796223f5 (netdev: Add new "struct netdev_rx" for capturing packets
>>> from a netdev) refactored send and receive into separate netdevs.  As a
>>> result, send and receive now use different socket descriptors (except for tap
>>> interfaces which are treated specially).  An unintended side effect was that
>>> all sent packets are looped back and received, which had previously been
>>> avoided as the kernel specifically prevents this from happening on a single
>>> socket descriptor.
>>> 
>>> To resolve the situation, a socket filter is added to the receive socket
>>> so that it only accepts inbound packets.
>>> 
>>> Simon Horman co-discovered and initially reported this issue.
>>> 
>>> Signed-off-by: Murphy McCauley <murphy.mccauley at gmail.com>
>> 
>> Reviewed-by: Simon Horman <horms at verge.net.au>
> 
> I'll push this in a minute.

Great; thanks.

> I had to fold in the following to get it to build:
> 
> diff --git a/lib/netdev-linux.c b/lib/netdev-linux.c
> index 2753878..47bec9b 100644
> --- a/lib/netdev-linux.c
> +++ b/lib/netdev-linux.c
> @@ -22,6 +22,7 @@
> #include <fcntl.h>
> #include <arpa/inet.h>
> #include <inttypes.h>
> +#include <linux/filter.h>
> #include <linux/gen_stats.h>
> #include <linux/if_ether.h>
> #include <linux/if_tun.h>
> 
> I also folded in this update to the "sparse" fake headers to avoid a
> undefined SO_ATTACH_FILTER building with sparse:
> 
> diff --git a/include/sparse/sys/socket.h b/include/sparse/sys/socket.h
> index 13f61e5..64b905e 100644
> --- a/include/sparse/sys/socket.h
> +++ b/include/sparse/sys/socket.h
> @@ -1,5 +1,5 @@
> /*
> - * Copyright (c) 2011, 2012 Nicira, Inc.
> + * Copyright (c) 2011, 2012, 2013 Nicira, Inc.
>  *
>  * Licensed under the Apache License, Version 2.0 (the "License");
>  * you may not use this file except in compliance with the License.
> @@ -106,7 +106,8 @@ enum {
>     SO_SNDLOWAT,
>     SO_SNDTIMEO,
>     SO_TYPE,
> -    SO_RCVBUFFORCE
> +    SO_RCVBUFFORCE,
> +    SO_ATTACH_FILTER
> };
> 
> enum {
> 
> I also folded in the following to make the sock_filter readable and to
> fix a GCC complaint about an obsolete style:
> 
> @@ -746,12 +747,12 @@ netdev_linux_rx_open(struct netdev *netdev_, struct netdev_rx **rxp)
>         int ifindex;
>         /* Result of tcpdump -dd inbound */
>         static struct sock_filter filt[] = {
> -            { 0x28, 0, 0, 0xfffff004 },
> -            { 0x15, 0, 1, 0x00000004 },
> -            { 0x6, 0, 0, 0x00000000 },
> -            { 0x6, 0, 0, 0x0000ffff }
> +            { 0x28, 0, 0, 0xfffff004 }, /* ldh [0] */
> +            { 0x15, 0, 1, 0x00000004 }, /* jeq #4     jt 2  jf 3 */
> +            { 0x6, 0, 0, 0x00000000 },  /* ret #0 */
> +            { 0x6, 0, 0, 0x0000ffff }   /* ret #65535 */

Good idea.

>         };
> -        static struct sock_fprog fprog = {len: ARRAY_SIZE(filt), filter: filt};
> +        static struct sock_fprog fprog = { ARRAY_SIZE(filt), filt };

That's how I had it originally and decided I'd rather do it with designated assignment or whatever it's called.  I guess I had the GCC extension and C99 backwards in my head.  My C is pretty rusty; sorry!

>         /* Create file descriptor. */
>         fd = socket(PF_PACKET, SOCK_RAW, 0);




More information about the dev mailing list