Why do we need tasklet in IFB?

Why do we need tasklet in IFB?

[Michael Ma]

Hi -

Currently IFB uses tasklet to process tx/rx on the interface that
forwarded the packet to IFB. My understanding on why we're doing this
is that since dev_queue_xmit() can be invoked in interrupt, we want to
defer the processing of original tx/rx in case ifb_xmit() is called
from interrupt.

However, if the packet is originally from rx, calling context should
already be a tasklet and there is no need to queue the processing to
another tasklet anymore. Even if the packet is originated from tx, we
can rely on the deferred processing of the "original device" or TC if
necessary. Did I miss anything here?

Furthermore, looking at the bonding device's code there isn't this
kind of buffering/tasklet handling for packet forwarding even though
bond also has its own txq/rxq configured separately from the actual
nic, which is very similar to IFB.

So why do we need tasklet in IFB?

Thanks,
Michael

Re: Why do we need tasklet in IFB?

[Stephen Hemminger]

On Fri, 28 Oct 2016 14:36:27 -0700
Michael Ma <make0818@gmail.com> wrote:

> Hi -
> 
> Currently IFB uses tasklet to process tx/rx on the interface that
> forwarded the packet to IFB. My understanding on why we're doing this
> is that since dev_queue_xmit() can be invoked in interrupt, we want to
> defer the processing of original tx/rx in case ifb_xmit() is called
> from interrupt.

dev_queue_xmit is only called from interrupt if doing netconsole.

Re: Why do we need tasklet in IFB?

[Michael Ma]

2016-10-28 14:52 GMT-07:00 Michael Ma <make0818@gmail.com>:
> 2016-10-28 14:48 GMT-07:00 Stephen Hemminger <stephen@networkplumber.org>:
>> On Fri, 28 Oct 2016 14:45:07 -0700
>> Michael Ma <make0818@gmail.com> wrote:
>>
>>> 2016-10-28 14:38 GMT-07:00 Stephen Hemminger <stephen@networkplumber.org>:
>>> > On Fri, 28 Oct 2016 14:36:27 -0700
>>> > Michael Ma <make0818@gmail.com> wrote:
>>> >
>>> >> Hi -
>>> >>
>>> >> Currently IFB uses tasklet to process tx/rx on the interface that
>>> >> forwarded the packet to IFB. My understanding on why we're doing this
>>> >> is that since dev_queue_xmit() can be invoked in interrupt, we want to
>>> >> defer the processing of original tx/rx in case ifb_xmit() is called
>>> >> from interrupt.
>>> >
>>> > dev_queue_xmit is only called from interrupt if doing netconsole.
>>>

In fact this doesn't seem to explain since if the original path is tx
and the context is interrupt, IFB will call dev_queue_xmit as well so
the context can be interrupt in that case.

Then tasklet is still unnecessary.

>>> OK - so the reason is that netif_receive_skb() can only be invoked
>>> from softirq and we have to use tasklet in IFB to guarantee this.
>>>
>>> Then if the original path is rx, tasklet is unnecessary because
>>> ifb_xmit() is invoked from netif_receive_skb() which is always in the
>>> softirq context, right?
>>
>> The other reason is to avoid deep kernel callstacks

Re: Why do we need tasklet in IFB?

[Michael Ma]

2016-10-31 11:02 GMT-07:00 Michael Ma <make0818@gmail.com>:
> 2016-10-28 14:52 GMT-07:00 Michael Ma <make0818@gmail.com>:
>> 2016-10-28 14:48 GMT-07:00 Stephen Hemminger <stephen@networkplumber.org>:
>>> On Fri, 28 Oct 2016 14:45:07 -0700
>>> Michael Ma <make0818@gmail.com> wrote:
>>>
>>>> 2016-10-28 14:38 GMT-07:00 Stephen Hemminger <stephen@networkplumber.org>:
>>>> > On Fri, 28 Oct 2016 14:36:27 -0700
>>>> > Michael Ma <make0818@gmail.com> wrote:
>>>> >
>>>> >> Hi -
>>>> >>
>>>> >> Currently IFB uses tasklet to process tx/rx on the interface that
>>>> >> forwarded the packet to IFB. My understanding on why we're doing this
>>>> >> is that since dev_queue_xmit() can be invoked in interrupt, we want to
>>>> >> defer the processing of original tx/rx in case ifb_xmit() is called
>>>> >> from interrupt.
>>>> >
>>>> > dev_queue_xmit is only called from interrupt if doing netconsole.
>>>>
>
> In fact this doesn't seem to explain since if the original path is tx
> and the context is interrupt, IFB will call dev_queue_xmit as well so
> the context can be interrupt in that case.
>
> Then tasklet is still unnecessary.
>
>>>> OK - so the reason is that netif_receive_skb() can only be invoked
>>>> from softirq and we have to use tasklet in IFB to guarantee this.
>>>>
>>>> Then if the original path is rx, tasklet is unnecessary because
>>>> ifb_xmit() is invoked from netif_receive_skb() which is always in the
>>>> softirq context, right?
>>>
>>> The other reason is to avoid deep kernel callstacks

