On Tue, Oct 10, 2017 at 09:41:04AM -0700, David Ahern wrote:
> Add support for inetaddr_validator and inet6addr_validator. The
> notifiers provide a means for validating ipv4 and ipv6 addresses
> before the addresses are installed and on failure the error
> is propagated back to the user.
> 
> Signed-off-by: David Ahern <dsah...@gmail.com>
> ---
>  drivers/net/ethernet/mellanox/mlxsw/spectrum.c     | 10 ++++
>  drivers/net/ethernet/mellanox/mlxsw/spectrum.h     |  4 ++
>  .../net/ethernet/mellanox/mlxsw/spectrum_router.c  | 53 
> ++++++++++++++++++++++
>  3 files changed, 67 insertions(+)
> 
> diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c 
> b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
> index 321988ac57cc..da4ee91235be 100644
> --- a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
> +++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
> @@ -4505,11 +4505,19 @@ static struct notifier_block mlxsw_sp_netdevice_nb 
> __read_mostly = {
>       .notifier_call = mlxsw_sp_netdevice_event,
>  };
>  
> +static struct notifier_block mlxsw_sp_inetaddr_valid_nb __read_mostly = {
> +     .notifier_call = mlxsw_sp_inetaddr_valid_event,
> +};
> +
>  static struct notifier_block mlxsw_sp_inetaddr_nb __read_mostly = {
>       .notifier_call = mlxsw_sp_inetaddr_event,
>       .priority = 10, /* Must be called before FIB notifier block */

This line can now be removed since the validator notifier is called
before the inetaddr notifier.

It's used to create a RIF before fib_add_ifaddr() is invoked to create a
prefix route. IPv6 doesn't need that since routes aren't created based
on a NETDEV_UP event in inet6addr.

>  };
>  
> +static struct notifier_block mlxsw_sp_inet6addr_valid_nb __read_mostly = {
> +     .notifier_call = mlxsw_sp_inet6addr_valid_event,
> +};
> +
>  static struct notifier_block mlxsw_sp_inet6addr_nb __read_mostly = {
>       .notifier_call = mlxsw_sp_inet6addr_event,
>  };
> @@ -4533,7 +4541,9 @@ static int __init mlxsw_sp_module_init(void)
>       int err;
>  
>       register_netdevice_notifier(&mlxsw_sp_netdevice_nb);
> +     register_inetaddr_validator_notifier(&mlxsw_sp_inetaddr_valid_nb);
>       register_inetaddr_notifier(&mlxsw_sp_inetaddr_nb);
> +     register_inet6addr_validator_notifier(&mlxsw_sp_inet6addr_valid_nb);
>       register_inet6addr_notifier(&mlxsw_sp_inet6addr_nb);
>       register_netevent_notifier(&mlxsw_sp_router_netevent_nb);

Need unregister in rollback and __exit function.

