Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Respect the number of bytes read per datagram when using recvmmsg #13399

Merged
merged 2 commits into from
May 24, 2023

Conversation

normanmaurer
Copy link
Member

Motivation:

We didnt correctly use the msg_len field after using recvmmsg. This resulted in incorrectly slicing the buffer and so returning larger datagrams then actually received.

Modifications:

  • Correctly use the msg_len field after recvmmsg
  • Adjust test to cover this defect.

Result:

Be able to use recvmmsg without the risk of generating invalid packets.

Motivation:

We didnt correctly use the msg_len field after using recvmmsg. This
resulted in incorrectly slicing the buffer and so returning larger
datagrams then actually received.

Modifications:

- Correctly use the msg_len field after recvmmsg
- Adjust test to cover this defect.

Result:

Be able to use recvmmsg without the risk of generating invalid packets.
Copy link
Contributor

@franz1981 franz1981 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How did you find you bud?

@normanmaurer
Copy link
Member Author

@franz1981 trying to improve quic perf and wondering why the packets were invalid.

@normanmaurer normanmaurer added this to the 4.1.93.Final milestone May 24, 2023
@Scottmitch
Copy link
Member

Scottmitch commented May 24, 2023

2023-05-24T14:35:00.6020139Z 14:35:00.600 [testsuite-epoll-busy-wait-22-2] ERROR io.netty.util.ResourceLeakDetector - LEAK: ByteBuf.release() was not called before it's garbage-collected. See https://netty.io/wiki/reference-counted-objects.html for more information.
2023-05-24T14:35:00.6020936Z Recent access records: 
2023-05-24T14:35:00.6021151Z #1:
2023-05-24T14:35:00.6021471Z 	io.netty.util.ReferenceCountUtil.release(ReferenceCountUtil.java:90)
2023-05-24T14:35:00.6024063Z 	io.netty.channel.DefaultAddressedEnvelope.release(DefaultAddressedEnvelope.java:99)
2023-05-24T14:35:00.6024601Z 	io.netty.util.ReferenceCountUtil.release(ReferenceCountUtil.java:90)
2023-05-24T14:35:00.6025136Z 	io.netty.channel.SimpleChannelInboundHandler.channelRead(SimpleChannelInboundHandler.java:106)
2023-05-24T14:35:00.6025789Z 	io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:444)
2023-05-24T14:35:00.6026480Z 	io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:420)
2023-05-24T14:35:00.6027150Z 	io.netty.channel.AbstractChannelHandlerContext.fireChannelRead(AbstractChannelHandlerContext.java:412)
2023-05-24T14:35:00.6027765Z 	io.netty.channel.DefaultChannelPipeline$HeadContext.channelRead(DefaultChannelPipeline.java:1410)
2023-05-24T14:35:00.6028369Z 	io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:440)
2023-05-24T14:35:00.6029105Z 	io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:420)
2023-05-24T14:35:00.6029820Z 	io.netty.channel.DefaultChannelPipeline.fireChannelRead(DefaultChannelPipeline.java:919)
2023-05-24T14:35:00.6030407Z 	io.netty.channel.epoll.EpollDatagramChannel.processPacket(EpollDatagramChannel.java:662)
2023-05-24T14:35:00.6030971Z 	io.netty.channel.epoll.EpollDatagramChannel.recvmsg(EpollDatagramChannel.java:697)
2023-05-24T14:35:00.6031509Z 	io.netty.channel.epoll.EpollDatagramChannel.access$300(EpollDatagramChannel.java:56)
2023-05-24T14:35:00.6032106Z 	io.netty.channel.epoll.EpollDatagramChannel$EpollDatagramChannelUnsafe.epollInReady(EpollDatagramChannel.java:536)
2023-05-24T14:35:00.6032677Z 	io.netty.channel.epoll.EpollEventLoop.processReady(EpollEventLoop.java:499)
2023-05-24T14:35:00.6033147Z 	io.netty.channel.epoll.EpollEventLoop.run(EpollEventLoop.java:397)
2023-05-24T14:35:00.6033646Z 	io.netty.util.concurrent.SingleThreadEventExecutor$4.run(SingleThreadEventExecutor.java:997)
2023-05-24T14:35:00.6034154Z 	io.netty.util.internal.ThreadExecutorMap$2.run(ThreadExecutorMap.java:74)
2023-05-24T14:35:00.6034655Z 	io.netty.util.concurrent.FastThreadLocalRunnable.run(FastThreadLocalRunnable.java:30)
2023-05-24T14:35:00.6035060Z 	java.lang.Thread.run(Thread.java:750)
2023-05-24T14:35:00.6035280Z #2:
2023-05-24T14:35:00.6035642Z 	io.netty.buffer.AdvancedLeakAwareByteBuf.getByte(AdvancedLeakAwareByteBuf.java:155)
2023-05-24T14:35:00.6036242Z 	io.netty.testsuite.transport.socket.DatagramUnicastInetTest$4$1.channelRead0(DatagramUnicastInetTest.java:126)
2023-05-24T14:35:00.6036880Z 	io.netty.testsuite.transport.socket.DatagramUnicastInetTest$4$1.channelRead0(DatagramUnicastInetTest.java:107)
2023-05-24T14:35:00.6037507Z 	io.netty.channel.SimpleChannelInboundHandler.channelRead(SimpleChannelInboundHandler.java:99)
2023-05-24T14:35:00.6038152Z 	io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:444)
2023-05-24T14:35:00.6038846Z 	io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:420)
2023-05-24T14:35:00.6039532Z 	io.netty.channel.AbstractChannelHandlerContext.fireChannelRead(AbstractChannelHandlerContext.java:412)
2023-05-24T14:35:00.6040142Z 	io.netty.channel.DefaultChannelPipeline$HeadContext.channelRead(DefaultChannelPipeline.java:1410)
2023-05-24T14:35:00.6040753Z 	io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:440)
2023-05-24T14:35:00.6041541Z 	io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:420)
2023-05-24T14:35:00.6042158Z 	io.netty.channel.DefaultChannelPipeline.fireChannelRead(DefaultChannelPipeline.java:919)
2023-05-24T14:35:00.6042812Z 	io.netty.channel.epoll.EpollDatagramChannel.processPacket(EpollDatagramChannel.java:662)
2023-05-24T14:35:00.6043363Z 	io.netty.channel.epoll.EpollDatagramChannel.recvmsg(EpollDatagramChannel.java:697)
2023-05-24T14:35:00.6043890Z 	io.netty.channel.epoll.EpollDatagramChannel.access$300(EpollDatagramChannel.java:56)
2023-05-24T14:35:00.6044648Z 	io.netty.channel.epoll.EpollDatagramChannel$EpollDatagramChannelUnsafe.epollInReady(EpollDatagramChannel.java:536)
2023-05-24T14:35:00.6045224Z 	io.netty.channel.epoll.EpollEventLoop.processReady(EpollEventLoop.java:499)
2023-05-24T14:35:00.6045685Z 	io.netty.channel.epoll.EpollEventLoop.run(EpollEventLoop.java:397)
2023-05-24T14:35:00.6046167Z 	io.netty.util.concurrent.SingleThreadEventExecutor$4.run(SingleThreadEventExecutor.java:997)
2023-05-24T14:35:00.6046668Z 	io.netty.util.internal.ThreadExecutorMap$2.run(ThreadExecutorMap.java:74)
2023-05-24T14:35:00.6047168Z 	io.netty.util.concurrent.FastThreadLocalRunnable.run(FastThreadLocalRunnable.java:30)
2023-05-24T14:35:00.6047576Z 	java.lang.Thread.run(Thread.java:750)
2023-05-24T14:35:00.6047809Z #3:
2023-05-24T14:35:00.6048267Z 	Hint: 'DatagramUnicastInetTest$4$1#0' will handle the message from this point.
2023-05-24T14:35:00.6048733Z 	io.netty.channel.DefaultAddressedEnvelope.touch(DefaultAddressedEnvelope.java:115)
2023-05-24T14:35:00.6049225Z 	io.netty.channel.socket.DatagramPacket.touch(DatagramPacket.java:85)
2023-05-24T14:35:00.6049669Z 	io.netty.channel.socket.DatagramPacket.touch(DatagramPacket.java:27)
2023-05-24T14:35:00.6050141Z 	io.netty.channel.DefaultChannelPipeline.touch(DefaultChannelPipeline.java:116)
2023-05-24T14:35:00.6050732Z 	io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:417)
2023-05-24T14:35:00.6051407Z 	io.netty.channel.AbstractChannelHandlerContext.fireChannelRead(AbstractChannelHandlerContext.java:412)
2023-05-24T14:35:00.6052000Z 	io.netty.channel.DefaultChannelPipeline$HeadContext.channelRead(DefaultChannelPipeline.java:1410)
2023-05-24T14:35:00.6052623Z 	io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:440)
2023-05-24T14:35:00.6053294Z 	io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:420)
2023-05-24T14:35:00.6053920Z 	io.netty.channel.DefaultChannelPipeline.fireChannelRead(DefaultChannelPipeline.java:919)
2023-05-24T14:35:00.6054496Z 	io.netty.channel.epoll.EpollDatagramChannel.processPacket(EpollDatagramChannel.java:662)
2023-05-24T14:35:00.6055048Z 	io.netty.channel.epoll.EpollDatagramChannel.recvmsg(EpollDatagramChannel.java:697)
2023-05-24T14:35:00.6055560Z 	io.netty.channel.epoll.EpollDatagramChannel.access$300(EpollDatagramChannel.java:56)
2023-05-24T14:35:00.6056150Z 	io.netty.channel.epoll.EpollDatagramChannel$EpollDatagramChannelUnsafe.epollInReady(EpollDatagramChannel.java:536)
2023-05-24T14:35:00.6056720Z 	io.netty.channel.epoll.EpollEventLoop.processReady(EpollEventLoop.java:499)
2023-05-24T14:35:00.6057184Z 	io.netty.channel.epoll.EpollEventLoop.run(EpollEventLoop.java:397)
2023-05-24T14:35:00.6057680Z 	io.netty.util.concurrent.SingleThreadEventExecutor$4.run(SingleThreadEventExecutor.java:997)
2023-05-24T14:35:00.6058181Z 	io.netty.util.internal.ThreadExecutorMap$2.run(ThreadExecutorMap.java:74)
2023-05-24T14:35:00.6058685Z 	io.netty.util.concurrent.FastThreadLocalRunnable.run(FastThreadLocalRunnable.java:30)
2023-05-24T14:35:00.6059079Z 	java.lang.Thread.run(Thread.java:750)
2023-05-24T14:35:00.6059309Z #4:
2023-05-24T14:35:00.6059705Z 	Hint: 'DefaultChannelPipeline$HeadContext#0' will handle the message from this point.
2023-05-24T14:35:00.6060178Z 	io.netty.channel.DefaultAddressedEnvelope.touch(DefaultAddressedEnvelope.java:115)
2023-05-24T14:35:00.6060728Z 	io.netty.channel.socket.DatagramPacket.touch(DatagramPacket.java:85)
2023-05-24T14:35:00.6061165Z 	io.netty.channel.socket.DatagramPacket.touch(DatagramPacket.java:27)
2023-05-24T14:35:00.6061697Z 	io.netty.channel.DefaultChannelPipeline.touch(DefaultChannelPipeline.java:116)
2023-05-24T14:35:00.6062290Z 	io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:417)
2023-05-24T14:35:00.6062914Z 	io.netty.channel.DefaultChannelPipeline.fireChannelRead(DefaultChannelPipeline.java:919)
2023-05-24T14:35:00.6063589Z 	io.netty.channel.epoll.EpollDatagramChannel.processPacket(EpollDatagramChannel.java:662)
2023-05-24T14:35:00.6064146Z 	io.netty.channel.epoll.EpollDatagramChannel.recvmsg(EpollDatagramChannel.java:697)
2023-05-24T14:35:00.6064669Z 	io.netty.channel.epoll.EpollDatagramChannel.access$300(EpollDatagramChannel.java:56)
2023-05-24T14:35:00.6065241Z 	io.netty.channel.epoll.EpollDatagramChannel$EpollDatagramChannelUnsafe.epollInReady(EpollDatagramChannel.java:536)
2023-05-24T14:35:00.6065814Z 	io.netty.channel.epoll.EpollEventLoop.processReady(EpollEventLoop.java:499)
2023-05-24T14:35:00.6066276Z 	io.netty.channel.epoll.EpollEventLoop.run(EpollEventLoop.java:397)
2023-05-24T14:35:00.6066771Z 	io.netty.util.concurrent.SingleThreadEventExecutor$4.run(SingleThreadEventExecutor.java:997)
2023-05-24T14:35:00.6067271Z 	io.netty.util.internal.ThreadExecutorMap$2.run(ThreadExecutorMap.java:74)
2023-05-24T14:35:00.6067773Z 	io.netty.util.concurrent.FastThreadLocalRunnable.run(FastThreadLocalRunnable.java:30)
2023-05-24T14:35:00.6068169Z 	java.lang.Thread.run(Thread.java:750)
2023-05-24T14:35:00.6068403Z #5:
2023-05-24T14:35:00.6068780Z 	io.netty.buffer.AdvancedLeakAwareByteBuf.retainedSlice(AdvancedLeakAwareByteBuf.java:95)
2023-05-24T14:35:00.6069427Z 	io.netty.channel.epoll.NativeDatagramPacketArray$NativeDatagramPacket.newDatagramPacket(NativeDatagramPacketArray.java:223)
2023-05-24T14:35:00.6070821Z 	io.netty.channel.epoll.EpollDatagramChannel.recvmsg(EpollDatagramChannel.java:695)
2023-05-24T14:35:00.6071356Z 	io.netty.channel.epoll.EpollDatagramChannel.access$300(EpollDatagramChannel.java:56)
2023-05-24T14:35:00.6071935Z 	io.netty.channel.epoll.EpollDatagramChannel$EpollDatagramChannelUnsafe.epollInReady(EpollDatagramChannel.java:536)
2023-05-24T14:35:00.6072513Z 	io.netty.channel.epoll.EpollEventLoop.processReady(EpollEventLoop.java:499)
2023-05-24T14:35:00.6072976Z 	io.netty.channel.epoll.EpollEventLoop.run(EpollEventLoop.java:397)
2023-05-24T14:35:00.6073470Z 	io.netty.util.concurrent.SingleThreadEventExecutor$4.run(SingleThreadEventExecutor.java:997)
2023-05-24T14:35:00.6073975Z 	io.netty.util.internal.ThreadExecutorMap$2.run(ThreadExecutorMap.java:74)
2023-05-24T14:35:00.6074475Z 	io.netty.util.concurrent.FastThreadLocalRunnable.run(FastThreadLocalRunnable.java:30)
2023-05-24T14:35:00.6074925Z 	java.lang.Thread.run(Thread.java:750)
2023-05-24T14:35:00.6075143Z #6:
2023-05-24T14:35:00.6075526Z 	io.netty.buffer.AdvancedLeakAwareByteBuf.nioBufferCount(AdvancedLeakAwareByteBuf.java:707)
2023-05-24T14:35:00.6075994Z 	io.netty.channel.unix.IovArray.add(IovArray.java:107)
2023-05-24T14:35:00.6076459Z 	io.netty.channel.epoll.NativeDatagramPacketArray.add0(NativeDatagramPacketArray.java:73)
2023-05-24T14:35:00.6077059Z 	io.netty.channel.epoll.NativeDatagramPacketArray.addWritable(NativeDatagramPacketArray.java:60)
2023-05-24T14:35:00.6077634Z 	io.netty.channel.epoll.EpollDatagramChannel.recvmsg(EpollDatagramChannel.java:681)
2023-05-24T14:35:00.6078147Z 	io.netty.channel.epoll.EpollDatagramChannel.access$300(EpollDatagramChannel.java:56)
2023-05-24T14:35:00.6078740Z 	io.netty.channel.epoll.EpollDatagramChannel$EpollDatagramChannelUnsafe.epollInReady(EpollDatagramChannel.java:536)
2023-05-24T14:35:00.6079307Z 	io.netty.channel.epoll.EpollEventLoop.processReady(EpollEventLoop.java:499)
2023-05-24T14:35:00.6079768Z 	io.netty.channel.epoll.EpollEventLoop.run(EpollEventLoop.java:397)
2023-05-24T14:35:00.6080264Z 	io.netty.util.concurrent.SingleThreadEventExecutor$4.run(SingleThreadEventExecutor.java:997)
2023-05-24T14:35:00.6080864Z 	io.netty.util.internal.ThreadExecutorMap$2.run(ThreadExecutorMap.java:74)
2023-05-24T14:35:00.6081354Z 	io.netty.util.concurrent.FastThreadLocalRunnable.run(FastThreadLocalRunnable.java:30)
2023-05-24T14:35:00.6081822Z 	java.lang.Thread.run(Thread.java:750)
2023-05-24T14:35:00.6082061Z Created at:
2023-05-24T14:35:00.6082460Z 	io.netty.buffer.UnpooledByteBufAllocator.newDirectBuffer(UnpooledByteBufAllocator.java:96)
2023-05-24T14:35:00.6083037Z 	io.netty.buffer.AbstractByteBufAllocator.directBuffer(AbstractByteBufAllocator.java:188)
2023-05-24T14:35:00.6083725Z 	io.netty.buffer.AbstractByteBufAllocator.directBuffer(AbstractByteBufAllocator.java:179)
2023-05-24T14:35:00.6084347Z 	io.netty.channel.unix.PreferredDirectByteBufAllocator.ioBuffer(PreferredDirectByteBufAllocator.java:53)
2023-05-24T14:35:00.6085077Z 	io.netty.channel.DefaultMaxMessagesRecvByteBufAllocator$MaxMessageHandle.allocate(DefaultMaxMessagesRecvByteBufAllocator.java:120)
2023-05-24T14:35:00.6085779Z 	io.netty.channel.epoll.EpollRecvByteAllocatorHandle.allocate(EpollRecvByteAllocatorHandle.java:75)
2023-05-24T14:35:00.6086429Z 	io.netty.channel.epoll.EpollDatagramChannel$EpollDatagramChannelUnsafe.epollInReady(EpollDatagramChannel.java:528)
2023-05-24T14:35:00.6086998Z 	io.netty.channel.epoll.EpollEventLoop.processReady(EpollEventLoop.java:499)
2023-05-24T14:35:00.6087462Z 	io.netty.channel.epoll.EpollEventLoop.run(EpollEventLoop.java:397)
2023-05-24T14:35:00.6087959Z 	io.netty.util.concurrent.SingleThreadEventExecutor$4.run(SingleThreadEventExecutor.java:997)
2023-05-24T14:35:00.6088454Z 	io.netty.util.internal.ThreadExecutorMap$2.run(ThreadExecutorMap.java:74)
2023-05-24T14:35:00.6088954Z 	io.netty.util.concurrent.FastThreadLocalRunnable.run(FastThreadLocalRunnable.java:30)
2023-05-24T14:35:00.6089360Z 	java.lang.Thread.run(Thread.java:750)
2023-05-24T14:35:00.6089672Z : 3 leak records were discarded because they were duplicates
2023-05-24T14:35:00.6125513Z 14:35:00.609 [testsuite-epoll-busy-wait-22-2] ERROR io.netty.util.ResourceLeakDetector - LEAK: ByteBuf.release() was not called before it's garbage-collected. See https://netty.io/wiki/reference-counted-objects.html for more information.
...

