Skip to content

lib/ip: fix build for debug kernel #254

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.

Sign up for GitHub

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

lib/ip: fix build for debug kernel #254

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions src/lib/transport/ip/udp_send.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -126,8 +126,9 @@ static bool ci_ipx_is_mf_set(int af, ci_ipx_hdr_t* ipx)
ci_noinline void ci_udp_sendmsg_chksum(ci_netif* ni, ci_ip_pkt_fmt* pkt,
int af, ci_ipx_hdr_t* first_hdr)
{
/* 1400*50 = 70000, i.e. in normal situation there are <50 fragments */
#define MAX_IP_FRAGMENTS 50
/* 1428*46 = 65688 > 65536, i.e. in normal situation there are <=46
* fragments */
#define MAX_IP_FRAGMENTS 46
Copy link
Contributor

@ivatet-amd ivatet-amd Dec 2, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Pragmatically, it seems highly unlikely to come across that many fragments, and so I agree.

At the same time, can we replace this array with a sliding window to eliminate the problem of the large stack footprint so we never come back to it?

ci_udp_sendmsg_chksum(...)
{
  ...
  uint32_t csum32;
  uint64_t csum64 = ef_ipx_pseudo_hdr_checksum(af, ip, udp->len, IPPROTO_UDP);
  csum64 = ip_csum64_partial(csum64, udp, 6);   /* omit udp_check_be16 */
  ...
  while( OO_PP_NOT_NULL(p->next) ) {
    struct iovec iov;
    ...
    iov.iov_base = frag_start;
    iov.iov_len = frag_len;
    ...
    csum64 = ip_csum64_partialv_sliding(csum64, &iov, csum64);
  }
  csum32 = ip_proto_csum64_finish(csum64);
  udp->udp_check_be16 = csum32 ? csum32 : 0xffff;
}

I appreciate that functions prefixed with ip_csum64 are "private" to the checksum unit, and we might not want to expose them as-is. Instead, shall we consider an "iterator" approach similar to what the hashing libraries do, e.g. create a hash-/checksum- context, call Init(), then Update() multiple times, then Final()?

Do you find it possible and appealing?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Frankly, I'm not interested in IP fragmenting at all, so I am not going to implement this approach. I agree that your approach makes sense, it would be nice if you implement it.

struct iovec iov[MAX_IP_FRAGMENTS];
int n = -1;
ci_udp_hdr* udp = TX_PKT_IPX_UDP(af, pkt, true);
Expand Down