Skip to content

Commit

Permalink
correct netlink socket recv read buffer size
Browse files Browse the repository at this point in the history
Summary:
netlink socket is set to send/rcv buffer size of 1MB. However, when socket is read with recv(), the buffer size it is reading is only 4KB which is much smaller than 1MB.

This causes reading of truncated data received via recv() call. Luckily, for EOS it works out because only interfaces (port-channel) we are interested in are sent by kernel in first 4KB size of the buffer.

On JunOS, it fails because port-channel interfaces are not sent out in first 4KB buffer size. When this recv() buffer size is increased to 64KB or 32KB, port-channel interfaces are read correctly in netlink notifications.

Reviewed By: xiangxu1121

Differential Revision: D66438614

fbshipit-source-id: 57c138df557db925c270dd17caf3356d09292d23
  • Loading branch information
Shitanshu Shah authored and facebook-github-bot committed Nov 26, 2024
1 parent 27fed29 commit 4dbedf5
Show file tree
Hide file tree
Showing 9 changed files with 21 additions and 19 deletions.
2 changes: 1 addition & 1 deletion openr/nl/NetlinkAddrMessage.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ NetlinkAddrMessage::addCacheInfo(const IfAddress& ifAddr) {
if (!ifAddr.getPreferredLft().has_value()) {
return 0;
}
std::array<char, kMaxNlPayloadSize> cacheInfo = {};
std::array<char, kMaxNlSendPayloadSize> cacheInfo = {};
struct ifa_cacheinfo* rta =
reinterpret_cast<struct ifa_cacheinfo*>(cacheInfo.data());
rta->ifa_prefered = ifAddr.getPreferredLft().value();
Expand Down
6 changes: 3 additions & 3 deletions openr/nl/NetlinkLinkMessage.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,7 @@ NetlinkLinkMessage::addLink(const Link& link) {

int
NetlinkLinkMessage::addLinkInfo(const Link& link) {
std::array<char, kMaxNlPayloadSize> linkInfo = {};
std::array<char, kMaxNlSendPayloadSize> linkInfo = {};
int status{0};
if (!link.getLinkKind().has_value()) {
return 0;
Expand All @@ -225,7 +225,7 @@ NetlinkLinkMessage::addLinkInfo(const Link& link) {

int
NetlinkLinkMessage::addLinkInfoSubAttrs(
std::array<char, kMaxNlPayloadSize>& linkInfo, const Link& link) const {
std::array<char, kMaxNlSendPayloadSize>& linkInfo, const Link& link) const {
struct rtattr* rta = reinterpret_cast<struct rtattr*>(linkInfo.data());

if (addSubAttributes(
Expand All @@ -239,7 +239,7 @@ NetlinkLinkMessage::addLinkInfoSubAttrs(
const auto greInfo = link.getGreInfo();
if (greInfo.has_value()) {
// init sub attribute
std::array<char, kMaxNlPayloadSize> linkInfoData = {};
std::array<char, kMaxNlSendPayloadSize> linkInfoData = {};
struct rtattr* subRta =
reinterpret_cast<struct rtattr*>(linkInfoData.data());
subRta->rta_type = IFLA_INFO_DATA;
Expand Down
3 changes: 2 additions & 1 deletion openr/nl/NetlinkLinkMessage.h
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,8 @@ class NetlinkLinkMessage final : public NetlinkMessageBase {

// add sub-attributes to IFLA_LINKINFO
int addLinkInfoSubAttrs(
std::array<char, kMaxNlPayloadSize>& linkInfo, const Link& link) const;
std::array<char, kMaxNlSendPayloadSize>& linkInfo,
const Link& link) const;

//
// Private variables for rtnetlink msg exchange
Expand Down
4 changes: 2 additions & 2 deletions openr/nl/NetlinkMessageBase.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ NetlinkMessageBase::addSubAttributes(
struct rtattr* subrta{nullptr};
uint32_t subRtaLen = RTA_LENGTH(len);

if (RTA_ALIGN(rta->rta_len) + RTA_ALIGN(subRtaLen) > kMaxNlPayloadSize) {
if (RTA_ALIGN(rta->rta_len) + RTA_ALIGN(subRtaLen) > kMaxNlSendPayloadSize) {
XLOG(ERR) << "No buffer for adding attr: " << type << " length: " << len;
return subrta;
}
Expand All @@ -72,7 +72,7 @@ NetlinkMessageBase::addAttributes(
uint32_t rtaLen = (RTA_LENGTH(len));
uint32_t nlmsgAlen = NLMSG_ALIGN((msghdr_)->nlmsg_len);

if (nlmsgAlen + RTA_ALIGN(rtaLen) > kMaxNlPayloadSize) {
if (nlmsgAlen + RTA_ALIGN(rtaLen) > kMaxNlSendPayloadSize) {
XLOG(ERR) << "Space not available to add attribute type " << type;
return ENOBUFS;
}
Expand Down
6 changes: 3 additions & 3 deletions openr/nl/NetlinkMessageBase.h
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@

namespace openr::fbnl {

constexpr uint16_t kMaxNlPayloadSize{4096};
constexpr uint16_t kMaxNlSendPayloadSize{4096};

/*
* Data structure representing a netlink message, either to be sent or received.
Expand All @@ -34,7 +34,7 @@ constexpr uint16_t kMaxNlPayloadSize{4096};
* Aim of the message is to faciliate serialization and deserialization of
* C++ object (application) to/from bytes (kernel).
*
* Maximum size of message is limited by `kMaxNlPayloadSize` parameter.
* Maximum size of message is limited by `kMaxNlSendPayloadSize` parameter.
*/
/*
* For netlink reference:
Expand Down Expand Up @@ -101,7 +101,7 @@ class NetlinkMessageBase {
uint32_t getDataLength() const;

// Buffer to create message
std::array<char, kMaxNlPayloadSize> msg = {};
std::array<char, kMaxNlSendPayloadSize> msg = {};

/**
* APIs for accumulating objects of `GET_<>` request. These APIs are invoked
Expand Down
9 changes: 5 additions & 4 deletions openr/nl/NetlinkProtocolSocket.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -284,7 +284,7 @@ NetlinkProtocolSocket::sendNetlinkMessage() {

void
NetlinkProtocolSocket::processMessage(
const std::array<char, kMaxNlPayloadSize>& rxMsg, uint32_t bytesRead) {
const std::array<char, kNetlinkSockRecvBuf>& rxMsg, uint32_t bytesRead) {
// first netlink message header
struct nlmsghdr* nlh = (struct nlmsghdr*)rxMsg.data();
do {
Expand Down Expand Up @@ -433,10 +433,11 @@ NetlinkProtocolSocket::processMessage(

void
NetlinkProtocolSocket::recvNetlinkMessage() {
// messages buffer
std::array<char, kMaxNlPayloadSize> recvMsg = {};
// messages buffer, set the size same as what is set as receive
// buffer size for nlSock_
std::array<char, kNetlinkSockRecvBuf> recvMsg = {};

int32_t bytesRead = ::recv(nlSock_, recvMsg.data(), kMaxNlPayloadSize, 0);
int32_t bytesRead = ::recv(nlSock_, recvMsg.data(), kNetlinkSockRecvBuf, 0);
XLOG(DBG4) << "Message received with size: " << bytesRead;

if (bytesRead < 0) {
Expand Down
2 changes: 1 addition & 1 deletion openr/nl/NetlinkProtocolSocket.h
Original file line number Diff line number Diff line change
Expand Up @@ -261,7 +261,7 @@ class NetlinkProtocolSocket : public folly::EventHandler {
// Process received netlink message. Set return values for pending requests
// or send notifications.
void processMessage(
const std::array<char, kMaxNlPayloadSize>& rxMsg, uint32_t bytesRead);
const std::array<char, kNetlinkSockRecvBuf>& rxMsg, uint32_t bytesRead);

// Process ack message. Set return status on pending requests in nlSeqNumMap_
// Resume sending messages from queue_ if any pending
Expand Down
6 changes: 3 additions & 3 deletions openr/nl/NetlinkRouteMessage.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -388,7 +388,7 @@ NetlinkRouteMessage::addPushNexthop(

int
NetlinkRouteMessage::addNextHops(const Route& route) {
std::array<char, kMaxNlPayloadSize> nhop = {};
std::array<char, kMaxNlSendPayloadSize> nhop = {};
int status{0};
if (route.getNextHops().size() && route.isMultiPath()) {
if ((status = addMultiPathNexthop(nhop, route))) {
Expand Down Expand Up @@ -416,7 +416,7 @@ NetlinkRouteMessage::addNextHops(const Route& route) {

int
NetlinkRouteMessage::addMultiPathNexthop(
std::array<char, kMaxNlPayloadSize>& nhop, const Route& route) const {
std::array<char, kMaxNlSendPayloadSize>& nhop, const Route& route) const {
// Add [RTA_MULTIPATH - label, via, dev][RTA_ENCAP][RTA_ENCAP_TYPE]
struct rtattr* rta = reinterpret_cast<struct rtattr*>(nhop.data());

Expand Down Expand Up @@ -482,7 +482,7 @@ NetlinkRouteMessage::addSingleMplsNexthop(const Route& route) {
}
size_t totalSize = labels.value().size() * sizeof(struct mpls_label);
// nested attr RTA_ENCAP + MPLS_IPTUNNEL_DST
std::array<char, kMaxNlPayloadSize> encapInfo = {};
std::array<char, kMaxNlSendPayloadSize> encapInfo = {};
struct rtattr* rta = reinterpret_cast<struct rtattr*>(encapInfo.data());
rta->rta_type = NLA_F_NESTED | RTA_ENCAP;
rta->rta_len = RTA_LENGTH(0);
Expand Down
2 changes: 1 addition & 1 deletion openr/nl/NetlinkRouteMessage.h
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ class NetlinkRouteMessage final : public NetlinkMessageBase {

// Add ECMP paths
int addMultiPathNexthop(
std::array<char, kMaxNlPayloadSize>& nhop, const Route& route) const;
std::array<char, kMaxNlSendPayloadSize>& nhop, const Route& route) const;

// Add single mpls encap nexthop
int addSingleMplsNexthop(const Route& route);
Expand Down

0 comments on commit 4dbedf5

Please sign in to comment.