| Message ID | 1267624874-22326-2-git-send-email-timo.teras@iki.fi |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | David Miller |
| Headers | show |
Timo Teras wrote: > If device flag IFF_NOARP is changed, we should flush the ARP cache as all > entries need to get refreshed. > > Signed-off-by: Timo Teras <timo.teras@iki.fi> > --- > net/ipv4/arp.c | 3 +++ > 1 files changed, 3 insertions(+), 0 deletions(-) > > diff --git a/net/ipv4/arp.c b/net/ipv4/arp.c > index c4dd135..036da92 100644 > --- a/net/ipv4/arp.c > +++ b/net/ipv4/arp.c > @@ -1245,6 +1245,9 @@ static int arp_netdev_event(struct notifier_block *this, unsigned long event, vo > neigh_changeaddr(&arp_tbl, dev); > rt_cache_flush(dev_net(dev), 0); > break; > + case NETDEV_CHANGE: > + neigh_changeaddr(&arp_tbl, dev); > + break; It would be nice if we could restrict this to IFF_NOARP changes. -- 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
Patrick McHardy wrote: > Timo Teras wrote: >> If device flag IFF_NOARP is changed, we should flush the ARP cache as all >> entries need to get refreshed. >> >> Signed-off-by: Timo Teras <timo.teras@iki.fi> >> --- >> net/ipv4/arp.c | 3 +++ >> 1 files changed, 3 insertions(+), 0 deletions(-) >> >> diff --git a/net/ipv4/arp.c b/net/ipv4/arp.c >> index c4dd135..036da92 100644 >> --- a/net/ipv4/arp.c >> +++ b/net/ipv4/arp.c >> @@ -1245,6 +1245,9 @@ static int arp_netdev_event(struct notifier_block *this, unsigned long event, vo >> neigh_changeaddr(&arp_tbl, dev); >> rt_cache_flush(dev_net(dev), 0); >> break; >> + case NETDEV_CHANGE: >> + neigh_changeaddr(&arp_tbl, dev); >> + break; > > It would be nice if we could restrict this to IFF_NOARP changes. Yes. But I did not see any easy way to figure out which flags have changed. Should we just keep a copy of the previous IFF_NOARP bit somewhere (where?). Or did I miss something obvious? - Timo -- 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
Timo Teräs wrote: > Patrick McHardy wrote: >> Timo Teras wrote: >>> If device flag IFF_NOARP is changed, we should flush the ARP cache as >>> all >>> entries need to get refreshed. >>> >>> Signed-off-by: Timo Teras <timo.teras@iki.fi> >>> --- >>> net/ipv4/arp.c | 3 +++ >>> 1 files changed, 3 insertions(+), 0 deletions(-) >>> >>> diff --git a/net/ipv4/arp.c b/net/ipv4/arp.c >>> index c4dd135..036da92 100644 >>> --- a/net/ipv4/arp.c >>> +++ b/net/ipv4/arp.c >>> @@ -1245,6 +1245,9 @@ static int arp_netdev_event(struct >>> notifier_block *this, unsigned long event, vo >>> neigh_changeaddr(&arp_tbl, dev); >>> rt_cache_flush(dev_net(dev), 0); >>> break; >>> + case NETDEV_CHANGE: >>> + neigh_changeaddr(&arp_tbl, dev); >>> + break; >> >> It would be nice if we could restrict this to IFF_NOARP changes. > > Yes. But I did not see any easy way to figure out which flags have changed. > > Should we just keep a copy of the previous IFF_NOARP bit somewhere > (where?). > Or did I miss something obvious? We shouldn't have any arp entries for devices with IFF_NOARP set, so perhaps we can flush only in that case. The transition IFF_NOARP -> ~IFF_NOARP shouldn't need flushing. -- 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
Patrick McHardy wrote: > Timo Teräs wrote: >> Patrick McHardy wrote: >>> Timo Teras wrote: >>> It would be nice if we could restrict this to IFF_NOARP changes. >> Yes. But I did not see any easy way to figure out which flags have changed. >> >> Should we just keep a copy of the previous IFF_NOARP bit somewhere >> (where?). >> Or did I miss something obvious? > > We shouldn't have any arp entries for devices with IFF_NOARP set, > so perhaps we can flush only in that case. The transition IFF_NOARP > -> ~IFF_NOARP shouldn't need flushing. IFF_NOARP devices do have neighbor entries with the nud NOARP. Exactly those entries I want to flush when IFF_NOARP flag is removed. You can see those entries with "ip neigh show nud all". You have them e.g. for loopback stuff and broad-/multicast stuff in general. With IFF_NOARP you get them on all unicast addresses used. - Timo -- 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
Timo Teräs wrote: > Patrick McHardy wrote: >> Timo Teräs wrote: >>> Patrick McHardy wrote: >>>> Timo Teras wrote: >>>> It would be nice if we could restrict this to IFF_NOARP changes. >>> Yes. But I did not see any easy way to figure out which flags have >>> changed. >>> >>> Should we just keep a copy of the previous IFF_NOARP bit somewhere >>> (where?). >>> Or did I miss something obvious? >> >> We shouldn't have any arp entries for devices with IFF_NOARP set, >> so perhaps we can flush only in that case. The transition IFF_NOARP >> -> ~IFF_NOARP shouldn't need flushing. > > IFF_NOARP devices do have neighbor entries with the nud NOARP. > Exactly those entries I want to flush when IFF_NOARP flag is > removed. > > You can see those entries with "ip neigh show nud all". You have > them e.g. for loopback stuff and broad-/multicast stuff in general. > With IFF_NOARP you get them on all unicast addresses used. I see. I don't have a better suggestion, except perhaps to store the bit in dev->priv_flags. -- 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
Patrick McHardy wrote: > Timo Teräs wrote: >> Patrick McHardy wrote: >>> Timo Teräs wrote: >>>> Patrick McHardy wrote: >>>>> Timo Teras wrote: >>>>> It would be nice if we could restrict this to IFF_NOARP changes. >>>> Yes. But I did not see any easy way to figure out which flags have >>>> changed. >>>> >>>> Should we just keep a copy of the previous IFF_NOARP bit somewhere >>>> (where?). >>>> Or did I miss something obvious? >>> We shouldn't have any arp entries for devices with IFF_NOARP set, >>> so perhaps we can flush only in that case. The transition IFF_NOARP >>> -> ~IFF_NOARP shouldn't need flushing. >> IFF_NOARP devices do have neighbor entries with the nud NOARP. >> Exactly those entries I want to flush when IFF_NOARP flag is >> removed. >> >> You can see those entries with "ip neigh show nud all". You have >> them e.g. for loopback stuff and broad-/multicast stuff in general. >> With IFF_NOARP you get them on all unicast addresses used. > > I see. I don't have a better suggestion, except perhaps > to store the bit in dev->priv_flags. Ok. Should I make a patch that uses dev->priv_flags and repost? Or would make sense to have more generic way, e.g. keep copy of the previous flags in struct net_device so notifiers can check which ones changed (or would this have locking issues?). - Timo -- 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/net/ipv4/arp.c b/net/ipv4/arp.c index c4dd135..036da92 100644 --- a/net/ipv4/arp.c +++ b/net/ipv4/arp.c @@ -1245,6 +1245,9 @@ static int arp_netdev_event(struct notifier_block *this, unsigned long event, vo neigh_changeaddr(&arp_tbl, dev); rt_cache_flush(dev_net(dev), 0); break; + case NETDEV_CHANGE: + neigh_changeaddr(&arp_tbl, dev); + break; default: break; }
If device flag IFF_NOARP is changed, we should flush the ARP cache as all entries need to get refreshed. Signed-off-by: Timo Teras <timo.teras@iki.fi> --- net/ipv4/arp.c | 3 +++ 1 files changed, 3 insertions(+), 0 deletions(-)