git: 25c2dd2f2c6c - main - netlink: return optional metadata with the operation result.
- Go to: [ bottom of page ] [ top of archives ] [ this month ]
Date: Thu, 09 Feb 2023 15:30:09 UTC
The branch main has been updated by melifaro:
URL: https://cgit.FreeBSD.org/src/commit/?id=25c2dd2f2c6c6144d59d463c95f0471301d6efaa
commit 25c2dd2f2c6c6144d59d463c95f0471301d6efaa
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-09 15:30:00 +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
---
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 7f098b808743..e944fe88994e 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"""