netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [RFC patch net-next] ipv4: use bcast as dst address in case IFF_NOARP is set
@ 2013-01-08 13:02 Jiri Pirko
  2013-01-08 17:11 ` Stephen Hemminger
  2013-01-08 22:27 ` David Miller
  0 siblings, 2 replies; 7+ messages in thread
From: Jiri Pirko @ 2013-01-08 13:02 UTC (permalink / raw)
  To: netdev; +Cc: davem, edumazet, shemminger, kuznet, jmorris, yoshfuji, kaber

When IFF_NOARP is set on a device, dev->dev_addr is used as *dst*
addr of sent frames. That does not make sense. Use rather bcast address
instead.

Signed-off-by: Jiri Pirko <jiri@resnulli.us>
---
 net/ipv4/arp.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/ipv4/arp.c b/net/ipv4/arp.c
index 9547a273..19d5ac2 100644
--- a/net/ipv4/arp.c
+++ b/net/ipv4/arp.c
@@ -290,11 +290,11 @@ static int arp_constructor(struct neighbour *neigh)
 		if (neigh->type == RTN_MULTICAST) {
 			neigh->nud_state = NUD_NOARP;
 			arp_mc_map(addr, neigh->ha, dev, 1);
-		} else if (dev->flags & (IFF_NOARP | IFF_LOOPBACK)) {
+		} else if (dev->flags & IFF_LOOPBACK) {
 			neigh->nud_state = NUD_NOARP;
 			memcpy(neigh->ha, dev->dev_addr, dev->addr_len);
 		} else if (neigh->type == RTN_BROADCAST ||
-			   (dev->flags & IFF_POINTOPOINT)) {
+			   (dev->flags & (IFF_POINTOPOINT | IFF_NOARP))) {
 			neigh->nud_state = NUD_NOARP;
 			memcpy(neigh->ha, dev->broadcast, dev->addr_len);
 		}
-- 
1.8.1

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [RFC patch net-next] ipv4: use bcast as dst address in case IFF_NOARP is set
  2013-01-08 13:02 [RFC patch net-next] ipv4: use bcast as dst address in case IFF_NOARP is set Jiri Pirko
@ 2013-01-08 17:11 ` Stephen Hemminger
  2013-01-08 17:45   ` Jiri Pirko
  2013-01-08 22:27 ` David Miller
  1 sibling, 1 reply; 7+ messages in thread
From: Stephen Hemminger @ 2013-01-08 17:11 UTC (permalink / raw)
  To: Jiri Pirko; +Cc: davem, edumazet, kuznet, jmorris, yoshfuji, kaber, netdev


> When IFF_NOARP is set on a device, dev->dev_addr is used as *dst*
> addr of sent frames. That does not make sense. Use rather bcast
> address
> instead.
> 
> Signed-off-by: Jiri Pirko <jiri@resnulli.us>

What did you test this on? I think this may have been
intentional to avoid broadcasting.

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [RFC patch net-next] ipv4: use bcast as dst address in case IFF_NOARP is set
  2013-01-08 17:11 ` Stephen Hemminger
@ 2013-01-08 17:45   ` Jiri Pirko
  2013-01-08 19:10     ` Stephen Hemminger
  2013-01-08 22:33     ` David Miller
  0 siblings, 2 replies; 7+ messages in thread
From: Jiri Pirko @ 2013-01-08 17:45 UTC (permalink / raw)
  To: Stephen Hemminger
  Cc: davem, edumazet, kuznet, jmorris, yoshfuji, kaber, netdev, pavlix

Tue, Jan 08, 2013 at 06:11:55PM CET, stephen.hemminger@vyatta.com wrote:
>
>> When IFF_NOARP is set on a device, dev->dev_addr is used as *dst*
>> addr of sent frames. That does not make sense. Use rather bcast
>> address
>> instead.
>> 
>> Signed-off-by: Jiri Pirko <jiri@resnulli.us>
>
>What did you test this on? I think this may have been
>intentional to avoid broadcasting.

Thanks for looking at this Stephen.

I tested this on two boxes connected via ethernet.

I believe this may have been intentional, but what sense does it have to
use dev_addr as destination address? That is what I do not understand.

Also, what is the issue with sending all packets to broadcast when NOARP is
set? In my opinion, it only makes sense.

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [RFC patch net-next] ipv4: use bcast as dst address in case IFF_NOARP is set
  2013-01-08 17:45   ` Jiri Pirko
