Date: Mon, 20 Feb 2023 14:34:05 GMT From: "Alexander V. Chernikov" <melifaro@FreeBSD.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-branches@FreeBSD.org Subject: git: 72467b39f1db - stable/13 - netlink: return optional metadata with the operation result. Message-ID: <202302201434.31KEY5JJ005728@gitrepo.freebsd.org>
next in thread | raw e-mail | index | archive | help
The branch stable/13 has been updated by melifaro: URL: https://cgit.FreeBSD.org/src/commit/?id=72467b39f1db794f2ac54ac9baa1de3166507c6b commit 72467b39f1db794f2ac54ac9baa1de3166507c6b Author: Alexander V. Chernikov <melifaro@FreeBSD.org> AuthorDate: 2023-02-09 14:53:44 +0000 Commit: Alexander V. Chernikov <melifaro@FreeBSD.org> CommitDate: 2023-02-20 14:33:28 +0000 netlink: return optional metadata with the operation result. Some operations like interface creation may need to return metadata - in this case, interface name - back to the caller if the operation is successful. This change implements attaching an `NLMSGERR_ATTR_COOKIE` nla to the operation reply message via `nlmsg_report_cookie()`. Additionally, on successful interface creation, interface index and interface name are returned in the `IFLA_NEW_IFINDEX` and `IFLA_IFNAME TLVs, encapsulated in the `NLMSGERR_ATTR_COOKIE`. Reviewed By: pauamma Differential Revision: https://reviews.freebsd.org/D38283 MFC after: 1 week (cherry picked from commit 25c2dd2f2c6c6144d59d463c95f0471301d6efaa) --- share/man/man4/netlink.4 | 4 ++ share/man/man4/rtnetlink.4 | 7 ++ sys/netlink/netlink_message_parser.c | 19 +++++ sys/netlink/netlink_message_parser.h | 4 ++ sys/netlink/netlink_message_writer.c | 6 +- sys/netlink/netlink_message_writer.h | 31 ++++++++ sys/netlink/route/iface_drivers.c | 37 ++++++++++ tests/atf_python/sys/net/netlink.py | 134 +++++++++++++++++++---------------- tests/sys/netlink/test_rtnl_iface.py | 35 +++++++++ 9 files changed, 213 insertions(+), 64 deletions(-) diff --git a/share/man/man4/netlink.4 b/share/man/man4/netlink.4 index 1894de5841fe..fdcc823b01a6 100644 --- a/share/man/man4/netlink.4 +++ b/share/man/man4/netlink.4 @@ -217,6 +217,10 @@ socket option is set, the kernel may add a string TLV with the textual error description, optionally followed by the .Dv NLMSGERR_ATTR_OFFS TLV, indicating the offset from the message start that triggered an error. +Some operations may return additional metadata encapsulated in the +.Dv NLMSGERR_ATTR_COOKIE +TLV. +The metadata format is specific to the operation. If the operation reply is a multipart message, then no .Dv NLMSG_ERROR reply is generated, only a diff --git a/share/man/man4/rtnetlink.4 b/share/man/man4/rtnetlink.4 index 5849508b74c2..2c601251f7b0 100644 --- a/share/man/man4/rtnetlink.4 +++ b/share/man/man4/rtnetlink.4 @@ -284,6 +284,13 @@ struct ifinfomsg { Creates a new interface. The only mandatory TLV is .Dv IFLA_IFNAME . +The following attributes are returned inside the nested +.Dv NLMSGERR_ATTR_COOKIE : +.Pp +.Bd -literal -offset indent -compact +IFLA_NEW_IFINDEX (uint32) created interface index +IFLA_IFNAME (string) created interface name +.Ed .Ss RTM_DELLINK Deletes the interface specified by .Dv IFLA_IFNAME . diff --git a/sys/netlink/netlink_message_parser.c b/sys/netlink/netlink_message_parser.c index dc0c38712613..e30138266d9d 100644 --- a/sys/netlink/netlink_message_parser.c +++ b/sys/netlink/netlink_message_parser.c @@ -78,6 +78,25 @@ nlmsg_report_err_offset(struct nl_pstate *npt, uint32_t off) return (true); } +void +nlmsg_report_cookie(struct nl_pstate *npt, struct nlattr *nla) +{ + MPASS(nla->nla_type == NLMSGERR_ATTR_COOKIE); + MPASS(nla->nla_len >= sizeof(struct nlattr)); + npt->cookie = nla; +} + +void +nlmsg_report_cookie_u32(struct nl_pstate *npt, uint32_t val) +{ + struct nlattr *nla = npt_alloc(npt, sizeof(*nla) + sizeof(uint32_t)); + + nla->nla_type = NLMSGERR_ATTR_COOKIE; + nla->nla_len = sizeof(*nla) + sizeof(uint32_t); + memcpy(nla + 1, &val, sizeof(uint32_t)); + nlmsg_report_cookie(npt, nla); +} + static const struct nlattr_parser * search_states(const struct nlattr_parser *ps, int pslen, int key) { diff --git a/sys/netlink/netlink_message_parser.h b/sys/netlink/netlink_message_parser.h index 94f0ca5260d7..083c93dcbf8d 100644 --- a/sys/netlink/netlink_message_parser.h +++ b/sys/netlink/netlink_message_parser.h @@ -71,6 +71,7 @@ struct nl_pstate { uint32_t err_off; /* error offset from hdr start */ int error; /* last operation error */ char *err_msg; /* Description of last error */ + struct nlattr *cookie; /* NLA to return to the userspace */ bool strict; /* Strict parsing required */ }; @@ -198,6 +199,9 @@ bool nlmsg_report_err_msg(struct nl_pstate *npt, const char *fmt, ...); bool nlmsg_report_err_offset(struct nl_pstate *npt, uint32_t off); +void nlmsg_report_cookie(struct nl_pstate *npt, struct nlattr *nla); +void nlmsg_report_cookie_u32(struct nl_pstate *npt, uint32_t val); + /* * Have it inline so compiler can optimize field accesses into * the list of direct function calls without iteration. diff --git a/sys/netlink/netlink_message_writer.c b/sys/netlink/netlink_message_writer.c index 2187c40f2786..8a9315eedd1b 100644 --- a/sys/netlink/netlink_message_writer.c +++ b/sys/netlink/netlink_message_writer.c @@ -643,10 +643,6 @@ nlmsg_ack(struct nlpcb *nlp, int error, struct nlmsghdr *hdr, if ((npt->err_msg || npt->err_off) && nlp->nl_flags & NLF_EXT_ACK) nl_flags |= NLM_F_ACK_TLVS; - /* - * TODO: handle cookies - */ - NL_LOG(LOG_DEBUG3, "acknowledging message type %d seq %d", hdr->nlmsg_type, hdr->nlmsg_seq); @@ -662,6 +658,8 @@ nlmsg_ack(struct nlpcb *nlp, int error, struct nlmsghdr *hdr, nlattr_add_string(nw, NLMSGERR_ATTR_MSG, npt->err_msg); if (npt->err_off != 0 && nlp->nl_flags & NLF_EXT_ACK) nlattr_add_u32(nw, NLMSGERR_ATTR_OFFS, npt->err_off); + if (npt->cookie != NULL) + nlattr_add_raw(nw, npt->cookie); if (nlmsg_end(nw)) return; diff --git a/sys/netlink/netlink_message_writer.h b/sys/netlink/netlink_message_writer.h index 99f50fb94213..bb4bb759251a 100644 --- a/sys/netlink/netlink_message_writer.h +++ b/sys/netlink/netlink_message_writer.h @@ -163,6 +163,27 @@ _nlmsg_reserve_attr(struct nl_writer *nw, uint16_t nla_type, uint16_t sz) } #define nlmsg_reserve_attr(_ns, _at, _t) ((_t *)_nlmsg_reserve_attr(_ns, _at, NLA_ALIGN(sizeof(_t)))) +static inline bool +nlattr_add_nla(struct nl_writer *nw, const struct nlattr *nla_src) +{ + MPASS(nla_src->nla_len >= sizeof(struct nlattr)); + + int required_len = NLA_ALIGN(nla_src->nla_len); + if (__predict_false(nw->offset + required_len > nw->alloc_len)) { + if (!nlmsg_refill_buffer(nw, required_len)) + return (false); + } + + struct nlattr *nla = (struct nlattr *)(&nw->data[nw->offset]); + if ((nla_src->nla_len % 4) != 0) { + /* clear padding bytes */ + bzero((char *)nla + nla_src->nla_len - 4, 4); + } + memcpy(nla, nla_src, nla_src->nla_len); + nw->offset += required_len; + return (true); +} + static inline bool nlattr_add(struct nl_writer *nw, int attr_type, int attr_len, const void *data) { @@ -188,6 +209,16 @@ nlattr_add(struct nl_writer *nw, int attr_type, int attr_len, const void *data) return (true); } +static inline bool +nlattr_add_raw(struct nl_writer *nw, const struct nlattr *nla_src) +{ + int attr_len = nla_src->nla_len - sizeof(struct nlattr); + + MPASS(attr_len >= 0); + + return (nlattr_add(nw, nla_src->nla_type, attr_len, (const void *)(nla_src + 1))); +} + static inline bool nlattr_add_u8(struct nl_writer *nw, int attrtype, uint8_t value) { diff --git a/sys/netlink/route/iface_drivers.c b/sys/netlink/route/iface_drivers.c index 1b8c27bfca68..7cfaa5a6614b 100644 --- a/sys/netlink/route/iface_drivers.c +++ b/sys/netlink/route/iface_drivers.c @@ -109,6 +109,41 @@ modify_generic(struct ifnet *ifp, struct nl_parsed_link *lattrs, return (0); } +/* + * Saves the resulting ifindex and ifname to report them + * to userland along with the operation result. + * NLA format: + * NLMSGERR_ATTR_COOKIE(nested) + * IFLA_NEW_IFINDEX(u32) + * IFLA_IFNAME(string) + */ +static void +store_cookie(struct nl_pstate *npt, struct ifnet *ifp) +{ + int ifname_len = strlen(if_name(ifp)); + uint32_t ifindex = (uint32_t)ifp->if_index; + + int nla_len = sizeof(struct nlattr) * 3 + + sizeof(ifindex) + NL_ITEM_ALIGN(ifname_len + 1); + struct nlattr *nla_cookie = npt_alloc(npt, nla_len); + + /* Nested TLV */ + nla_cookie->nla_len = nla_len; + nla_cookie->nla_type = NLMSGERR_ATTR_COOKIE; + + struct nlattr *nla = nla_cookie + 1; + nla->nla_len = sizeof(struct nlattr) + sizeof(ifindex); + nla->nla_type = IFLA_NEW_IFINDEX; + memcpy(NLA_DATA(nla), &ifindex, sizeof(ifindex)); + + nla = NLA_NEXT(nla); + nla->nla_len = sizeof(struct nlattr) + ifname_len + 1; + nla->nla_type = IFLA_IFNAME; + strlcpy(NLA_DATA(nla), if_name(ifp), ifname_len + 1); + + nlmsg_report_cookie(npt, nla_cookie); +} + /* * Generic creation interface handler. * Responsible for creating interfaces w/o parameters and setting @@ -135,6 +170,8 @@ create_generic(struct nl_parsed_link *lattrs, const struct nlattr_bmask *bm, if (!success) return (EINVAL); error = modify_generic(ifp, lattrs, bm, nlp, npt); + if (error == 0) + store_cookie(npt, ifp); if_rele(ifp); } diff --git a/tests/atf_python/sys/net/netlink.py b/tests/atf_python/sys/net/netlink.py index 57c8582627cf..ba7d41c7cbf8 100644 --- a/tests/atf_python/sys/net/netlink.py +++ b/tests/atf_python/sys/net/netlink.py @@ -950,58 +950,68 @@ def prepare_attrs_map(attrs: List[AttrDescr]) -> Dict[str, Dict]: return ret -rtnl_route_attrs = [ - AttrDescr(RtattrType.RTA_DST, NlAttrIp), - AttrDescr(RtattrType.RTA_SRC, NlAttrIp), - AttrDescr(RtattrType.RTA_IIF, NlAttrIfindex), - AttrDescr(RtattrType.RTA_OIF, NlAttrIfindex), - AttrDescr(RtattrType.RTA_GATEWAY, NlAttrTable), - AttrDescr(RtattrType.RTA_VIA, NlAttrVia), - AttrDescr(RtattrType.RTA_NH_ID, NlAttrNhId), - AttrDescr( - RtattrType.RTA_METRICS, - NlAttrNested, - [ - AttrDescr(NlRtaxType.RTAX_MTU, NlAttrU32), - ], - ), -] - -nldone_attrs = [] - -nlerr_attrs = [ - AttrDescr(NlErrattrType.NLMSGERR_ATTR_MSG, NlAttrStr), - AttrDescr(NlErrattrType.NLMSGERR_ATTR_OFFS, NlAttrU32), -] - -rtnl_ifla_attrs = [ - AttrDescr(IflattrType.IFLA_ADDRESS, NlAttrMac), - AttrDescr(IflattrType.IFLA_BROADCAST, NlAttrMac), - AttrDescr(IflattrType.IFLA_IFNAME, NlAttrStr), - AttrDescr(IflattrType.IFLA_MTU, NlAttrU32), - AttrDescr(IflattrType.IFLA_PROMISCUITY, NlAttrU32), - AttrDescr(IflattrType.IFLA_OPERSTATE, NlAttrU8), - AttrDescr(IflattrType.IFLA_CARRIER, NlAttrU8), - AttrDescr(IflattrType.IFLA_IFALIAS, NlAttrStr), - AttrDescr(IflattrType.IFLA_STATS64, NlAttrIfStats), - AttrDescr( - IflattrType.IFLA_LINKINFO, - NlAttrNested, - [ - AttrDescr(IflinkInfo.IFLA_INFO_KIND, NlAttrStr), - AttrDescr(IflinkInfo.IFLA_INFO_DATA, NlAttr), - ], - ), -] - -rtnl_ifa_attrs = [ - AttrDescr(IfattrType.IFA_ADDRESS, NlAttrIp), - AttrDescr(IfattrType.IFA_LOCAL, NlAttrIp), - AttrDescr(IfattrType.IFA_LABEL, NlAttrStr), - AttrDescr(IfattrType.IFA_BROADCAST, NlAttrIp), - AttrDescr(IfattrType.IFA_ANYCAST, NlAttrIp), - AttrDescr(IfattrType.IFA_FLAGS, NlAttrU32), -] +rtnl_route_attrs = prepare_attrs_map( + [ + AttrDescr(RtattrType.RTA_DST, NlAttrIp), + AttrDescr(RtattrType.RTA_SRC, NlAttrIp), + AttrDescr(RtattrType.RTA_IIF, NlAttrIfindex), + AttrDescr(RtattrType.RTA_OIF, NlAttrIfindex), + AttrDescr(RtattrType.RTA_GATEWAY, NlAttrTable), + AttrDescr(RtattrType.RTA_VIA, NlAttrVia), + AttrDescr(RtattrType.RTA_NH_ID, NlAttrNhId), + AttrDescr( + RtattrType.RTA_METRICS, + NlAttrNested, + [ + AttrDescr(NlRtaxType.RTAX_MTU, NlAttrU32), + ], + ), + ] +) + +nldone_attrs = prepare_attrs_map([]) + +nlerr_attrs = prepare_attrs_map( + [ + AttrDescr(NlErrattrType.NLMSGERR_ATTR_MSG, NlAttrStr), + AttrDescr(NlErrattrType.NLMSGERR_ATTR_OFFS, NlAttrU32), + AttrDescr(NlErrattrType.NLMSGERR_ATTR_COOKIE, NlAttr), + ] +) + +rtnl_ifla_attrs = prepare_attrs_map( + [ + AttrDescr(IflattrType.IFLA_ADDRESS, NlAttrMac), + AttrDescr(IflattrType.IFLA_BROADCAST, NlAttrMac), + AttrDescr(IflattrType.IFLA_IFNAME, NlAttrStr), + AttrDescr(IflattrType.IFLA_MTU, NlAttrU32), + AttrDescr(IflattrType.IFLA_PROMISCUITY, NlAttrU32), + AttrDescr(IflattrType.IFLA_OPERSTATE, NlAttrU8), + AttrDescr(IflattrType.IFLA_CARRIER, NlAttrU8), + AttrDescr(IflattrType.IFLA_IFALIAS, NlAttrStr), + AttrDescr(IflattrType.IFLA_STATS64, NlAttrIfStats), + AttrDescr(IflattrType.IFLA_NEW_IFINDEX, NlAttrU32), + AttrDescr( + IflattrType.IFLA_LINKINFO, + NlAttrNested, + [ + AttrDescr(IflinkInfo.IFLA_INFO_KIND, NlAttrStr), + AttrDescr(IflinkInfo.IFLA_INFO_DATA, NlAttr), + ], + ), + ] +) + +rtnl_ifa_attrs = prepare_attrs_map( + [ + AttrDescr(IfattrType.IFA_ADDRESS, NlAttrIp), + AttrDescr(IfattrType.IFA_LOCAL, NlAttrIp), + AttrDescr(IfattrType.IFA_LABEL, NlAttrStr), + AttrDescr(IfattrType.IFA_BROADCAST, NlAttrIp), + AttrDescr(IfattrType.IFA_ANYCAST, NlAttrIp), + AttrDescr(IfattrType.IFA_FLAGS, NlAttrU32), + ] +) class BaseNetlinkMessage(object): @@ -1140,7 +1150,7 @@ class StdNetlinkMessage(BaseNetlinkMessage): raise return self - def _parse_attrs(self, data: bytes, attr_map): + def parse_attrs(self, data: bytes, attr_map): ret = [] off = 0 while len(data) - off >= 4: @@ -1157,7 +1167,7 @@ class StdNetlinkMessage(BaseNetlinkMessage): val = v["ad"].cls.from_bytes(data[off : off + nla_len], v["ad"].val) if "child" in v: # nested - attrs, _ = self._parse_attrs(data[off : off + nla_len], v["child"]) + attrs, _ = self.parse_attrs(data[off : off + nla_len], v["child"]) val = NlAttrNested(raw_nla_type, attrs) else: # unknown attribute @@ -1167,7 +1177,7 @@ class StdNetlinkMessage(BaseNetlinkMessage): return ret, off def parse_nla_list(self, data: bytes) -> List[NlAttr]: - return self._parse_attrs(data, self.nl_attrs_map) + return self.parse_attrs(data, self.nl_attrs_map) def print_message(self): self.print_nl_header(self.nl_hdr) @@ -1178,7 +1188,7 @@ class StdNetlinkMessage(BaseNetlinkMessage): class NetlinkDoneMessage(StdNetlinkMessage): messages = [NlMsgType.NLMSG_DONE.value] - nl_attrs_map = prepare_attrs_map(nldone_attrs) + nl_attrs_map = nldone_attrs @property def error_code(self): @@ -1197,7 +1207,7 @@ class NetlinkDoneMessage(StdNetlinkMessage): class NetlinkErrorMessage(StdNetlinkMessage): messages = [NlMsgType.NLMSG_ERROR.value] - nl_attrs_map = prepare_attrs_map(nlerr_attrs) + nl_attrs_map = nlerr_attrs @property def error_code(self): @@ -1217,6 +1227,10 @@ class NetlinkErrorMessage(StdNetlinkMessage): return nla.u32 return None + @property + def cookie(self): + return self.get_nla(NlErrattrType.NLMSGERR_ATTR_COOKIE) + def parse_base_header(self, data): if len(data) < sizeof(Nlmsgerr): raise ValueError("length less than nlmsgerr header") @@ -1257,7 +1271,7 @@ class NetlinkRtMessage(BaseNetlinkRtMessage): NlRtMsgType.RTM_DELROUTE.value, NlRtMsgType.RTM_GETROUTE.value, ] - nl_attrs_map = prepare_attrs_map(rtnl_route_attrs) + nl_attrs_map = rtnl_route_attrs def __init__(self, helper, nlm_type): super().__init__(helper, nlm_type) @@ -1297,7 +1311,7 @@ class NetlinkIflaMessage(BaseNetlinkRtMessage): NlRtMsgType.RTM_DELLINK.value, NlRtMsgType.RTM_GETLINK.value, ] - nl_attrs_map = prepare_attrs_map(rtnl_ifla_attrs) + nl_attrs_map = rtnl_ifla_attrs def __init__(self, helper, nlm_type): super().__init__(helper, nlm_type) @@ -1329,7 +1343,7 @@ class NetlinkIfaMessage(BaseNetlinkRtMessage): NlRtMsgType.RTM_DELADDR.value, NlRtMsgType.RTM_GETADDR.value, ] - nl_attrs_map = prepare_attrs_map(rtnl_ifa_attrs) + nl_attrs_map = rtnl_ifa_attrs def __init__(self, helper, nlm_type): super().__init__(helper, nlm_type) diff --git a/tests/sys/netlink/test_rtnl_iface.py b/tests/sys/netlink/test_rtnl_iface.py index 7cff592b1676..ecc932c6213e 100644 --- a/tests/sys/netlink/test_rtnl_iface.py +++ b/tests/sys/netlink/test_rtnl_iface.py @@ -17,6 +17,7 @@ from atf_python.sys.net.netlink import NlmBaseFlags from atf_python.sys.net.netlink import NlmNewFlags from atf_python.sys.net.netlink import NlMsgType from atf_python.sys.net.netlink import NlRtMsgType +from atf_python.sys.net.netlink import rtnl_ifla_attrs from atf_python.sys.net.vnet import SingleVnetTestTemplate @@ -91,6 +92,40 @@ class TestRtNlIface(NetlinkTestTemplate, SingleVnetTestTemplate): self.get_interface_byname("lo10") + @pytest.mark.require_user("root") + def test_create_iface_plain_retvals(self): + """Tests loopback creation w/o any parameters""" + flags = NlmNewFlags.NLM_F_EXCL.value | NlmNewFlags.NLM_F_CREATE.value + msg = NetlinkIflaMessage(self.helper, NlRtMsgType.RTM_NEWLINK.value) + msg.nl_hdr.nlmsg_flags = ( + flags | NlmBaseFlags.NLM_F_ACK.value | NlmBaseFlags.NLM_F_REQUEST.value + ) + msg.add_nla(NlAttrStr(IflattrType.IFLA_IFNAME, "lo10")) + msg.add_nla( + NlAttrNested( + IflattrType.IFLA_LINKINFO, + [ + NlAttrStrn(IflinkInfo.IFLA_INFO_KIND, "lo"), + ], + ) + ) + + rx_msg = self.get_reply(msg) + assert rx_msg.is_type(NlMsgType.NLMSG_ERROR) + assert rx_msg.error_code == 0 + assert rx_msg.cookie is not None + nla_list, _ = rx_msg.parse_attrs(bytes(rx_msg.cookie)[4:], rtnl_ifla_attrs) + nla_map = {n.nla_type: n for n in nla_list} + assert IflattrType.IFLA_IFNAME.value in nla_map + assert nla_map[IflattrType.IFLA_IFNAME.value].text == "lo10" + assert IflattrType.IFLA_NEW_IFINDEX.value in nla_map + assert nla_map[IflattrType.IFLA_NEW_IFINDEX.value].u32 > 0 + + lo_msg = self.get_interface_byname("lo10") + assert ( + lo_msg.base_hdr.ifi_index == nla_map[IflattrType.IFLA_NEW_IFINDEX.value].u32 + ) + @pytest.mark.require_user("root") def test_create_iface_attrs(self): """Tests interface creation with additional properties"""
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202302201434.31KEY5JJ005728>