ARP resolving for switch drivers

ARP resolving for switch drivers

[David Miller]

Scott I looked into the current state of affairs and you should
be able to use generic infrastructure to resolve a neighbour
entry and even trigger the state machine.

For ipv4:

	n = __ipv4_neigh_lookup(dev, ip_addr);
	if (!n)
		n = neigh_create(&arp_tbl, &ip_addr, dev, true);
	if (!n)
		goto error;

	if (!(n->nud_state & NUD_VALID))
		neigh_event_send(n, NULL);
	else
		memcpy(&hw_entry->mac_addr, n->ha, dev_addr_len);

If you have to take the neigh_event_send() path, you have to wait for
the notifier to be invoked.  And in the notifier you can fetch the
MAC address.

You shouldn't have to hand craft ARP requests and listen for responses
or anything like that.

Re: ARP resolving for switch drivers

[Scott Feldman]

On Sat, Feb 21, 2015 at 10:11 PM, David Miller <davem@davemloft.net> wrote:
>
> Scott I looked into the current state of affairs and you should
> be able to use generic infrastructure to resolve a neighbour
> entry and even trigger the state machine.
>
> For ipv4:
>
>         n = __ipv4_neigh_lookup(dev, ip_addr);
>         if (!n)
>                 n = neigh_create(&arp_tbl, &ip_addr, dev, true);
>         if (!n)
>                 goto error;
>
>         if (!(n->nud_state & NUD_VALID))
>                 neigh_event_send(n, NULL);
>         else
>                 memcpy(&hw_entry->mac_addr, n->ha, dev_addr_len);
>
> If you have to take the neigh_event_send() path, you have to wait for
> the notifier to be invoked.  And in the notifier you can fetch the
> MAC address.

Perfect!  I'll give this a try shortly and test and report back.
(Trying to beat down some remaining L2 issues first).

I saw similar code in IB, but I don't understand the IB use-case.

> You shouldn't have to hand craft ARP requests and listen for responses
> or anything like that.

Thanks Dave.

-scott

Re: ARP resolving for switch drivers

[Scott Feldman]

On Sat, Feb 21, 2015 at 10:28 PM, Scott Feldman <sfeldma@gmail.com> wrote:
> On Sat, Feb 21, 2015 at 10:11 PM, David Miller <davem@davemloft.net> wrote:
>>
>> Scott I looked into the current state of affairs and you should
>> be able to use generic infrastructure to resolve a neighbour
>> entry and even trigger the state machine.
>>
>> For ipv4:
>>
>>         n = __ipv4_neigh_lookup(dev, ip_addr);
>>         if (!n)
>>                 n = neigh_create(&arp_tbl, &ip_addr, dev, true);
>>         if (!n)
>>                 goto error;
>>
>>         if (!(n->nud_state & NUD_VALID))
>>                 neigh_event_send(n, NULL);
>>         else
>>                 memcpy(&hw_entry->mac_addr, n->ha, dev_addr_len);
>>
>> If you have to take the neigh_event_send() path, you have to wait for
>> the notifier to be invoked.  And in the notifier you can fetch the
>> MAC address.
>
> Perfect!  I'll give this a try shortly and test and report back.
> (Trying to beat down some remaining L2 issues first).
>

David,

Just following up on this.  The code above works great, at least for
the initial nh resolution when route is installed.  I need to do some
more testing for the cases when the neigh entry goes to !NUD_VALID,
but I'm still holding a route with nhs looking for that neigh.  I
guess I need to re-trigger the above code.  Need to play around with
it.

There are two other items I had on my list for L3 from netconf:

1) Routes that overlap tables, or how to assign priority.  I'll look
at the RFC you sent for collapsing local/main tables.  For now, I used
fib_info->fib_priority for the priority in the rocker L3 table.  Maybe
that's good enough once local/main are collapsed?  But for other
tables, I'm not sure if fib_info->fib_priority will be sufficient.  I
probably need some guidance here.

2) Marking routes as "external"[1].  I added a RTAX_FEATURE_EXTERNAL
bit to be set by application (iproute2, quagga, etc) when wanting to
install route externally, for example to a switchdev switch.  Then if
application sets bit, it is the applications responsibility to handle
failure cases and rollback any lesser-prefixed routes than may have
been pushed earlier, successfully to external.  The problem I'm
running into is internal kernel routes need to be pushed to external
and I don't want to mark those as "external", but I also want those to
be install externally (to the device), if possible.


[1] "external" seems to be the label we're giving kernel objects that
get mirrored externally to a device, such as the mark we used for FDB
entries populated into the bridge's FDB by a learning device.

Re: ARP resolving for switch drivers