@ 2013-01-08 19:10     ` Stephen Hemminger
  2013-01-08 21:51       ` Jan Engelhardt
  2013-01-08 22:33     ` David Miller
  1 sibling, 1 reply; 7+ messages in thread
From: Stephen Hemminger @ 2013-01-08 19:10 UTC (permalink / raw)
  To: Jiri Pirko
  Cc: davem, edumazet, kuznet, jmorris, yoshfuji, kaber, netdev, pavlix


> Tue, Jan 08, 2013 at 06:11:55PM CET, stephen.hemminger@vyatta.com
> wrote:
> >
> >> When IFF_NOARP is set on a device, dev->dev_addr is used as *dst*
> >> addr of sent frames. That does not make sense. Use rather bcast
> >> address
> >> instead.
> >> 
> >> Signed-off-by: Jiri Pirko <jiri@resnulli.us>
> >
> >What did you test this on? I think this may have been
> >intentional to avoid broadcasting.
> 
> Thanks for looking at this Stephen.
> 
> I tested this on two boxes connected via ethernet.
> 
> I believe this may have been intentional, but what sense does it have
> to
> use dev_addr as destination address? That is what I do not
> understand.
> 
> Also, what is the issue with sending all packets to broadcast when
> NOARP is
> set? In my opinion, it only makes sense.
> 

It looks like NOARP devices are point-to-point and don't really
have any concept of broadcast.

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [RFC patch net-next] ipv4: use bcast as dst address in case IFF_NOARP is set
  2013-01-08 19:10     ` Stephen Hemminger
@ 2013-01-08 21:51       ` Jan Engelhardt
  0 siblings, 0 replies; 7+ messages in thread
From: Jan Engelhardt @ 2013-01-08 21:51 UTC (permalink / raw)
  To: Stephen Hemminger
  Cc: Jiri Pirko, davem, edumazet, kuznet, jmorris, yoshfuji, kaber,
	netdev, pavlix


On Tuesday 2013-01-08 20:10, Stephen Hemminger wrote:
>> 
>>Also, what is the issue with sending all packets to broadcast when
>>NOARP is set? In my opinion, it only makes sense.
>
>It looks like NOARP devices are point-to-point and don't really have
>any concept of broadcast.

PTP links usually are NOARP, but NOARP does not necessarily imply
a PTP link.

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [RFC patch net-next] ipv4: use bcast as dst address in case IFF_NOARP is set
  2013-01-08 13:02 [RFC patch net-next] ipv4: use bcast as dst address in case IFF_NOARP is set Jiri Pirko
  2013-01-08 17:11 ` Stephen Hemminger
@ 2013-01-08 22:27 ` David Miller
  1 sibling, 0 replies; 7+ messages in thread
From: David Miller @ 2013-01-08 22:27 UTC (permalink / raw)
  To: jiri; +Cc: netdev, edumazet, shemminger, kuznet, jmorris, yoshfuji, kaber

From: Jiri Pirko <jiri@resnulli.us>
Date: Tue,  8 Jan 2013 14:02:42 +0100

> When IFF_NOARP is set on a device, dev->dev_addr is used as *dst*
> addr of sent frames. That does not make sense. Use rather bcast address
> instead.
> 
> Signed-off-by: Jiri Pirko <jiri@resnulli.us>

The address is actually arbitrary in this case, I believe the
current behavior is intentional, and even if it is not intentional
since the address is arbitrary this change carrys only potential
risk for zero gain.

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [RFC patch net-next] ipv4: use bcast as dst address in case IFF_NOARP is set
  2013-01-08 17:45   ` Jiri Pirko
  2013-01-08 19:10     ` Stephen Hemminger
@ 2013-01-08 22:33     ` David Miller
  1 sibling, 0 replies; 7+ messages in thread
From: David Miller @ 2013-01-08 22:33 UTC (permalink / raw)
  To: jiri
  Cc: stephen.hemminger, edumazet, kuznet, jmorris, yoshfuji, kaber,
	netdev, pavlix

From: Jiri Pirko <jiri@resnulli.us>
Date: Tue, 8 Jan 2013 18:45:38 +0100

> I believe this may have been intentional, but what sense does it have to
> use dev_addr as destination address? That is what I do not understand.

It's the only address we know isn't someone else's and is not broadcast.

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2013-01-08 22:33 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-01-08 13:02 [RFC patch net-next] ipv4: use bcast as dst address in case IFF_NOARP is set Jiri Pirko
2013-01-08 17:11 ` Stephen Hemminger
2013-01-08 17:45   ` Jiri Pirko
2013-01-08 19:10     ` Stephen Hemminger
2013-01-08 21:51       ` Jan Engelhardt
2013-01-08 22:33     ` David Miller
2013-01-08 22:27 ` 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).