Re: [patch net-next 14/17] mlxsw: spectrum_router: Add support for IPv6 routes addition / deletion
On Wed, Jul 19, 2017 at 10:36:52AM -0600, David Ahern wrote: > >> 2. How are routes with devices unrelated to ports owned by this driver > >> handled? > > > > They are handled just like any other route, but they don't have a valid > > RIF (for directly connected routes) or an adjacency group (for > > gatewayed routes), so the check in mlxsw_sp_fib_entry_should_offload() > > will return false and they will be programmed to the device with trap > > action, but using a trap ID (RTR_INGRESS0) with a lower traffic class > > than IP2ME, so packets that actually need to be locally received by the > > CPU have a better QoS. > > so mlxsw keeps a copy of the complete FIB for IPv4 and IPv6, even routes > unrelated to its ports? If we don't reflect all the routes in the system to the ASIC, then we'll have a broken routing table and a different behavior from what you would get with plain NICs.
Re: [patch net-next 14/17] mlxsw: spectrum_router: Add support for IPv6 routes addition / deletion
On 7/19/17 10:30 AM, Ido Schimmel wrote: >> rif == 0 means the dst device is not related to a port owned by this >> driver? > > Yes. > >> >> >> A lot to process so I am sure I missed the answer to these: >> >> 1. How do you handle host routes for local addresses? IPv6 inserts the >> host and anycast routes with the device set to 'lo' (or VRF device) >> instead of the device with the address. I have a patch to change this, >> but needs more testing > > In mlxsw_sp_fib6_entry_type_set() we check for RTF_LOCAL and set the > FIB entry type to MLXSW_SP_FIB_ENTRY_TYPE_TRAP. Packets hitting these > routes will be trapped with IP2ME trap ID towards the CPU. got it. thanks. > >> 2. How are routes with devices unrelated to ports owned by this driver >> handled? > > They are handled just like any other route, but they don't have a valid > RIF (for directly connected routes) or an adjacency group (for > gatewayed routes), so the check in mlxsw_sp_fib_entry_should_offload() > will return false and they will be programmed to the device with trap > action, but using a trap ID (RTR_INGRESS0) with a lower traffic class > than IP2ME, so packets that actually need to be locally received by the > CPU have a better QoS. so mlxsw keeps a copy of the complete FIB for IPv4 and IPv6, even routes unrelated to its ports?
Re: [patch net-next 14/17] mlxsw: spectrum_router: Add support for IPv6 routes addition / deletion
On Wed, Jul 19, 2017 at 10:14:54AM -0600, David Ahern wrote:
> On 7/19/17 1:02 AM, Jiri Pirko wrote:
> > @@ -2094,6 +2106,40 @@ mlxsw_sp_fib_entry_should_offload(const struct
> > mlxsw_sp_fib_entry *fib_entry)
> > }
> > }
> >
> > +static void
> > +mlxsw_sp_fib6_entry_offload_set(struct mlxsw_sp_fib_entry *fib_entry)
> > +{
> > + struct mlxsw_sp_fib6_entry *fib6_entry;
> > + struct mlxsw_sp_rt6 *mlxsw_sp_rt6;
> > +
> > + fib6_entry = container_of(fib_entry, struct mlxsw_sp_fib6_entry,
> > + common);
> > + list_for_each_entry(mlxsw_sp_rt6, _entry->rt6_list, list) {
> > + struct rt6_info *rt = mlxsw_sp_rt6->rt;
> > +
> > + write_lock_bh(>rt6i_table->tb6_lock);
> > + rt->rt6i_flags |= RTF_OFFLOAD;
> > + write_unlock_bh(>rt6i_table->tb6_lock);
>
> Seems wrong. A device driver should not be taking FIB table locks.
Will remove this in v2.
[...]
> > +static int mlxsw_sp_nexthop6_init(struct mlxsw_sp *mlxsw_sp,
> > + struct mlxsw_sp_nexthop_group *nh_grp,
> > + struct mlxsw_sp_nexthop *nh,
> > + const struct rt6_info *rt)
> > +{
> > + struct net_device *dev = rt->dst.dev;
> > + struct mlxsw_sp_rif *rif;
> > + int err;
> > +
> > + nh->nh_grp = nh_grp;
> > + memcpy(>gw_addr, >rt6i_gateway, sizeof(nh->gw_addr));
> > +
> > + if (!dev)
> > + return 0;
> > +
> > + rif = mlxsw_sp_rif_find_by_dev(mlxsw_sp, dev);
> > + if (!rif)
> > + return 0;
>
> rif == 0 means the dst device is not related to a port owned by this
> driver?
Yes.
>
>
> A lot to process so I am sure I missed the answer to these:
>
> 1. How do you handle host routes for local addresses? IPv6 inserts the
> host and anycast routes with the device set to 'lo' (or VRF device)
> instead of the device with the address. I have a patch to change this,
> but needs more testing
In mlxsw_sp_fib6_entry_type_set() we check for RTF_LOCAL and set the
FIB entry type to MLXSW_SP_FIB_ENTRY_TYPE_TRAP. Packets hitting these
routes will be trapped with IP2ME trap ID towards the CPU.
> 2. How are routes with devices unrelated to ports owned by this driver
> handled?
They are handled just like any other route, but they don't have a valid
RIF (for directly connected routes) or an adjacency group (for
gatewayed routes), so the check in mlxsw_sp_fib_entry_should_offload()
will return false and they will be programmed to the device with trap
action, but using a trap ID (RTR_INGRESS0) with a lower traffic class
than IP2ME, so packets that actually need to be locally received by the
CPU have a better QoS.
Re: [patch net-next 14/17] mlxsw: spectrum_router: Add support for IPv6 routes addition / deletion
On 7/19/17 1:02 AM, Jiri Pirko wrote:
> @@ -2094,6 +2106,40 @@ mlxsw_sp_fib_entry_should_offload(const struct
> mlxsw_sp_fib_entry *fib_entry)
> }
> }
>
> +static void
> +mlxsw_sp_fib6_entry_offload_set(struct mlxsw_sp_fib_entry *fib_entry)
> +{
> + struct mlxsw_sp_fib6_entry *fib6_entry;
> + struct mlxsw_sp_rt6 *mlxsw_sp_rt6;
> +
> + fib6_entry = container_of(fib_entry, struct mlxsw_sp_fib6_entry,
> + common);
> + list_for_each_entry(mlxsw_sp_rt6, _entry->rt6_list, list) {
> + struct rt6_info *rt = mlxsw_sp_rt6->rt;
> +
> + write_lock_bh(>rt6i_table->tb6_lock);
> + rt->rt6i_flags |= RTF_OFFLOAD;
> + write_unlock_bh(>rt6i_table->tb6_lock);
Seems wrong. A device driver should not be taking FIB table locks.
> + }
> +}
> +
> +static void
> +mlxsw_sp_fib6_entry_offload_unset(struct mlxsw_sp_fib_entry *fib_entry)
> +{
> + struct mlxsw_sp_fib6_entry *fib6_entry;
> + struct mlxsw_sp_rt6 *mlxsw_sp_rt6;
> +
> + fib6_entry = container_of(fib_entry, struct mlxsw_sp_fib6_entry,
> + common);
> + list_for_each_entry(mlxsw_sp_rt6, _entry->rt6_list, list) {
> + struct rt6_info *rt = mlxsw_sp_rt6->rt;
> +
> + write_lock_bh(>rt6i_table->tb6_lock);
> + rt->rt6i_flags &= ~RTF_OFFLOAD;
> + write_unlock_bh(>rt6i_table->tb6_lock);
same here.
> +static int mlxsw_sp_nexthop6_init(struct mlxsw_sp *mlxsw_sp,
> + struct mlxsw_sp_nexthop_group *nh_grp,
> + struct mlxsw_sp_nexthop *nh,
> + const struct rt6_info *rt)
> +{
> + struct net_device *dev = rt->dst.dev;
> + struct mlxsw_sp_rif *rif;
> + int err;
> +
> + nh->nh_grp = nh_grp;
> + memcpy(>gw_addr, >rt6i_gateway, sizeof(nh->gw_addr));
> +
> + if (!dev)
> + return 0;
> +
> + rif = mlxsw_sp_rif_find_by_dev(mlxsw_sp, dev);
> + if (!rif)
> + return 0;
rif == 0 means the dst device is not related to a port owned by this
driver?
A lot to process so I am sure I missed the answer to these:
1. How do you handle host routes for local addresses? IPv6 inserts the
host and anycast routes with the device set to 'lo' (or VRF device)
instead of the device with the address. I have a patch to change this,
but needs more testing
2. How are routes with devices unrelated to ports owned by this driver
handled?