>  
> diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum.h 
> b/drivers/net/ethernet/mellanox/mlxsw/spectrum.h
> index 8e45183dc9bb..4865a6f58c83 100644
> --- a/drivers/net/ethernet/mellanox/mlxsw/spectrum.h
> +++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum.h
> @@ -390,8 +390,12 @@ int mlxsw_sp_router_netevent_event(struct notifier_block 
> *unused,
>  int mlxsw_sp_netdevice_router_port_event(struct net_device *dev);
>  int mlxsw_sp_inetaddr_event(struct notifier_block *unused,
>                           unsigned long event, void *ptr);
> +int mlxsw_sp_inetaddr_valid_event(struct notifier_block *unused,
> +                               unsigned long event, void *ptr);
>  int mlxsw_sp_inet6addr_event(struct notifier_block *unused,
>                            unsigned long event, void *ptr);
> +int mlxsw_sp_inet6addr_valid_event(struct notifier_block *unused,
> +                                unsigned long event, void *ptr);
>  int mlxsw_sp_netdevice_vrf_event(struct net_device *l3_dev, unsigned long 
> event,
>                                struct netdev_notifier_changeupper_info *info);
>  void
> diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c 
> b/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
> index 6a356f4b99a3..7d53fdf2c0a8 100644
> --- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
> +++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
> @@ -5656,6 +5656,33 @@ int mlxsw_sp_inetaddr_event(struct notifier_block 
> *unused,
>       struct mlxsw_sp_rif *rif;
>       int err = 0;
>  
> +     /* NETDEV_UP event is handled by mlxsw_sp_inetaddr_valid_event */
> +     if (event == NETDEV_UP)
> +             goto out;
> +
> +     mlxsw_sp = mlxsw_sp_lower_get(dev);
> +     if (!mlxsw_sp)
> +             goto out;
> +
> +     rif = mlxsw_sp_rif_find_by_dev(mlxsw_sp, dev);
> +     if (!mlxsw_sp_rif_should_config(rif, dev, event))
> +             goto out;
> +
> +     err = __mlxsw_sp_inetaddr_event(dev, event);
> +out:
> +     return notifier_from_errno(err);
> +}
> +
> +/* only expected to be called for event == NETDEV_UP */

I don't really mind, but I don't think the comment is necessary.

> +int mlxsw_sp_inetaddr_valid_event(struct notifier_block *unused,
> +                               unsigned long event, void *ptr)
> +{
> +     struct in_validator_info *ivi = (struct in_validator_info *)ptr;

We usually put a space after the cast in mlxsw (I'm aware of the
checkpatch CHECK).

> +     struct net_device *dev = ivi->ivi_dev->dev;
> +     struct mlxsw_sp *mlxsw_sp;
> +     struct mlxsw_sp_rif *rif;
> +     int err = 0;
> +
>       mlxsw_sp = mlxsw_sp_lower_get(dev);
>       if (!mlxsw_sp)
>               goto out;
> @@ -5708,6 +5735,10 @@ int mlxsw_sp_inet6addr_event(struct notifier_block 
> *unused,
>       struct mlxsw_sp_inet6addr_event_work *inet6addr_work;
>       struct net_device *dev = if6->idev->dev;
>  
> +     /* NETDEV_UP event is handled by mlxsw_sp_inet6addr_valid_event */
> +     if (event == NETDEV_UP)
> +             return NOTIFY_DONE;
> +
>       if (!mlxsw_sp_port_dev_lower_find_rcu(dev))
>               return NOTIFY_DONE;
>  
> @@ -5724,6 +5755,28 @@ int mlxsw_sp_inet6addr_event(struct notifier_block 
> *unused,
>       return NOTIFY_DONE;
>  }
>  
> +int mlxsw_sp_inet6addr_valid_event(struct notifier_block *unused,
> +                                unsigned long event, void *ptr)
> +{
> +     struct in6_validator_info *i6vi = (struct in6_validator_info *)ptr;

Same as above.

Looks good otherwise, thanks!

> +     struct net_device *dev = i6vi->i6vi_dev->dev;
> +     struct mlxsw_sp *mlxsw_sp;
> +     struct mlxsw_sp_rif *rif;
> +     int err = 0;
> +
> +     mlxsw_sp = mlxsw_sp_lower_get(dev);
> +     if (!mlxsw_sp)
> +             goto out;
> +
> +     rif = mlxsw_sp_rif_find_by_dev(mlxsw_sp, dev);
> +     if (!mlxsw_sp_rif_should_config(rif, dev, event))
> +             goto out;
> +
> +     err = __mlxsw_sp_inetaddr_event(dev, event);
> +out:
> +     return notifier_from_errno(err);
> +}
> +
>  static int mlxsw_sp_rif_edit(struct mlxsw_sp *mlxsw_sp, u16 rif_index,
>                            const char *mac, int mtu)
>  {
> -- 
> 2.1.4
> 

Reply via email to