I see - this seems to be the ultimate reason but in case we know the
actual IFB configuration is simple then tasklet can still be avoided,
right?

Re: Why do we need tasklet in IFB?

[David Miller]

From: Michael Ma <make0818@gmail.com>
Date: Mon, 31 Oct 2016 11:02:28 -0700

> 2016-10-28 14:52 GMT-07:00 Michael Ma <make0818@gmail.com>:
>> 2016-10-28 14:48 GMT-07:00 Stephen Hemminger <stephen@networkplumber.org>:
>>> On Fri, 28 Oct 2016 14:45:07 -0700
>>> Michael Ma <make0818@gmail.com> wrote:
>>>
>>>> 2016-10-28 14:38 GMT-07:00 Stephen Hemminger <stephen@networkplumber.org>:
>>>> > On Fri, 28 Oct 2016 14:36:27 -0700
>>>> > Michael Ma <make0818@gmail.com> wrote:
>>>> >
>>>> >> Hi -
>>>> >>
>>>> >> Currently IFB uses tasklet to process tx/rx on the interface that
>>>> >> forwarded the packet to IFB. My understanding on why we're doing this
>>>> >> is that since dev_queue_xmit() can be invoked in interrupt, we want to
>>>> >> defer the processing of original tx/rx in case ifb_xmit() is called
>>>> >> from interrupt.
>>>> >
>>>> > dev_queue_xmit is only called from interrupt if doing netconsole.
>>>>
> 
> In fact this doesn't seem to explain since if the original path is tx
> and the context is interrupt, IFB will call dev_queue_xmit as well so
> the context can be interrupt in that case.
> 
> Then tasklet is still unnecessary.

Perhaps it's necessary to deal with potential recursion, that's my only
guess at this point.

Jamal, do you remember what went through your mind back in 2005? :-)

Re: Why do we need tasklet in IFB?

[Jamal Hadi Salim]

On 16-10-31 02:10 PM, David Miller wrote:
> From: Michael Ma <make0818@gmail.com>
> Date: Mon, 31 Oct 2016 11:02:28 -0700
>
>> 2016-10-28 14:52 GMT-07:00 Michael Ma <make0818@gmail.com>:
>>> 2016-10-28 14:48 GMT-07:00 Stephen Hemminger <stephen@networkplumber.org>:
>>>> On Fri, 28 Oct 2016 14:45:07 -0700
>>>> Michael Ma <make0818@gmail.com> wrote:
>>>>
>>>>> 2016-10-28 14:38 GMT-07:00 Stephen Hemminger <stephen@networkplumber.org>:
>>>>>> On Fri, 28 Oct 2016 14:36:27 -0700
>>>>>> Michael Ma <make0818@gmail.com> wrote:
>>>>>>
>>>>>>> Hi -
>>>>>>>
>>>>>>> Currently IFB uses tasklet to process tx/rx on the interface that
>>>>>>> forwarded the packet to IFB. My understanding on why we're doing this
>>>>>>> is that since dev_queue_xmit() can be invoked in interrupt, we want to
>>>>>>> defer the processing of original tx/rx in case ifb_xmit() is called
>>>>>>> from interrupt.
>>>>>>
>>>>>> dev_queue_xmit is only called from interrupt if doing netconsole.
>>>>>
>>
>> In fact this doesn't seem to explain since if the original path is tx
>> and the context is interrupt, IFB will call dev_queue_xmit as well so
>> the context can be interrupt in that case.
>>
>> Then tasklet is still unnecessary.
>
> Perhaps it's necessary to deal with potential recursion, that's my only
> guess at this point.
>
> Jamal, do you remember what went through your mind back in 2005? :-)
>

