public inbox for netdev@vger.kernel.org
 help / color / mirror / Atom feed
From: "wanghai (M)" <wanghai38@huawei.com>
To: Ido Schimmel <idosch@idosch.org>
Cc: David Miller <davem@davemloft.net>,
	Jakub Kicinski <kuba@kernel.org>, <edumazet@google.com>,
	<yoshfuji@linux-ipv6.org>, <dsahern@kernel.org>,
	<netdev@vger.kernel.org>,
	linux-kernel <linux-kernel@vger.kernel.org>
Subject: Re: [BUG] net: ipv4: The sent udp broadcast message may be converted to an arp request message
Date: Tue, 8 Feb 2022 17:18:37 +0800	[thread overview]
Message-ID: <f4a0f41c-d315-22a8-ee1f-b12bbf56187a@huawei.com> (raw)
In-Reply-To: <YgIhGhh75mR5uLaS@shredder>


在 2022/2/8 15:51, Ido Schimmel 写道:
> On Mon, Feb 07, 2022 at 10:09:49PM +0800, wanghai (M) wrote:
>> Hello,
>>
>> I found a bug, but I don't know how to fix it. Anyone have some good ideas?
>>
>> This bug will cause udp broadcast messages not to be sent, but instead send
>> non-expected arp request messages.
>>
>> Deleting the ip while sending udp broadcast messages and then configuring
>> the ip again will probably trigger the bug.
>>
>> The following is the timing diagram of the bug, cpu0 sends a broadcast
>> message and cpu1 deletes the routing table at the appropriate time.
>>
>> cpu0                                        cpu1
>> send()
>>    udp_sendmsg()
>>      ip_route_output_flow()
>>      |  fib_lookup()
>>      udp_send_skb()
>>        ip_send_skb()
>>          ip_finish_output2()
>>
>>                                              ifconfig eth0:2 down
>>                                                fib_del_ifaddr
>>                                                  fib_table_delete // delete
>> fib table
>>
>>            ip_neigh_for_gw()
>>            |  ip_neigh_gw4()
>>            |    __ipv4_neigh_lookup_noref()
>>            |    __neigh_create()
>>            |      tbl->constructor(n) --> arp_constructor()
>>            |        neigh->type = inet_addr_type_dev_table(); // no route,
>> neigh->type = RTN_UNICAST
>>            neigh_output() // unicast, send an arp request and create an
>> exception arp entry
>>
>> After the above operation, an abnormal arp entry will be generated. If
>> the ip is configured again(ifconfig eth0:2 12.0.208.0), the abnormal arp
>> entry will still exist, and the udp broadcast message will be converted
>> to an arp request message when it is sent.
> Can you try the below? Not really happy with it, but don't have a better
> idea at the moment. It seemed better to handle it from the control path
> than augmenting the data path with more checks
>
> diff --git a/include/net/arp.h b/include/net/arp.h
> index 031374ac2f22..9e6a1961b64c 100644
> --- a/include/net/arp.h
> +++ b/include/net/arp.h
> @@ -64,6 +64,7 @@ void arp_send(int type, int ptype, __be32 dest_ip,
>   	      const unsigned char *dest_hw,
>   	      const unsigned char *src_hw, const unsigned char *th);
>   int arp_mc_map(__be32 addr, u8 *haddr, struct net_device *dev, int dir);
> +int arp_invalidate(struct net_device *dev, __be32 ip);
>   void arp_ifdown(struct net_device *dev);
>   
>   struct sk_buff *arp_create(int type, int ptype, __be32 dest_ip,
> diff --git a/net/ipv4/arp.c b/net/ipv4/arp.c
> index 4db0325f6e1a..b81665ce2b57 100644
> --- a/net/ipv4/arp.c
> +++ b/net/ipv4/arp.c
> @@ -1116,7 +1116,7 @@ static int arp_req_get(struct arpreq *r, struct net_device *dev)
>   	return err;
>   }
>   
> -static int arp_invalidate(struct net_device *dev, __be32 ip)
> +int arp_invalidate(struct net_device *dev, __be32 ip)
>   {
>   	struct neighbour *neigh = neigh_lookup(&arp_tbl, &ip, dev);
>   	int err = -ENXIO;
> diff --git a/net/ipv4/fib_frontend.c b/net/ipv4/fib_frontend.c
> index 4d61ddd8a0ec..2d7085232cb5 100644
> --- a/net/ipv4/fib_frontend.c
> +++ b/net/ipv4/fib_frontend.c
> @@ -1112,9 +1112,11 @@ void fib_add_ifaddr(struct in_ifaddr *ifa)
>   		return;
>   
>   	/* Add broadcast address, if it is explicitly assigned. */
> -	if (ifa->ifa_broadcast && ifa->ifa_broadcast != htonl(0xFFFFFFFF))
> +	if (ifa->ifa_broadcast && ifa->ifa_broadcast != htonl(0xFFFFFFFF)) {
>   		fib_magic(RTM_NEWROUTE, RTN_BROADCAST, ifa->ifa_broadcast, 32,
>   			  prim, 0);
> +		arp_invalidate(dev, ifa->ifa_broadcast);
> +	}
>   
>   	if (!ipv4_is_zeronet(prefix) && !(ifa->ifa_flags & IFA_F_SECONDARY) &&
>   	    (prefix != addr || ifa->ifa_prefixlen < 32)) {
> @@ -1128,6 +1130,7 @@ void fib_add_ifaddr(struct in_ifaddr *ifa)
>   		if (ifa->ifa_prefixlen < 31) {
>   			fib_magic(RTM_NEWROUTE, RTN_BROADCAST, prefix | ~mask,
>   				  32, prim, 0);
> +			arp_invalidate(dev, prefix | ~mask);
>   		}
>   	}
>   }

Thank you for your help. I tested it and it solved my problem.

Tested-by: Wang Hai <wanghai38@huawei.com>

> .
>
-- 
Wang Hai


  reply	other threads:[~2022-02-08  9:18 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-02-07 14:09 [BUG] net: ipv4: The sent udp broadcast message may be converted to an arp request message wanghai (M)
2022-02-08  7:51 ` Ido Schimmel
2022-02-08  9:18   ` wanghai (M) [this message]
2022-02-16  3:19   ` wanghai (M)
2022-02-16  7:29     ` Ido Schimmel
2022-02-16  8:03       ` wanghai (M)

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=f4a0f41c-d315-22a8-ee1f-b12bbf56187a@huawei.com \
    --to=wanghai38@huawei.com \
    --cc=davem@davemloft.net \
    --cc=dsahern@kernel.org \
    --cc=edumazet@google.com \
    --cc=idosch@idosch.org \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=yoshfuji@linux-ipv6.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox