| Message ID | 20181230171423.26104-1-idosch@mellanox.com |
|---|---|
| State | Accepted, archived |
| Delegated to: | David Ahern |
| Headers | show |
| Series | [iproute2-next] bridge: fdb: Use 'struct ndmsg' for FDB dumping | expand |
On 12/30/18 10:14 AM, Ido Schimmel wrote: > Since commit aea41afcfd6d ("ip bridge: Set NETLINK_GET_STRICT_CHK on > socket") iproute2 uses strict checking on kernels that support it. This > causes FDB dumping to fail [1], as iproute2 uses 'struct ifinfomsg' > whereas the kernel expects 'struct ndmsg'. > > Note that with this change iproute2 continues to work on old kernels > that do not support strict checking, but contain the fix introduced in > kernel commit bd961c9bc664 ("rtnetlink: fix rtnl_fdb_dump() for ndmsg > header"). > > [1] > # bridge fdb show > [ 5365.137224] netlink: 4 bytes leftover after parsing attributes in process `bridge'. > Error: bytes leftover after parsing attributes. > Dump terminated > > Fixes: aea41afcfd6d ("ip bridge: Set NETLINK_GET_STRICT_CHK on socket") > Signed-off-by: Ido Schimmel <idosch@mellanox.com> > --- > bridge/fdb.c | 12 ++++++------ > 1 file changed, 6 insertions(+), 6 deletions(-) applied to iproute2-next. Thanks, Ido.
On Sun, 30 Dec 2018 18:03:47 -0700 David Ahern <dsahern@gmail.com> wrote: > On 12/30/18 10:14 AM, Ido Schimmel wrote: > > Since commit aea41afcfd6d ("ip bridge: Set NETLINK_GET_STRICT_CHK on > > socket") iproute2 uses strict checking on kernels that support it. This > > causes FDB dumping to fail [1], as iproute2 uses 'struct ifinfomsg' > > whereas the kernel expects 'struct ndmsg'. > > > > Note that with this change iproute2 continues to work on old kernels > > that do not support strict checking, but contain the fix introduced in > > kernel commit bd961c9bc664 ("rtnetlink: fix rtnl_fdb_dump() for ndmsg > > header"). Sorry, I don't think that is good enough backward compatibility guarantee. Iproute2 should work on really old kernels like 2.6.32. If not then then iproute2 utility is broken or the changes to the kernel API were incorrect. You can't depend on some old commit.
On Mon, Dec 31, 2018 at 08:25:38PM -0800, Stephen Hemminger wrote: > On Sun, 30 Dec 2018 18:03:47 -0700 > David Ahern <dsahern@gmail.com> wrote: > > > On 12/30/18 10:14 AM, Ido Schimmel wrote: > > > Since commit aea41afcfd6d ("ip bridge: Set NETLINK_GET_STRICT_CHK on > > > socket") iproute2 uses strict checking on kernels that support it. This > > > causes FDB dumping to fail [1], as iproute2 uses 'struct ifinfomsg' > > > whereas the kernel expects 'struct ndmsg'. > > > > > > Note that with this change iproute2 continues to work on old kernels > > > that do not support strict checking, but contain the fix introduced in > > > kernel commit bd961c9bc664 ("rtnetlink: fix rtnl_fdb_dump() for ndmsg > > > header"). > > Sorry, I don't think that is good enough backward compatibility guarantee. > Iproute2 should work on really old kernels like 2.6.32. If not then then iproute2 > utility is broken or the changes to the kernel API were incorrect. I found two more issues for which I have patches. I tested with and without strict checking and I'll test on some old kernels as well. No intention to break backward compatibility. Will Cc you. Thanks
diff --git a/bridge/fdb.c b/bridge/fdb.c index a5abc1b6c78d..a7a0d8052307 100644 --- a/bridge/fdb.c +++ b/bridge/fdb.c @@ -260,16 +260,16 @@ static int fdb_show(int argc, char **argv) { struct { struct nlmsghdr n; - struct ifinfomsg ifm; + struct ndmsg ndm; char buf[256]; } req = { - .n.nlmsg_len = NLMSG_LENGTH(sizeof(struct ifinfomsg)), - .ifm.ifi_family = PF_BRIDGE, + .n.nlmsg_len = NLMSG_LENGTH(sizeof(struct ndmsg)), + .ndm.ndm_family = PF_BRIDGE, }; char *filter_dev = NULL; char *br = NULL; - int msg_size = sizeof(struct ifinfomsg); + int msg_size = sizeof(struct ndmsg); while (argc > 0) { if ((strcmp(*argv, "brport") == 0) || strcmp(*argv, "dev") == 0) { @@ -313,10 +313,10 @@ static int fdb_show(int argc, char **argv) filter_index = ll_name_to_index(filter_dev); if (!filter_index) return nodev(filter_dev); - req.ifm.ifi_index = filter_index; + req.ndm.ndm_ifindex = filter_index; } - if (rtnl_dump_request(&rth, RTM_GETNEIGH, &req.ifm, msg_size) < 0) { + if (rtnl_dump_request(&rth, RTM_GETNEIGH, &req.ndm, msg_size) < 0) { perror("Cannot send dump request"); exit(1); }
Since commit aea41afcfd6d ("ip bridge: Set NETLINK_GET_STRICT_CHK on socket") iproute2 uses strict checking on kernels that support it. This causes FDB dumping to fail [1], as iproute2 uses 'struct ifinfomsg' whereas the kernel expects 'struct ndmsg'. Note that with this change iproute2 continues to work on old kernels that do not support strict checking, but contain the fix introduced in kernel commit bd961c9bc664 ("rtnetlink: fix rtnl_fdb_dump() for ndmsg header"). [1] # bridge fdb show [ 5365.137224] netlink: 4 bytes leftover after parsing attributes in process `bridge'. Error: bytes leftover after parsing attributes. Dump terminated Fixes: aea41afcfd6d ("ip bridge: Set NETLINK_GET_STRICT_CHK on socket") Signed-off-by: Ido Schimmel <idosch@mellanox.com> --- bridge/fdb.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-)