feat: implement dgram for unix socket #164
Conversation
HeartLinked
requested review from
lhw2002426 and
ken4647
and removed request for
ken4647
modules/ruxnet/src/unix.rs
Outdated
| } | ||
|
|
||
| /// Get inode from addr, if not exist, create a new unix socket file | ||
| fn get_or_create_inode(&mut self, addr: &SocketAddrUnix) -> Result<usize, LinuxError> { |
There was a problem hiding this comment.
In fn get_inode , it will create file if not exist also, you can del one of them
There was a problem hiding this comment.
Upon closer inspection, I realized that this function is actually a wrapper around the two functions you previously wrote, get_inode and create_socket_file, rather than a direct replacement. In fact, this portion of the code was originally written directly within the bind function. However, since I noticed that the same code was needed in both the stream and dgram match arms, I extracted it and encapsulated it into a separate function. It's possible that the function name and comments I provided were not clear enough, leading to the misunderstanding. I have now updated the function's name and comment to: "Finds or creates the inode associated with the SocketAddrUnix address and updates the handle related to the socket." This should help avoid any further confusion.
modules/ruxnet/src/unix.rs
Outdated
| unimplemented!() | ||
| } | ||
|
|
||
| /// Returns the file descriptor for the socket. | ||
| fn fd(&self) -> c_int { |
There was a problem hiding this comment.
this func return unix_table index (inode number) instread of real file discriptor, I think there will be some problems?
There was a problem hiding this comment.
This function is not referenced elsewhere, and I can't recall why I added it in the first place. Alright, so I've gone ahead and deleted it, haha.
| pub addr: Mutex<SocketAddrUnix>, | ||
| pub buf: SocketBuffer<'a>, | ||
| pub peer_socket: Option<usize>, | ||
| pub status: UnixSocketStatus, | ||
|
|
||
| /// DGRAM socket, use a queue to store (source address, datagram). | ||
| pub datagram_queue: VecDeque<(SocketAddrUnix, Vec<u8>)>, |
There was a problem hiding this comment.
there is already a buf, is there necessary to use datagram_queue？
There was a problem hiding this comment.
For the DGRAM type, each message is an independent unit of data, known as a datagram. Typically, these messages are discrete, do not span across buffers, and each message must be tagged with its source address. This is distinctly different from STREAM sockets, which are connection-oriented and transmit data in a continuous stream. Therefore, I have designed a message queue that includes both the source address and the datagram for each message. I believe this approach should be appropriate, don't you think?
modules/ruxnet/src/unix.rs
Outdated
| target_inner | ||
| .datagram_queue | ||
| .push_back((source_addr, buf.to_vec())); | ||
| target_inner |
There was a problem hiding this comment.
This mistake was likely caused by an oversight on my part. fixed.
| return Err(LinuxError::EAGAIN); | ||
| } else { | ||
| // block until data is available | ||
| drop(inner); |
There was a problem hiding this comment.
can drop(inner) able to drop lock on uinx_table?
There was a problem hiding this comment.
I might not have a fully comprehensive understanding of Rust's locking mechanism, but here's my take: socket_inner is of type Arc<Mutex>. The inner variable acquires the Mutex lock on UnixSocketInner via socket_inner.lock(), and drop(inner) will release the lock on socket_inner. Additionally, there's no need to explicitly release the lock on UNIX_TABLE because Rust will automatically release it when its scope ends. So, I believe there shouldn't be an issue here, right?
HeartLinked
force-pushed
the
dev_unix_dgram
branch
from
e1589a5 to
555cfe0
Compare
No description provided.