Skip to content

Conversation

@maxime-leroy
Copy link
Collaborator

Reverts #244

To support jumbo mbuf:

  • we can increase mbuf size, the side effect is increased memory consumption.
  • To support multi segs. In this case, doing the check about multi segments when writing a mbuf or reading this one must be done. Of course, the side effect is an extra cost for cpu.

Choose 2 have been done without fixing/checking that operation done on mbuf are safe or not with multiseg. It just the perfect way to get segfault.

Some work has done on my side in ip_input and ip6_input to check that data are on the first segment. I have added some unit tests. But it's not enough at all to support multi segments.

@maxime-leroy
Copy link
Collaborator Author

maxime-leroy commented Jun 26, 2025

See with Robin. For now, possible crash are very rarely. Should happens as soon we add segments like for fragmentation or using encapsulation with new segments.

Next step will be to have a function combined rte_pktmbuf_mtod et rte_pktmbuf_data_len together. packet with header on second or later segment will be dropped. It will prevent crash for weird case in production.

This next step can be done in a later PR. It's just to prevent crash, not to support header on second segment.

See ip6_input.c for reference:

  •          if (rte_pktmbuf_data_len(mbuf) < sizeof(struct rte_ipv6_hdr)) {
    
  •                  // XXX: call rte_pktmuf_data_len is used to ensure that the IPv6 header
    
  •                  // is located on the first segment. IPv6 headers located on the second,
    
  •                  // third or subsequent segments, as well spanning segment boundaries, are
    
  •                  // not currently handled.
    
  •                  edge = BAD_LENGTH;
    
  •                  goto next;
    
  •          }
    
  • -> example en ipv6

@maxime-leroy
Copy link
Collaborator Author

Reopen because enable rx scatter and tx multi segments support will drop performance (at least > 10% au doigt mouillé). Don't have time to do the test. But by enabling scatter in rx and multi segment in tx, you don't use the same function of driver for RX/TX . The RX/TX functions support mutli segments are less performant that the one supporting single segments because of vector instructions.

At least, if we need multi segments, we should enable only when mtu is > mbuf headroom. To keep the best performance for standard mtu.

Else it's to allocate huge mbuf to support jumbo frame. In this case, we should have an option to enable/disable this features at init.

@maxime-leroy maxime-leroy reopened this Jun 26, 2025
@rjarry
Copy link
Collaborator

rjarry commented Jun 30, 2025

Closing this. The revert was done in #246

@rjarry rjarry closed this Jun 30, 2025
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.

2 participants