[roopa]

On 2/25/15, 9:08 PM, Scott Feldman wrote:
> On Sat, Feb 21, 2015 at 10:28 PM, Scott Feldman <sfeldma@gmail.com> wrote:
>> On Sat, Feb 21, 2015 at 10:11 PM, David Miller <davem@davemloft.net> wrote:
>>> Scott I looked into the current state of affairs and you should
>>> be able to use generic infrastructure to resolve a neighbour
>>> entry and even trigger the state machine.
>>>
>>> For ipv4:
>>>
>>>          n = __ipv4_neigh_lookup(dev, ip_addr);
>>>          if (!n)
>>>                  n = neigh_create(&arp_tbl, &ip_addr, dev, true);
>>>          if (!n)
>>>                  goto error;
>>>
>>>          if (!(n->nud_state & NUD_VALID))
>>>                  neigh_event_send(n, NULL);
>>>          else
>>>                  memcpy(&hw_entry->mac_addr, n->ha, dev_addr_len);
>>>
>>> If you have to take the neigh_event_send() path, you have to wait for
>>> the notifier to be invoked.  And in the notifier you can fetch the
>>> MAC address.
>> Perfect!  I'll give this a try shortly and test and report back.
>> (Trying to beat down some remaining L2 issues first).
>>
> David,
>
> Just following up on this.  The code above works great, at least for
> the initial nh resolution when route is installed.  I need to do some
> more testing for the cases when the neigh entry goes to !NUD_VALID,
> but I'm still holding a route with nhs looking for that neigh.  I
> guess I need to re-trigger the above code.  Need to play around with
> it.
>
> There are two other items I had on my list for L3 from netconf:
>
> 1) Routes that overlap tables, or how to assign priority.  I'll look
> at the RFC you sent for collapsing local/main tables.  For now, I used
> fib_info->fib_priority for the priority in the rocker L3 table.  Maybe
> that's good enough once local/main are collapsed?  But for other
> tables, I'm not sure if fib_info->fib_priority will be sufficient.  I
> probably need some guidance here.
>
> 2) Marking routes as "external"[1].  I added a RTAX_FEATURE_EXTERNAL
> bit to be set by application (iproute2, quagga, etc) when wanting to
> install route externally, for example to a switchdev switch.  Then if
> application sets bit, it is the applications responsibility to handle
> failure cases and rollback any lesser-prefixed routes than may have
> been pushed earlier, successfully to external.  The problem I'm
> running into is internal kernel routes need to be pushed to external
> and I don't want to mark those as "external", but I also want those to
> be install externally (to the device), if possible.
>
> [1] "external" seems to be the label we're giving kernel objects that
> get mirrored externally to a device, such as the mark we used for FDB
> entries populated into the bridge's FDB by a learning device.
scott,  don't mean to interrupt dave and your conversation here.
But, I am not seeing the hw learnt fdb entries to be same as
the routes from iproute2 or quagga.  In the fdb case, you had to mark it
external to indicate ownership of the entry to age it appropriately in 
the offload model
where both kernel and hw mirror fdb database and both can populate the 
fdb database.

I am not seeing the need to generalize the notion of  'external' used in 
fdb offloads across other sub-systems.
In this case  its the app (iproute2, quagga). What you want here is 
failure policy flags (as discussed @netdev01)
that user-space owns. And these should be optional with kernel having 
its own default policy (And default policy is where I am interested in :).

I guess I am re-iterating your intent but labeling it differently. :)

Thanks,
Roopa

Re: ARP resolving for switch drivers

[Scott Feldman]