@normanmaurer normanmaurer merged commit 4eda913 into 4.1 May 24, 2023
14 checks passed
@normanmaurer normanmaurer deleted the recvmmsg_bug branch May 24, 2023 18:59
normanmaurer added a commit that referenced this pull request May 25, 2023
…3399)

Motivation:

We didnt correctly use the msg_len field after using recvmmsg. This
resulted in incorrectly slicing the buffer and so returning larger
datagrams then actually received.

Modifications:

- Correctly use the msg_len field after recvmmsg
- Adjust test to cover this defect.

Result:

Be able to use recvmmsg without the risk of generating invalid packets.
dongjoon-hyun pushed a commit to apache/spark that referenced this pull request Jun 28, 2023
### What changes were proposed in this pull request?
This pr aims to upgrade netty from 4.1.92 to 4.1.93.

### Why are the changes needed?
1.v4.1.92 VS v4.1.93
netty/netty@netty-4.1.92.Final...netty-4.1.93.Final

2.The new version brings some bug fix, eg:
- Reset byte buffer in loop for AbstractDiskHttpData.setContent ([#13320](netty/netty#13320))
- OpenSSL MAX_CERTIFICATE_LIST_BYTES option supported ([#13365](netty/netty#13365))
- Adapt to DirectByteBuffer constructor in Java 21 ([#13366](netty/netty#13366))
- HTTP/2 encoder: allow HEADER_TABLE_SIZE greater than Integer.MAX_VALUE ([#13368](netty/netty#13368))
- Upgrade to latest netty-tcnative to fix memory leak ([#13375](netty/netty#13375))
- H2/H2C server stream channels deactivated while write still in progress ([#13388](netty/netty#13388))
- Channel#bytesBefore(un)writable off by 1 ([#13389](netty/netty#13389))
- HTTP/2 should forward shutdown user events to active streams ([#13394](netty/netty#13394))
- Respect the number of bytes read per datagram when using recvmmsg ([#13399](netty/netty#13399))

3.The release notes as follows:
- https://netty.io/news/2023/05/25/4-1-93-Final.html

4.Why not upgrade to `4-1-94-Final` version?
Because the return value of the 'threadCache()' (from `PoolThreadCache` to `PoolArenasCache`) method of the netty Inner class used in the 'arrow memory netty' version '12.0.1' has changed and belongs to break change, let's wait for the upgrade of the 'arrow memory netty' before upgrading to the '4-1-94-Final' version.

The reference is as follows:
https://github.com/apache/arrow/blob/6af660f48472b8b45a5e01b7136b9b040b185eb1/java/memory/memory-netty/src/main/java/io/netty/buffer/PooledByteBufAllocatorL.java#L164
https://github.com/netty/netty/blob/da1a448d5bc4f36cc1744db93fcaf64e198db2bd/buffer/src/main/java/io/netty/buffer/PooledByteBufAllocator.java#L732-L736

### Does this PR introduce _any_ user-facing change?
No

### How was this patch tested?
Pass GA.

Closes #41681 from panbingkun/upgrade_netty.

Authored-by: panbingkun <pbk1982@gmail.com>
Signed-off-by: Dongjoon Hyun <dongjoon@apache.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants