Hi John,
John Crispin <j...@phrozen.org> writes:
> @@ -237,6 +237,7 @@ struct switchdev_obj;
> struct switchdev_obj_port_fdb;
> struct switchdev_obj_port_mdb;
> struct switchdev_obj_port_vlan;
> +struct switchdev_obj_ipv4_fib;
Can you keep it ordered please (put obj_ipv4 above port_fdb).
>
> struct dsa_switch_ops {
> struct list_head list;
> @@ -386,6 +387,18 @@ struct dsa_switch_ops {
> int (*port_mdb_dump)(struct dsa_switch *ds, int port,
> struct switchdev_obj_port_mdb *mdb,
> int (*cb)(struct switchdev_obj *obj));
> +
> + /*
> + * IPV4 routing
> + */
> + int (*ipv4_fib_prepare)(struct dsa_switch *ds, int port,
> + const struct switchdev_obj_ipv4_fib *fib4,
> + struct switchdev_trans *trans);
> + int (*ipv4_fib_add)(struct dsa_switch *ds, int port,
> + const struct switchdev_obj_ipv4_fib *fib4,
> + struct switchdev_trans *trans);
DSA *_add ops should return void, since no error is supposed to occure in
the commit phase.
If they are port-based operations, please prefix them with "port_",
otherwise, the int port parameter is not necessary.
> + int (*ipv4_fib_del)(struct dsa_switch *ds, int port,
> + const struct switchdev_obj_ipv4_fib *fib4);
> };
>
> void register_switch_driver(struct dsa_switch_ops *type);
> diff --git a/net/dsa/slave.c b/net/dsa/slave.c
> index 9ecbe78..c974ac0 100644
> --- a/net/dsa/slave.c
> +++ b/net/dsa/slave.c
> @@ -334,6 +334,38 @@ static int dsa_slave_port_mdb_dump(struct net_device
> *dev,
> return -EOPNOTSUPP;
> }
>
> +static int dsa_slave_ipv4_fib_add(struct net_device *dev,
> + const struct switchdev_obj_ipv4_fib *fib4,
> + struct switchdev_trans *trans)
> +{
> + struct dsa_slave_priv *p = netdev_priv(dev);
> + struct dsa_switch *ds = p->parent;
> + int ret;
> +
> + if (!ds->ops->ipv4_fib_prepare || !ds->ops->ipv4_fib_add)
> + return -EOPNOTSUPP;
> +
> + if (switchdev_trans_ph_prepare(trans))
> + ret = ds->ops->ipv4_fib_prepare(ds, p->port, fib4, trans);
> + else
> + ret = ds->ops->ipv4_fib_add(ds, p->port, fib4, trans);
> +
> + return ret;
> +}
Please see dsa_slave_port_vlan_add for a better logic with the prepare
phase and void add routine.
> +
> +static int dsa_slave_ipv4_fib_del(struct net_device *dev,
> + const struct switchdev_obj_ipv4_fib *fib4)
> +{
> + struct dsa_slave_priv *p = netdev_priv(dev);
> + struct dsa_switch *ds = p->parent;
> + int ret = -EOPNOTSUPP;
> +
> + if (ds->ops->ipv4_fib_del)
> + ret = ds->ops->ipv4_fib_del(ds, p->port, fib4);
> +
> + return ret;
> +}
Just curious, isn't there a dump operation for SWITCHDEV_OBJ_ID_IPV4_FIB?
> +
> static int dsa_slave_ioctl(struct net_device *dev, struct ifreq *ifr, int
> cmd)
> {
> struct dsa_slave_priv *p = netdev_priv(dev);
> @@ -465,6 +497,11 @@ static int dsa_slave_port_obj_add(struct net_device *dev,
> SWITCHDEV_OBJ_PORT_VLAN(obj),
> trans);
> break;
> + case SWITCHDEV_OBJ_ID_IPV4_FIB:
> + err = dsa_slave_ipv4_fib_add(dev,
> + SWITCHDEV_OBJ_IPV4_FIB(obj),
> + trans);
> + break;
> default:
> err = -EOPNOTSUPP;
> break;
> @@ -490,6 +527,10 @@ static int dsa_slave_port_obj_del(struct net_device *dev,
> err = dsa_slave_port_vlan_del(dev,
> SWITCHDEV_OBJ_PORT_VLAN(obj));
> break;
> + case SWITCHDEV_OBJ_ID_IPV4_FIB:
> + err = dsa_slave_ipv4_fib_del(dev,
> + SWITCHDEV_OBJ_IPV4_FIB(obj));
> + break;
> default:
> err = -EOPNOTSUPP;
> break;
Please keep the SWITCHDEV_OBJ_ID_IPV4_FIB case ordered with other cases
as well.
I'm adding Jiri's in the loop, since he has started a thread on FIB
notifications a few days ago, his feedback might be interesting. If I'm
not mistaken, there is a plan to factorize FID routines (not sure).
Thanks,
Vivien