On Wed, Feb 25, 2015 at 9:56 PM, roopa <roopa@cumulusnetworks.com> wrote:
> On 2/25/15, 9:08 PM, Scott Feldman wrote:
>>
>> On Sat, Feb 21, 2015 at 10:28 PM, Scott Feldman <sfeldma@gmail.com> wrote:
>>>
>>> On Sat, Feb 21, 2015 at 10:11 PM, David Miller <davem@davemloft.net>
>>> wrote:
>>>>
>>>> Scott I looked into the current state of affairs and you should
>>>> be able to use generic infrastructure to resolve a neighbour
>>>> entry and even trigger the state machine.
>>>>
>>>> For ipv4:
>>>>
>>>>          n = __ipv4_neigh_lookup(dev, ip_addr);
>>>>          if (!n)
>>>>                  n = neigh_create(&arp_tbl, &ip_addr, dev, true);
>>>>          if (!n)
>>>>                  goto error;
>>>>
>>>>          if (!(n->nud_state & NUD_VALID))
>>>>                  neigh_event_send(n, NULL);
>>>>          else
>>>>                  memcpy(&hw_entry->mac_addr, n->ha, dev_addr_len);
>>>>
>>>> If you have to take the neigh_event_send() path, you have to wait for
>>>> the notifier to be invoked.  And in the notifier you can fetch the
>>>> MAC address.
>>>
>>> Perfect!  I'll give this a try shortly and test and report back.
>>> (Trying to beat down some remaining L2 issues first).
>>>
>> David,
>>
>> Just following up on this.  The code above works great, at least for
>> the initial nh resolution when route is installed.  I need to do some
>> more testing for the cases when the neigh entry goes to !NUD_VALID,
>> but I'm still holding a route with nhs looking for that neigh.  I
>> guess I need to re-trigger the above code.  Need to play around with
>> it.
>>
>> There are two other items I had on my list for L3 from netconf:
>>
>> 1) Routes that overlap tables, or how to assign priority.  I'll look
>> at the RFC you sent for collapsing local/main tables.  For now, I used
>> fib_info->fib_priority for the priority in the rocker L3 table.  Maybe
>> that's good enough once local/main are collapsed?  But for other
>> tables, I'm not sure if fib_info->fib_priority will be sufficient.  I
>> probably need some guidance here.
>>
>> 2) Marking routes as "external"[1].  I added a RTAX_FEATURE_EXTERNAL
>> bit to be set by application (iproute2, quagga, etc) when wanting to
>> install route externally, for example to a switchdev switch.  Then if
>> application sets bit, it is the applications responsibility to handle
>> failure cases and rollback any lesser-prefixed routes than may have
>> been pushed earlier, successfully to external.  The problem I'm
>> running into is internal kernel routes need to be pushed to external
>> and I don't want to mark those as "external", but I also want those to
>> be install externally (to the device), if possible.
>>
>> [1] "external" seems to be the label we're giving kernel objects that
>> get mirrored externally to a device, such as the mark we used for FDB
>> entries populated into the bridge's FDB by a learning device.
>
> scott,  don't mean to interrupt dave and your conversation here.
> But, I am not seeing the hw learnt fdb entries to be same as
> the routes from iproute2 or quagga.  In the fdb case, you had to mark it
> external to indicate ownership of the entry to age it appropriately in the
> offload model
> where both kernel and hw mirror fdb database and both can populate the fdb
> database.
>
> I am not seeing the need to generalize the notion of  'external' used in fdb
> offloads across other sub-systems.
> In this case  its the app (iproute2, quagga). What you want here is failure
> policy flags (as discussed @netdev01)
> that user-space owns. And these should be optional with kernel having its
> own default policy (And default policy is where I am interested in :).
>
> I guess I am re-iterating your intent but labeling it differently. :)

I don't have a better label at this time.  "external" for both FDB and
FIB seems consistent to me since in both cases the kernel's DB (FDB or
FIB) has entries marked with "external" meaning they exist also in
some offload device's DB (FDB or FIB), and the device will offload the
fwd processing from the kernel.

-scott

Re: ARP resolving for switch drivers

[David Miller]

From: Scott Feldman <sfeldma@gmail.com>
Date: Thu, 26 Feb 2015 04:03:20 -0800

> I don't have a better label at this time.  "external" for both FDB and
> FIB seems consistent to me since in both cases the kernel's DB (FDB or
> FIB) has entries marked with "external" meaning they exist also in
> some offload device's DB (FDB or FIB), and the device will offload the
> fwd processing from the kernel.

I think allowing explicit control of entry loading into HW is a totally
separate piece of work from initial L3 offload support.

I want to see the most stupid implementation possible at the beginning.

Which means load entries until we cannot any more, and when we reach
that limit flush the entire HW table, stop offloading, and do
everything in software.

If you try to take the next step and allow for fine grained control,
and most intelligent policy on the kernel side of things, we're going
to go back and forth forever.

Get the basic stuff in now, and then we can argue about how to do the
rest meanwhile.

Re: ARP resolving for switch drivers

[David Miller]

From: Scott Feldman <sfeldma@gmail.com>
Date: Wed, 25 Feb 2015 21:08:30 -0800

> 1) Routes that overlap tables, or how to assign priority.  I'll look
> at the RFC you sent for collapsing local/main tables.  For now, I used
> fib_info->fib_priority for the priority in the rocker L3 table.  Maybe
> that's good enough once local/main are collapsed?  But for other
> tables, I'm not sure if fib_info->fib_priority will be sufficient.  I
> probably need some guidance here.

It should work with my local/main unification work.

You just have to abort hw offloading if any FIB rule modifications
happen. (ie. all the spots that set net->ipv4.fib_has_custom_rules)