buglwIP - A Lightweight TCP/IP stack - Bugs: bug #50698, Need separate sys_arch functions...

 
 

You are not allowed to post comments on this tracker with your current authentication level.

bug #50698: Need separate sys_arch functions for interrupt context

Submitted by:  Daniel Elstner <danielk>
Submitted on:  Sat 01 Apr 2017 07:28:36 PM UTC  
 
Category: Platform portsSeverity: 1 - Wish
Item Group: Feature RequestStatus: Fixed
Privacy: PublicAssigned to: Simon Goldschmidt <goldsimon>
Open/Closed: ClosedPlanned Release: None
lwIP version: git head

(Jump to the original submission Jump to the original submission)

Fri 05 Jan 2018 08:10:44 PM UTC, comment #6:

> Any idea of how you would propagate the "BaseType_t *pxHigherPriorityTaskWoken" to the original ISR


I've decided to let the port handle this via the sys_mbox_trypost_fromisr() return value (which e.g. can be positive when scheduling is needed - normally all errors are negative).

I've pushed the lwIP core change for this and will be pushing an adaption for contrib soon.

Simon Goldschmidt <goldsimon>
Project AdministratorIn charge of this item.
Thu 04 Jan 2018 04:45:06 PM UTC, comment #5:

Any idea of how you would propagate the "BaseType_t *pxHigherPriorityTaskWoken" to the original ISR? For Cortex ports, we could invoke the scheduler directly, but not for a generic FreeRTOS port...

Simon Goldschmidt <goldsimon>
Project AdministratorIn charge of this item.
Thu 04 Jan 2018 06:57:17 AM UTC, comment #4:

OK, reopened :-)

That solution is indeed good to save an RX task. I wasn't aware of the stupid naming of this function though (tcpip_try_callback vs tcpip_trycallback, not the missing '_') and thought that it could not work because tcpip_try_callback() calls memp_malloc()...

Maybe we should change the name of tcpip_trycallback().

Simon Goldschmidt <goldsimon>
Project AdministratorIn charge of this item.
Thu 04 Jan 2018 06:49:06 AM UTC, comment #3:

Well, my initial minimal proposal was to only add a new tcpip_trycallback_from_isr() API function, which would not require the pbuf alloc if I remember things correctly.

Daniel Elstner <danielk>
Thu 04 Jan 2018 06:41:29 AM UTC, comment #2:

Daniel, looking at this again I don't think it will work to adapt sys_arch to FreeRTOS:
- all code sections calling SYS_ARCH_PROTECT() would have to know if they are in interrupt or not (at least the Cortex ports check that taskENTER_CRITICAL() is not called from an ISR), and this is done from pbuf_alloc(), not only for "free from other context"
- functions that call "fromISR" code would have to return the bool that indicates if scheduling is required

I guess I'll implement an optional mechanism that keeps a global counter "inISR" instead...

Simon Goldschmidt <goldsimon>
Project AdministratorIn charge of this item.
Wed 05 Apr 2017 07:22:09 PM UTC, comment #1:

I'm aware of that freaky interrupt context interface FreeRTOS has. I don't know of any other OS doing it like that and honestly can't follow the argumentation of people telling me that doing it the FreeRTOS way is a performance improvement.

I do use FreeRTOS and like you, I don't need a change in lwIP (although it's fixed in a different way, not using DSRs, at least not on all platforms).

Nevertheless, since FreeRTOS seems to be often bundled with lwIP, I'm open for suggestions here.

Simon Goldschmidt <goldsimon>
Project AdministratorIn charge of this item.
Sat 01 Apr 2017 07:28:36 PM UTC, original submission:

Depending on the configuration, lwIP allows some operations to be executed from interrupt context. E.g. the static callback mechanism is intended to allow posting messages to the TCP/IP thread via tcpip_trycallback() directly from an interrupt handler.

However, I noticed that tcpip_trycallback() calls into the same sys_arch function -- sys_mbox_trypost() -- as tcpip_callback_with_block() does. Unfortunately, this makes it impossible to use this feature on systems which have separate API calls for user context and interrupt context, such as FreeRTOS.

Thus I think the sys_arch API needs to be extended with a new function, say sys_mbox_trypost_from_isr(). It would be optional and simply alias to sys_mbox_trypost() by default. On the public API side, a corresponding tcpip_trycallback_from_isr() would need to be added. Alternatively, tcpip_trycallback() could be changed to use the new function and documented as being callable from interrupt context only.

In order to support LWIP_ALLOW_MEM_FREE_FROM_OTHER_CONTEXT, there would also have to be separate versions of SYS_ARCH_(UN)PROTECT for use from interrupt context, as some FreeRTOS ports also require different calls for these. This would in turn also require separate ISR variants of memp_free() and pbuf_free(), so it may not be worth the effort.

Caveat: In my own design I delegate most interrupt work to high-priority threads which then in turn post messages to the lwIP thread. So I personally don't have any immediate need for this, and wouldn't be eating my own dog food if I contribute a patch.

Daniel Elstner <danielk>

 

No files currently attached

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -unavailable- added by goldsimon (Posted a comment)
  • -unavailable- added by danielk (Submitted the item)
  •  

    Do you think this task is very important?
    If so, you can click here to add your encouragement to it.
    This task has 0 encouragements so far.

    Only logged-in users can vote.

     

    Please enter the title of George Orwell's famous dystopian book (it's a date):

     

     

    Follow 8 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Fri 05 Jan 2018 08:10:44 PM UTCgoldsimonStatusIn Progress=>Fixed
      Open/ClosedOpen=>Closed
    Thu 04 Jan 2018 06:57:17 AM UTCgoldsimonStatusWont Fix=>In Progress
      Open/ClosedClosed=>Open
    Thu 04 Jan 2018 06:41:29 AM UTCgoldsimonStatusNone=>Wont Fix
      Assigned toNone=>goldsimon
      Open/ClosedOpen=>Closed
    Wed 05 Apr 2017 07:22:09 PM UTCgoldsimonSeverity3 - Normal=>1 - Wish

    Back to the top


    Powered by Savane 3.1-cleanup1