Certainly recursions (which were a huge issue in the original
implementations) as well as the need to have qdisc which shape traffic
(which implies they will sit on the queue for some period of time and
where accuracy of timing is important; therefore low latency of
scheduling was  needed).

I didnt follow the context of "tasklets are unnecessary":
Are tasklets bad or is the OP interested in replacing them with
something s/he thinks is better? IIRC, at some point the idea was to
infact use a softirq but it became obvious it was overkill.

cheers,
jamal

Re: Why do we need tasklet in IFB?

[Michael Ma]

2016-11-01 4:38 GMT-07:00 Jamal Hadi Salim <jhs@mojatatu.com>:
> On 16-10-31 02:10 PM, David Miller wrote:
>>
>> From: Michael Ma <make0818@gmail.com>
>> Date: Mon, 31 Oct 2016 11:02:28 -0700
>>
>>> 2016-10-28 14:52 GMT-07:00 Michael Ma <make0818@gmail.com>:
>>>>
>>>> 2016-10-28 14:48 GMT-07:00 Stephen Hemminger
>>>> <stephen@networkplumber.org>:
>>>>>
>>>>> On Fri, 28 Oct 2016 14:45:07 -0700
>>>>> Michael Ma <make0818@gmail.com> wrote:
>>>>>
>>>>>> 2016-10-28 14:38 GMT-07:00 Stephen Hemminger
>>>>>> <stephen@networkplumber.org>:
>>>>>>>
>>>>>>> On Fri, 28 Oct 2016 14:36:27 -0700
>>>>>>> Michael Ma <make0818@gmail.com> wrote:
>>>>>>>
>>>>>>>> Hi -
>>>>>>>>
>>>>>>>> Currently IFB uses tasklet to process tx/rx on the interface that
>>>>>>>> forwarded the packet to IFB. My understanding on why we're doing
>>>>>>>> this
>>>>>>>> is that since dev_queue_xmit() can be invoked in interrupt, we want
>>>>>>>> to
>>>>>>>> defer the processing of original tx/rx in case ifb_xmit() is called
>>>>>>>> from interrupt.
>>>>>>>
>>>>>>>
>>>>>>> dev_queue_xmit is only called from interrupt if doing netconsole.
>>>>>>
>>>>>>
>>>
>>> In fact this doesn't seem to explain since if the original path is tx
>>> and the context is interrupt, IFB will call dev_queue_xmit as well so
>>> the context can be interrupt in that case.
>>>
>>> Then tasklet is still unnecessary.
>>
>>
>> Perhaps it's necessary to deal with potential recursion, that's my only
>> guess at this point.
>>
>> Jamal, do you remember what went through your mind back in 2005? :-)
>>
>
> Certainly recursions (which were a huge issue in the original
> implementations) as well as the need to have qdisc which shape traffic
> (which implies they will sit on the queue for some period of time and
> where accuracy of timing is important; therefore low latency of
> scheduling was  needed).
>
Thanks for the explanation - I can get the point of avoiding
recursion. However latency wise if we don't have queue/tasklet in IFB
and rely on the upstream/downstream part (either qdisc or tx/rx) to do
the queueing, latency wouldn't be a problem anyway, or did I miss
anything here?

> I didnt follow the context of "tasklets are unnecessary":
> Are tasklets bad or is the OP interested in replacing them with
> something s/he thinks is better? IIRC, at some point the idea was to
> infact use a softirq but it became obvious it was overkill.

I was thinking of directly forwarding the packet in the thread context
of IFB since it's really an intermediate forwarding component and
additional queueing/tasklet doesn't seem to be necessary (let's put
the consideration of too deep call stack/recursion problem aside).

The problem that I have found is that queue selection on NIC will
actually be inherited in IFB and that affects how packets are handled
in tasklet since the tasklet is associated with each queue. We don't
really have a strict one to one mapping of cpu core to queue in NIC,
whereas in IFB case the queue is strictly mapped to tasklet/cpu core
which can cause undesirable contention.

The question here was raised mostly because I wanted to adopt the
thread context of packet in IFB.

>
> cheers,
> jamal