Skip to content

Set of minor fixes across the board #442

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

Merged
thom311 merged 8 commits into from
Aug 7, 2025
+89 −45
Merged

Set of minor fixes across the board #442

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
f3a2a47
[ipvti] Fix a bug in ipvti_put_attrs
cpaasch-oai Aug 5, 2025
5463545
[nexthop] Fix clone'ing of route nexthop
cpaasch-oai Aug 5, 2025
873d543
[geneve] Use NLA_PUT_U16 for the port
cpaasch-oai Aug 6, 2025
c05a7ff
[ipgre] Fix ipgre_put_attrs and dump_details
cpaasch-oai Aug 6, 2025
c20968a
[vxlan] Use proper u16 for vxi_port and fixes to vxlan_compare
cpaasch-oai Aug 6, 2025
6d7b10d
[sit/ipip/ipvti] Use correct byte-order conversion routing
cpaasch-oai Aug 6, 2025
e71bdfb
[ip6gre/ip6vti]: Fix printing of the link-name
cpaasch-oai Aug 6, 2025
75352b7
[link] Avoid leaking rtnl_link object
cpaasch-oai Aug 6, 2025
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
2 changes: 1 addition & 1 deletion lib/route/link/geneve.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -294,7 +294,7 @@ static int geneve_put_attrs(struct nl_msg *msg, struct rtnl_link *link)
NLA_PUT_U32(msg, IFLA_GENEVE_LABEL, geneve->label);

if (geneve->mask & GENEVE_ATTR_PORT)
NLA_PUT_U32(msg, IFLA_GENEVE_PORT, geneve->port);
NLA_PUT_U16(msg, IFLA_GENEVE_PORT, geneve->port);

if (geneve->mask & GENEVE_ATTR_UDP_CSUM)
NLA_PUT_U8(msg, IFLA_GENEVE_UDP_CSUM, geneve->udp_csum);
Expand Down
14 changes: 7 additions & 7 deletions lib/route/link/ipgre.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -181,10 +181,10 @@ static int ipgre_put_attrs(struct nl_msg *msg, struct rtnl_link *link)
if (ipgre->ipgre_mask & IPGRE_ATTR_LINK)
NLA_PUT_U32(msg, IFLA_GRE_LINK, ipgre->link);

if (ipgre->ipgre_mask & IFLA_GRE_IFLAGS)
if (ipgre->ipgre_mask & IPGRE_ATTR_IFLAGS)
NLA_PUT_U16(msg, IFLA_GRE_IFLAGS, ipgre->iflags);

if (ipgre->ipgre_mask & IFLA_GRE_OFLAGS)
if (ipgre->ipgre_mask & IPGRE_ATTR_OFLAGS)
NLA_PUT_U16(msg, IFLA_GRE_OFLAGS, ipgre->oflags);

if (ipgre->ipgre_mask & IPGRE_ATTR_IKEY)
Expand Down Expand Up @@ -263,7 +263,7 @@ static void ipgre_dump_details(struct rtnl_link *link, struct nl_dump_params *p)

if (ipgre->ipgre_mask & IPGRE_ATTR_IKEY) {
nl_dump(p, " ikey ");
nl_dump_line(p, "%x\n",ipgre->ikey);
nl_dump_line(p, "%x\n", ipgre->ikey);
}

