YoSTEALTH / Liburing

Liburing is Python + Cython wrapper around C Liburing, which is a helper to setup and tear-down io_uring instances.
https://pypi.org/project/liburing/
Creative Commons Zero v1.0 Universal
98 stars 3 forks source link

helper: add `decode_sockaddr` function #9

Closed qweeze closed 4 years ago

YoSTEALTH commented 4 years ago

sockaddr_in only supports AF_INET https://github.com/YoSTEALTH/Liburing/commit/0796467db7d43942f57096d6f5e0706f3a0825d5

p.s also whats the use-case for this code?

qweeze commented 4 years ago

I tried to parse client's request in a webserver and realized that I have to extract its address from sockaddr. I wrote this function and thought maybe it would fit into helpers module since it already has sockaddr_in to convert ip, port to sockaddr and this is the inverse conversion example:

io_uring_prep_accept(sqe, fd, addr, addrlen, 0)
io_uring_submit(ring)
io_uring_wait_cqe(ring, cqes)
client_ip, client_port = decode_sockaddr(addr)
YoSTEALTH commented 4 years ago

Ok, I see.

I wrote this function and thought maybe it would fit into helpers module

I really appreciate this :)

I do have few concerns:

Sorry, I don't like to rush into things, if have to maintain this in the future I need to know what it does and provide some level of quality code for users.

qweeze commented 4 years ago

Thanks for the feedback!

I looked at socket.getpeername but it works only with python's socket.socket objects, I don't see how to get a socket.socket from a sockaddr struct. There's an internal function in socketmodule.c that could be used but it is not a part of public api - https://github.com/python/cpython/blob/master/Modules/socketmodule.c#L1294

I take it you are not wrapping your client with python's socket module

Sorry, I'm not quite following here, what client did you mention?

Shouldn't https://github.com/qweeze/py_uring_sandbox/blob/master/benchmarks/uring_server.py#L90 be inside https://github.com/qweeze/py_uring_sandbox/blob/master/benchmarks/uring_server.py#L31

Yes, it should, I guess now it just reuses the same addr for each new client so that works, but of course it's incorrect and I should fix it.

Anyway, I see now that this implementation is very premature so I think I'll close this PR for now

YoSTEALTH commented 4 years ago

Sorry, I'm not quite following here, what client did you mention?

This is a tiny sample code of how I wrote the Socket module https://github.com/YoSTEALTH/Shakti/blob/master/shakti/io/socket.py

I have already done a lot of what you are trying to do, Its just that I want the project to be a bit more stable before releasing to public.