Page MenuHomeFreeBSD

inpcb: return ENOMEM if bind(2) fails to allocate lbgroup
ClosedPublic

Authored by glebius on Thu, Feb 27, 6:24 AM.
Tags
None
Referenced Files
F112566850: D49153.diff
Wed, Mar 19, 10:40 PM
Unknown Object (File)
Fri, Mar 7, 8:18 PM
Unknown Object (File)
Fri, Mar 7, 8:44 AM
Unknown Object (File)
Mon, Mar 3, 8:10 PM
Unknown Object (File)
Mon, Mar 3, 6:58 AM
Unknown Object (File)
Sat, Mar 1, 12:52 AM
Unknown Object (File)
Fri, Feb 28, 3:28 PM
Unknown Object (File)
Fri, Feb 28, 10:09 AM
Subscribers

Details

Summary

The SO_REUSEPORT_LB isn't a standard option, neither ENOMEM is a specified
return code from bind(2), but it definitely is more appropriate than
EAGAIN or the masked ENOBUFS.

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

markj added inline comments.
sys/netinet/in_pcb.c
736

What's the purpose of this assertion? If you want to document that in_pcbinshash() can only fail for lbgroup sockets, IMO it would be better to somehow document that fact in in_pcbinshash().

This revision is now accepted and ready to land.Thu, Feb 27, 2:26 PM
sys/netinet/in_pcb.c
736

Yes, that's the purpose of course. How do want this be documented in in_pcbinshash()? Like in the comment above the function?

P.S. We potentially can pre-allocate memory at the beginning of the bind(2) syscall and make it never fail even with SO_REUSEPORT_LB. Or pre-allocate it at setsockopt(2) stage. But I don't think the added complexity is worthy. If you are running a load balanced web server and you end up with kernel memory so low that such a tiny malloc(9) fails, you already are screwed up too bad. So bad, that non-standard ENOMEM from bind(2) no longer is an issue :)

sys/netinet/in_pcb.c
736

My point about the assertion is that it's a bit fragile. If someone changes in_pcbinshash() such that the assertion is wrong, they might not notice it. I'd just add it as a comment above in_pcbinshash().

IMO it's ugly for a system call to fail because of a transient memory shortage, but indeed, I'm not sure the complexity of fixing it is worth it. I do consider it a bug, though.

sys/netinet/in_pcb.c
736

Got it. I will add comment above in_pcbinshash().

Note that this revision depends on D49151 to proceed.