if (ipgre->ipgre_mask & IPGRE_ATTR_OKEY) {
Expand All @@ -273,18 +273,18 @@ static void ipgre_dump_details(struct rtnl_link *link, struct nl_dump_params *p)

if (ipgre->ipgre_mask & IPGRE_ATTR_LOCAL) {
nl_dump(p, " local ");
if(inet_ntop(AF_INET, &ipgre->local, addr, sizeof(addr)))
if (inet_ntop(AF_INET, &ipgre->local, addr, sizeof(addr)))
nl_dump_line(p, "%s\n", addr);
else
nl_dump_line(p, "%#x\n", ntohs(ipgre->local));
nl_dump_line(p, "%#x\n", ntohl(ipgre->local));
}

if (ipgre->ipgre_mask & IPGRE_ATTR_REMOTE) {
nl_dump(p, " remote ");
if(inet_ntop(AF_INET, &ipgre->remote, addr, sizeof(addr)))
if (inet_ntop(AF_INET, &ipgre->remote, addr, sizeof(addr)))
nl_dump_line(p, "%s\n", addr);
else
nl_dump_line(p, "%#x\n", ntohs(ipgre->remote));
nl_dump_line(p, "%#x\n", ntohl(ipgre->remote));
}

if (ipgre->ipgre_mask & IPGRE_ATTR_TTL) {
Expand Down
10 changes: 6 additions & 4 deletions lib/route/link/ipip.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -208,18 +208,20 @@ static void ipip_dump_details(struct rtnl_link *link, struct nl_dump_params *p)

if (ipip->ipip_mask & IPIP_ATTR_LOCAL) {
nl_dump(p, " local ");
if(inet_ntop(AF_INET, &ipip->local, addr, sizeof(addr)))

if (inet_ntop(AF_INET, &ipip->local, addr, sizeof(addr)))
Copy link
Owner

@thom311 thom311 Aug 6, 2025

Choose a reason for hiding this comment

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

I think checking the return value for inet_ntop() is wrong. This function can only fail, if you pass invalid address/family or buffer size, which the caller should just not do. There is _nl_inet_ntop() for that reason.

I mean, if libc is not able to convert an u32 / struct in6_addr to a string buffer (of sufficient size), then we should reimplement it. But of course, libc is not going to fail here. Ever.

And the else path is just unreached code and noise and place for bugs.

But the patch is good. Maybe I will drop some inet_ntop() later -- unless you want to do it first :) Thank you.

Copy link
Contributor Author

@cpaasch-oai cpaasch-oai Aug 6, 2025

Choose a reason for hiding this comment

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

I see! I just blindly fixed whitespace here without actually reading what the purpose of inet_ntop here is 😅

nl_dump_line(p, "%s\n", addr);
else
nl_dump_line(p, "%#x\n", ntohs(ipip->local));
nl_dump_line(p, "%#x\n", ntohl(ipip->local));
}

if (ipip->ipip_mask & IPIP_ATTR_REMOTE) {
nl_dump(p, " remote ");
if(inet_ntop(AF_INET, &ipip->remote, addr, sizeof(addr)))

if (inet_ntop(AF_INET, &ipip->remote, addr, sizeof(addr)))
nl_dump_line(p, "%s\n", addr);
else
nl_dump_line(p, "%#x\n", ntohs(ipip->remote));
nl_dump_line(p, "%#x\n", ntohl(ipip->remote));
}

if (ipip->ipip_mask & IPIP_ATTR_TTL) {
Expand Down
14 changes: 8 additions & 6 deletions lib/route/link/ipvti.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ static int ipvti_put_attrs(struct nl_msg *msg, struct rtnl_link *link)
if (ipvti->ipvti_mask & IPVTI_ATTR_IKEY)
NLA_PUT_U32(msg, IFLA_VTI_IKEY, ipvti->ikey);

if (ipvti->ipvti_mask & IFLA_VTI_IKEY)
if (ipvti->ipvti_mask & IPVTI_ATTR_OKEY)
NLA_PUT_U32(msg, IFLA_VTI_OKEY, ipvti->okey);

if (ipvti->ipvti_mask & IPVTI_ATTR_LOCAL)
Expand Down Expand Up @@ -208,18 +208,20 @@ static void ipvti_dump_details(struct rtnl_link *link, struct nl_dump_params *p)

if (ipvti->ipvti_mask & IPVTI_ATTR_LOCAL) {
nl_dump(p, " local ");
if(inet_ntop(AF_INET, &ipvti->local, addr, sizeof(addr)))

if (inet_ntop(AF_INET, &ipvti->local, addr, sizeof(addr)))
nl_dump_line(p, "%s\n", addr);
else
nl_dump_line(p, "%#x\n", ntohs(ipvti->local));
nl_dump_line(p, "%#x\n", ntohl(ipvti->local));
}

if (ipvti->ipvti_mask & IPVTI_ATTR_REMOTE) {
nl_dump(p, " remote ");
if(inet_ntop(AF_INET, &ipvti->remote, addr, sizeof(addr)))

if (inet_ntop(AF_INET, &ipvti->remote, addr, sizeof(addr)))
nl_dump_line(p, "%s\n", addr);
else
nl_dump_line(p, "%#x\n", ntohs(ipvti->remote));
nl_dump_line(p, "%#x\n", ntohl(ipvti->remote));
}

if (ipvti->ipvti_mask & IPVTI_ATTR_FWMARK) {
Expand Down Expand Up @@ -264,7 +266,7 @@ static struct rtnl_link_info_ops ipvti_info_ops = {

#define IS_IPVTI_LINK_ASSERT(link) \
if ((link)->l_info_ops != &ipvti_info_ops) { \
APPBUG("Link is not a ipvti link. set type \vti\" first."); \
APPBUG("Link is not a ipvti link. set type \"vti\" first.");\
return -NLE_OPNOTSUPP; \
}

Expand Down
10 changes: 6 additions & 4 deletions lib/route/link/sit.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -276,18 +276,20 @@ static void sit_dump_details(struct rtnl_link *link, struct nl_dump_params *p)

if (sit->sit_mask & SIT_ATTR_LOCAL) {
nl_dump(p, " local ");
if(inet_ntop(AF_INET, &sit->local, addr, sizeof(addr)))

if (inet_ntop(AF_INET, &sit->local, addr, sizeof(addr)))
nl_dump_line(p, "%s\n", addr);
else
nl_dump_line(p, "%#x\n", ntohs(sit->local));
nl_dump_line(p, "%#x\n", ntohl(sit->local));
}

if (sit->sit_mask & SIT_ATTR_REMOTE) {
nl_dump(p, " remote ");
if(inet_ntop(AF_INET, &sit->remote, addr, sizeof(addr)))

if (inet_ntop(AF_INET, &sit->remote, addr, sizeof(addr)))
nl_dump_line(p, "%s\n", addr);
else
nl_dump_line(p, "%#x\n", ntohs(sit->remote));
nl_dump_line(p, "%#x\n", ntohl(sit->remote));
}

if (sit->sit_mask & SIT_ATTR_TTL) {
Expand Down
14 changes: 8 additions & 6 deletions lib/route/link/vxlan.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -572,7 +572,7 @@ static int vxlan_put_attrs(struct nl_msg *msg, struct rtnl_link *link)
NLA_PUT_U8(msg, IFLA_VXLAN_L3MISS, vxi->vxi_l3miss);

if (vxi->ce_mask & VXLAN_ATTR_PORT)
NLA_PUT_U32(msg, IFLA_VXLAN_PORT, vxi->vxi_port);
NLA_PUT_U16(msg, IFLA_VXLAN_PORT, vxi->vxi_port);

if (vxi->ce_mask & VXLAN_ATTR_UDP_CSUM)
NLA_PUT_U8(msg, IFLA_VXLAN_UDP_CSUM, vxi->vxi_udp_csum);
Expand Down Expand Up @@ -643,13 +643,15 @@ static int vxlan_compare(struct rtnl_link *link_a, struct rtnl_link *link_b,
sizeof(a->vxi_group6)) != 0);
diff |= _DIFF(VXLAN_ATTR_LOCAL6, memcmp(&a->vxi_local6, &b->vxi_local6,
sizeof(a->vxi_local6)) != 0);
diff |= _DIFF(VXLAN_ATTR_UDP_CSUM, a->vxi_proxy != b->vxi_proxy);
diff |= _DIFF(VXLAN_ATTR_UDP_CSUM, a->vxi_udp_csum != b->vxi_udp_csum);
diff |= _DIFF(VXLAN_ATTR_UDP_ZERO_CSUM6_TX,
a->vxi_proxy != b->vxi_proxy);
a->vxi_udp_zero_csum6_tx != b->vxi_udp_zero_csum6_tx);
diff |= _DIFF(VXLAN_ATTR_UDP_ZERO_CSUM6_RX,
a->vxi_proxy != b->vxi_proxy);
diff |= _DIFF(VXLAN_ATTR_REMCSUM_TX, a->vxi_proxy != b->vxi_proxy);
diff |= _DIFF(VXLAN_ATTR_REMCSUM_RX, a->vxi_proxy != b->vxi_proxy);
a->vxi_udp_zero_csum6_rx != b->vxi_udp_zero_csum6_rx);
diff |= _DIFF(VXLAN_ATTR_REMCSUM_TX,
a->vxi_remcsum_tx != b->vxi_remcsum_tx);
diff |= _DIFF(VXLAN_ATTR_REMCSUM_RX,
a->vxi_remcsum_rx != b->vxi_remcsum_rx);
diff |= _DIFF(VXLAN_ATTR_COLLECT_METADATA,
a->vxi_collect_metadata != b->vxi_collect_metadata);
diff |= _DIFF(VXLAN_ATTR_LABEL, a->vxi_label != b->vxi_label);
Expand Down
1 change: 1 addition & 0 deletions lib/route/nexthop.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ struct rtnl_nexthop *rtnl_route_nh_clone(struct rtnl_nexthop *src)
nh->rtnh_flag_mask = src->rtnh_flag_mask;
nh->rtnh_weight = src->rtnh_weight;
nh->rtnh_ifindex = src->rtnh_ifindex;
nh->rtnh_realms = src->rtnh_realms;
nh->ce_mask = src->ce_mask;

if (src->rtnh_gateway) {
Expand Down