patchlwIP - A Lightweight TCP/IP stack - Patches: patch #9283, Returning access modes with fcntl

 
 

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

patch #9283: Returning access modes with fcntl

Submitter:  Joan Lledó <jllledo>
Submitted:  Fri 10 Mar 2017 06:57:36 PM UTC
   
 
Category:  sockets/netconn Priority:  5 - Normal
Status:  Done Privacy:  Public
Assigned to:  goldsimon Open/Closed:  Closed
Planned Release:  None

Jump to the original submission

Wed 29 Mar 2017 06:54:57 PM UTC, comment #12: 

Pushed (and slightly improved). Thanks for the patch.

Simon Goldschmidt <goldsimon>
Group administrator
Wed 29 Mar 2017 10:51:46 AM UTC, comment #11: 

I've added the lock to access the pcb, but now it will return nothing for TCP sockets if LWIP_TCPIP_CORE_LOCKING is not set.

I could try to write a callback function to access the tcpip thread for this case, do you thing it is worth it or should we just return nothing?

(file #40190)

Joan Lledó <jllledo>
Tue 28 Mar 2017 08:27:21 PM UTC, comment #10: 

That looks a bit better.

> About the lock, I think I need your help here.


While a socket is only accessed from a single thread (although it is by now not limited to that any more), the underlying pcb is accessed from tcpip_thread as well. It can even be deallocated asynchronously (e.g. RST received, pcb closed in favour to allocating a higher prio pcb).

Simon Goldschmidt <goldsimon>
Group administrator
Tue 28 Mar 2017 04:56:07 PM UTC, comment #9: 

Hello Simon,

I've made some changes on the patch. Now it checks for null pointers and returns O_RDWR for UDP and raw sockets. It also returns O_RDWR when both O_RDONLY and O_WRONLY flags are set, because in some systems like Linux[1] O_RDWR is not equal to O_RDONLY|O_WRONLY.

About the lock, I think I need your help here. The code doesn't make changes on the socket, it only reads a flag from the pcb. And anyway, I read that it's not supported that two threads work on the same socket at the same time. What's the danger?

Thanks.

-----
[1] http://lxr.free-electrons.com/source/include/uapi/asm-generic/fcntl.h

(file #40182)

Joan Lledó <jllledo>
Mon 27 Mar 2017 11:12:20 AM UTC, comment #8: 

Hmm, that patch does not check for null pointers, does not care for threading/locking issues and does not return anything for udp/raw sockets...

Simon Goldschmidt <goldsimon>
Group administrator
Thu 23 Mar 2017 08:48:07 PM UTC, comment #7: 

Hi,

attached is a patch with the changes.

Regarding your question, the current stack 'pfinet' is running in a single task, but of course it is multithreaded. There's one thread that listens to incoming requests from user tasks and creates a new thread for attending each request.

The LwIP port works exactly the same way, each request gets a new thread that calls the proper sockets API function. I think that's enough since the sockets API is safe-thread, but I'd appreciate your comments.

(file #40101)

Joan Lledó <jllledo>
Thu 23 Mar 2017 09:43:22 AM UTC, comment #6: 

We do have other linux specialities, so if it's small, I'm OK with adding it...

OT: How does lwIP fit into a Hurd system when it comes to multiprocessor support? Or is it OK to run the network stack in a single task only?

Simon Goldschmidt <goldsimon>
Group administrator
Wed 22 Mar 2017 10:09:21 PM UTC, comment #5: 

Hi Simon,

I made some changes in my port to get the open modes from the TF_RXCLOSED and TF_FIN flags in netconn. You may see them in [1].

I thought you preferred to not include this feature to keep the stack small. That's why I said nothing but I could prepare a patch if you want.

---
[1] https://github.com/jlledom/lwip-hurd/commit/4332b1c93c6cd5cc0c9bffd3cb74523f68aff33b

Joan Lledó <jllledo>
Wed 22 Mar 2017 09:44:23 PM UTC, comment #4: 

Any progress or should I close this?

Simon Goldschmidt <goldsimon>
Group administrator
Sun 12 Mar 2017 07:22:50 PM UTC, comment #3: 

Hi Simon, thanks for your comments.

I'll do it that way in my port.

Regards.




Joan Lledó <jllledo>
Fri 10 Mar 2017 09:13:30 PM UTC, comment #2: 

EAM should have been RAM. My smartphone somehow changed that :-)

Simon Goldschmidt <goldsimon>
Group administrator
Fri 10 Mar 2017 09:07:18 PM UTC, comment #1: 

No, that's not the proper way. Not because you break things but because you increased EAM usage of each socket. The proper way would be to get the state from the underlying PCB or netconn.

I'm addition to that, I'm unsure if we should support all Linux specialities given it adds at least code size...

Simon Goldschmidt <goldsimon>
Group administrator
Fri 10 Mar 2017 06:57:36 PM UTC, original submission:  

Hello,

In my port I need lwip_fcntl() from sockets API to return the access mode when passing the F_GETFL command.

Attached is a patch with the changes I wrote in my lwip copy. I'm not sure if this is the proper way to do it or may be breaking anything.

I'd appreciate your comments.

Regards.

Joan Lledó <jllledo>

 

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

 

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 jllledo (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 7 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2017-03-29 goldsimon StatusNone Done
        Assigned toNone goldsimon
        Open/ClosedOpen Closed
    2017-03-29 jllledo Attached File- Added 0001-lwip_fcntl-returns-access-modes.patch, #40190
    2017-03-28 jllledo Attached File- Added 0001-lwip_fcntl-returns-access-modes.patch, #40182
    2017-03-23 jllledo Attached File- Added 0001-lwip_fcntl-returns-access-modes.patch, #40101
    2017-03-10 jllledo Attached File- Added 0001-fcntl-returns-access-modes.patch, #39968

    Back to the top

    Powered by Savane 3.13-f8d8.
    Corresponding source code