Re: [PATCH net 1/2] net: ipv4: update fnhe_pmtu when first hop's MTU changes
2018-10-08, 11:18:49 -0600, David Ahern wrote:
> On 10/8/18 6:36 AM, Sabrina Dubroca wrote:
> > diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
> > index c7861e4b402c..dc9d2668d9bb 100644
> > --- a/include/linux/netdevice.h
> > +++ b/include/linux/netdevice.h
> > @@ -2458,6 +2458,13 @@ struct netdev_notifier_info {
> > struct netlink_ext_ack *extack;
> > };
> >
> > +struct netdev_notifier_info_ext {
> > + struct netdev_notifier_info info; /* must be first */
> > + union {
> > + u32 u32;
>
> I realize you want this to be generic, but that is a really odd
> definition. can you make that mtu instead? the union allows other use
> cases to add new names.
It might get ugly if we end up with 4 different u32, but ok, I'll
rename this and we can see how it evolves.
--
Sabrina
Re: [PATCH net 1/2] net: ipv4: update fnhe_pmtu when first hop's MTU changes
Hi Sabrina, Thank you for the patch! Perhaps something to improve: [auto build test WARNING on net/master] url: https://github.com/0day-ci/linux/commits/Sabrina-Dubroca/net-ipv4-update-fnhe_pmtu-when-first-hop-s-MTU-changes/20181008-225709 reproduce: make htmldocs All warnings (new ones prefixed by >>): net/mac80211/sta_info.h:588: warning: Function parameter or member 'status_stats.retry_failed' not described in 'sta_info' net/mac80211/sta_info.h:588: warning: Function parameter or member 'status_stats.retry_count' not described in 'sta_info' net/mac80211/sta_info.h:588: warning: Function parameter or member 'status_stats.lost_packets' not described in 'sta_info' net/mac80211/sta_info.h:588: warning: Function parameter or member 'status_stats.last_tdls_pkt_time' not described in 'sta_info' net/mac80211/sta_info.h:588: warning: Function parameter or member 'status_stats.msdu_retries' not described in 'sta_info' net/mac80211/sta_info.h:588: warning: Function parameter or member 'status_stats.msdu_failed' not described in 'sta_info' net/mac80211/sta_info.h:588: warning: Function parameter or member 'status_stats.last_ack' not described in 'sta_info' net/mac80211/sta_info.h:588: warning: Function parameter or member 'status_stats.last_ack_signal' not described in 'sta_info' net/mac80211/sta_info.h:588: warning: Function parameter or member 'status_stats.ack_signal_filled' not described in 'sta_info' net/mac80211/sta_info.h:588: warning: Function parameter or member 'status_stats.avg_ack_signal' not described in 'sta_info' net/mac80211/sta_info.h:588: warning: Function parameter or member 'tx_stats.packets' not described in 'sta_info' net/mac80211/sta_info.h:588: warning: Function parameter or member 'tx_stats.bytes' not described in 'sta_info' net/mac80211/sta_info.h:588: warning: Function parameter or member 'tx_stats.last_rate' not described in 'sta_info' net/mac80211/sta_info.h:588: warning: Function parameter or member 'tx_stats.msdu' not described in 'sta_info' include/linux/dma-buf.h:304: warning: Function parameter or member 'cb_excl.cb' not described in 'dma_buf' include/linux/dma-buf.h:304: warning: Function parameter or member 'cb_excl.poll' not described in 'dma_buf' include/linux/dma-buf.h:304: warning: Function parameter or member 'cb_excl.active' not described in 'dma_buf' include/linux/dma-buf.h:304: warning: Function parameter or member 'cb_shared.cb' not described in 'dma_buf' include/linux/dma-buf.h:304: warning: Function parameter or member 'cb_shared.poll' not described in 'dma_buf' include/linux/dma-buf.h:304: warning: Function parameter or member 'cb_shared.active' not described in 'dma_buf' include/linux/dma-fence-array.h:54: warning: Function parameter or member 'work' not described in 'dma_fence_array' include/linux/gpio/driver.h:142: warning: Function parameter or member 'request_key' not described in 'gpio_irq_chip' include/linux/iio/hw-consumer.h:1: warning: no structured comments found include/linux/input/sparse-keymap.h:46: warning: Function parameter or member 'sw' not described in 'key_entry' drivers/pci/pci.c:218: warning: Excess function parameter 'p' description in 'pci_dev_str_match_path' include/linux/regulator/driver.h:227: warning: Function parameter or member 'resume' not described in 'regulator_ops' drivers/regulator/core.c:4479: warning: Excess function parameter 'state' description in 'regulator_suspend' arch/s390/include/asm/cio.h:245: warning: Function parameter or member 'esw.esw0' not described in 'irb' arch/s390/include/asm/cio.h:245: warning: Function parameter or member 'esw.esw1' not described in 'irb' arch/s390/include/asm/cio.h:245: warning: Function parameter or member 'esw.esw2' not described in 'irb' arch/s390/include/asm/cio.h:245: warning: Function parameter or member 'esw.esw3' not described in 'irb' arch/s390/include/asm/cio.h:245: warning: Function parameter or member 'esw.eadm' not described in 'irb' drivers/slimbus/stream.c:1: warning: no structured comments found drivers/target/target_core_device.c:1: warning: no structured comments found drivers/usb/typec/bus.c:1: warning: no structured comments found drivers/usb/typec/class.c:1: warning: no structured comments found include/linux/w1.h:281: warning: Function parameter or member 'of_match_table' not described in 'w1_family' fs/direct-io.c:257: warning: Excess function parameter 'offset' description in 'dio_complete' fs/file_table.c:1: warning: no structured comments found fs/libfs.c:477: warning: Excess function parameter 'available' description in 'simple_write_end' fs/posix_acl.c:646: warning: Function parameter or member 'inode' not described in 'posix_acl_update_mode' fs/posix_acl.c:646: warning: Function parameter or member 'mode_p' not described in 'posix_acl_update_mode' fs/posix_acl.c:646: warning: Function parameter
Re: [PATCH net 1/2] net: ipv4: update fnhe_pmtu when first hop's MTU changes
From: Sabrina Dubroca
Date: Mon, 8 Oct 2018 14:36:38 +0200
> Since commit 5aad1de5ea2c ("ipv4: use separate genid for next hop
> exceptions"), exceptions get deprecated separately from cached
> routes. In particular, administrative changes don't clear PMTU anymore.
>
> As Stefano described in commit e9fa1495d738 ("ipv6: Reflect MTU changes
> on PMTU of exceptions for MTU-less routes"), the PMTU discovered before
> the local MTU change can become stale:
> - if the local MTU is now lower than the PMTU, that PMTU is now
>incorrect
> - if the local MTU was the lowest value in the path, and is increased,
>we might discover a higher PMTU
>
> Similarly to what commit e9fa1495d738 did for IPv6, update PMTU in those
> cases.
>
> If the exception was locked, discovered the PMTU was smaller than the
> minimal accepted PMTU. In that case, if the new local MTU is smaller
> than the current PMTU, let PMTU discovery figure out if locking of the
> exception is still needed.
>
> To do this, we need to know the old link MTU in the NETDEV_CHANGEMTU
> notifier. By the time the notifier is called, dev->mtu has been
> changed. This patch adds the old MTU as additional information in the
> notifier structure, and a new call_netdevice_notifiers_u32() function.
>
> Fixes: 5aad1de5ea2c ("ipv4: use separate genid for next hop exceptions")
> Signed-off-by: Sabrina Dubroca
> Reviewed-by: Stefano Brivio
Please, when you respin this to address David Ahern's feedback, provide
a proper "0/N" header posting.
Thank you.
Re: [PATCH net 1/2] net: ipv4: update fnhe_pmtu when first hop's MTU changes
On 10/8/18 6:36 AM, Sabrina Dubroca wrote:
> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
> index c7861e4b402c..dc9d2668d9bb 100644
> --- a/include/linux/netdevice.h
> +++ b/include/linux/netdevice.h
> @@ -2458,6 +2458,13 @@ struct netdev_notifier_info {
> struct netlink_ext_ack *extack;
> };
>
> +struct netdev_notifier_info_ext {
> + struct netdev_notifier_info info; /* must be first */
> + union {
> + u32 u32;
I realize you want this to be generic, but that is a really odd
definition. can you make that mtu instead? the union allows other use
cases to add new names.
> + } ext;
> +};
> +
> struct netdev_notifier_change_info {
> struct netdev_notifier_info info; /* must be first */
> unsigned int flags_changed;
