buglwIP - A Lightweight TCP/IP stack - Bugs: bug #50064, Zero-copy RX: ARP reply fails with...

 
 

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

bug #50064: Zero-copy RX: ARP reply fails with PBUF_REF

Submitter:  Daniel Elstner <danielk>
Submitted:  Tue 17 Jan 2017 10:27:19 AM UTC
   
 
Category:  ARP Severity:  3 - Normal
Item Group:  Faulty Behaviour Status:  Fixed
Privacy:  Public Assigned to:  dziegel
Open/Closed:  Closed Planned Release:  2.0.2
lwIP version:  2.0.0

Jump to the original submission

Wed 18 Jan 2017 01:04:05 PM UTC, comment #10: 

Thanks for reporting and testing!

Dirk Ziegelmeier <dziegel>
Group administrator
Wed 18 Jan 2017 12:49:05 PM UTC, comment #9: 

Now it works, great. Thanks a lot!

Daniel Elstner <danielk>
Wed 18 Jan 2017 12:44:55 PM UTC, comment #8: 
Dirk Ziegelmeier <dziegel>
Group administrator
Wed 18 Jan 2017 12:34:44 PM UTC, comment #7: 

Just tested it and it doesn't work. Looking at Wireshark logs I can see ARP replies, but the Ethernet destination address is set to 00:00:00:00:00:00 so they end up nowhere.

Daniel Elstner <danielk>
Wed 18 Jan 2017 11:25:47 AM UTC, comment #6: 

I also vote to allocate a new PBUF

Dirk Ziegelmeier <dziegel>
Group administrator
Wed 18 Jan 2017 10:50:39 AM UTC, comment #5: 

No, its OK, etharp_raw() also has the AUTOIP special case built-in.

Dirk Ziegelmeier <dziegel>
Group administrator
Wed 18 Jan 2017 10:11:32 AM UTC, comment #4: 

Doesn't the fix break the LWIP_AUTOIP speciality of sending the response as broadcast?

I'd vote for allocating a new pbuf for the response instead of adding a speciality for PBUF_REF.

Simon Goldschmidt <goldsimon>
Group administrator
Wed 18 Jan 2017 09:16:03 AM UTC, comment #3: 

I pushed a fix, can you please test if it works for you, Daniel?

Dirk Ziegelmeier <dziegel>
Group administrator
Wed 18 Jan 2017 08:51:00 AM UTC, comment #2: 

Alternative 1: Removie the special case of reusing the received pbuf in etharp_input() and call etharp_raw() to create ARP response. This is less performant, but saves some code bytes. Is there a reason for optimizing the ARP reply case?

Alternative 2: Use an if clause to detect PBUF_REFs and only use etharp_raw() in this case, keeping the performance optimization for the PBUF_POOL case.

Any opinions?

Dirk Ziegelmeier <dziegel>
Group administrator
Tue 17 Jan 2017 11:16:41 AM UTC, comment #1: 

I messed it up in commit 31778193da09a4394749ed3d5cc20aa35c418b1f

Work on bug #48868: Cleanup etharp.c not to use struct eth_hdr: Cleanup etharp_input() to use ethernet_output()

Dirk Ziegelmeier <dziegel>
Group administrator
Tue 17 Jan 2017 10:27:19 AM UTC, original submission:  

After implementing zero-copy DMA in my Ethernet driver with custom PBUF_REF pbufs, ARP replies stopped working.

It turns out that etharp_input() re-uses the incoming pbuf for ARP replies. However, this leads to an error later in ethernet_output() because pbuf_header() refuses to adjust the header size for PBUF_REF pbufs.

As a quick hack, I replaced the pbuf_header() call in ethernet_output() by pbuf_header_force() and it worked. However, this essentially removes the error checking for pbufs from any other source, so it is not a proper solution. Also, it relies on the Ethernet driver to implicitly copy PBUF_REF pbufs to avoid potentially deadlocking the input queue.

So I think it would be best if etharp_input() itself would create the new pbuf. Either always (simplicity), or only for PBUF_REF if the small allocation is a concern.

Daniel Elstner <danielk>

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

No files currently attached

 

Depends on the following items: None found

Items that depend on this one: None found

 

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

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

     

    Follow 6 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2017-01-18 dziegel StatusReady For Test Fixed
        Open/ClosedOpen Closed
    2017-01-18 dziegel Assigned toNone dziegel
    2017-01-18 dziegel StatusNone Ready For Test
        Planned ReleaseNone 2.0.2
    2017-01-17 dziegel SummaryARP reply fails with PBUF_REF Zero-copy RX: ARP reply fails with PBUF_REF

    Back to the top

    Powered by Savane 3.13-f8d8.
    Corresponding source code