`NetlinkMixin.get` moves additional errors to backlog zero

[tik-stbuehler]

Hi.

In NetlinkMixin.get if an error is found, the remaining backlog for that sequence is moved to "backlog zero", including error messages.

But new error messages for a non-existing backlog or ignored.

I think the latter behavior is the correct one, and orphaned error messages should not appear on "backlog zero".

(I think it might even be reasonable to drop all messages for unknown sequence ids, or to make moving them to "backlog zero" at least depending on some flag.)

Context: with netfilter batch transactions it'd be nice to use a single sequence number (get doesn't seam to deal well with multiple sequence ids to wait for simultaneously or with ids that might not even create a response on success, as neither NFNL_MSG_BATCH_BEGIN nor NFNL_MSG_BATCH_END supports NLM_F_ACK). Then one can simply set NLM_F_ACK only for the last message in a batch (before the end), and either receive an ack with error=0 or an real error - but you might get multiple errors, and the additional ones look like they'd end up on backlog zero if they are read before the backlog gets destroyed.

The workaround is to clear backlog zero after such transactions (i.e. it can't be used for anything else).

cheers,
Stefan

[svinota]

It took some time to understand the issue, but I believe I've got it.

Give me a day to double check if we need to keep the remaining messages after the first error.

And the subsequent errors should not end up in the backlog zero in any case, you're right.

[svinota]

Pls check out the branch, if it will work.

[tik-stbuehler]

Hi. Sorry for the delayed response.

Here is some demo code demonstrating the issue. Your commit fixes the "real" errors, but not the ack (errno=0) messages.

(Needs to be run as root; with user namespaces easy to do like this: unshare -rn python3 demo_batch_errors.py)

from pr2modules.netlink.nfnetlink import nftsocket
from pr2modules.netlink import NETLINK_NETFILTER, nlsocket, NLM_F_REQUEST, NLM_F_ACK
from pr2modules.netlink.nfnetlink import nfgen_msg
from pr2modules.netlink.nfnetlink import NFNL_SUBSYS_NFTABLES
from pr2modules.netlink.exceptions import NetlinkError


def encode(**value) -> bytes:
    v = nfgen_msg()
    v.setvalue(value)
    v.encode()
    return v.data


def main():
    print("test")
    sock = nlsocket.NetlinkSocket(family=NETLINK_NETFILTER)
    policy = {
        (x | (NFNL_SUBSYS_NFTABLES << 8)): y
        for (x, y) in nftsocket.NFTSocket.policy.items()
    }
    sock.register_policy(policy)
    sock.bind()

    # first message: batch
    msg_seq = sock.addr_pool.alloc()
    msg_data = b''
    msg_data += encode(
        res_id=NFNL_SUBSYS_NFTABLES,
        header=dict(
            type=0x10,  # NFNL_MSG_BATCH_BEGIN
            flags=NLM_F_REQUEST,
            sequence_number=msg_seq,
        ),
    )
    # this should fail
    msg_data += encode(
        header=dict(
            type=(NFNL_SUBSYS_NFTABLES << 8) | nftsocket.NFT_MSG_GETTABLE,
            flags=NLM_F_REQUEST | NLM_F_ACK,
            sequence_number=msg_seq,
        ),
    )
    # this should fail too
    msg_data += encode(
        header=dict(
            type=(NFNL_SUBSYS_NFTABLES << 8) | nftsocket.NFT_MSG_GETCHAIN,
            flags=NLM_F_REQUEST | NLM_F_ACK,
            sequence_number=msg_seq,
        ),
    )
    # this should return an Ack (error with errno=0)
    msg_data += encode(
        header=dict(
            type=(NFNL_SUBSYS_NFTABLES << 8) | nftsocket.NFT_MSG_DELTABLE,
            flags=NLM_F_REQUEST | NLM_F_ACK,
            sequence_number=msg_seq,
        ),
    )
    msg_data += encode(
        res_id=NFNL_SUBSYS_NFTABLES,
        header=dict(
            type=0x11,  # NFNL_MSG_BATCH_END
            flags=NLM_F_REQUEST,
            sequence_number=msg_seq,
        ),
    )
    with sock.lock[msg_seq]:
        sock.backlog[msg_seq] = []
    sock.sendto(msg_data, (0, 0))

    # second message: simple "get", should succeed (when run as root)
    # when waiting for reponse to this the errors for the first message will be put into the backlog
    sock.nlm_request({}, (NFNL_SUBSYS_NFTABLES << 8) | nftsocket.NFT_MSG_GETGEN, msg_flags=NLM_F_REQUEST)

    try:
        # this should raise an error for the first failure and cleanup the backlog for `msq_seq`
        print(sock.get(msg_seq=msg_seq))
    except NetlinkError:
        pass
    finally:
        with sock.lock[0]:
            # we don't want neither real errors nor Acks here:
            print("Global Backlog: ", sock.backlog[0])


if __name__ == "__main__":
    main()

[svinota]

Thanks, working on it.

[svinota]

The only way I see so far is to completely drop the response messages after the first error. I hope that it will not break any existing applications, but let's see.

Or to move the messages into a separate debug backlog of a limited size for possible investigation, if there will be any.

[tik-stbuehler]

I think there are various ways one could go for, but I'd recommend consistent handling between receveing new messages for unknown sequences ids and clearing an existing backlog, as otherwise you'll get "race conditions" in the semantics.

[svinota]

Closing the ticket. Feel free to reopen in the case if more features needed here.