test branch of patches that use a "tc" filter to fixup UDP checksums
This branch uses tc to fixup the checksum. It's appearing to work with the few guests I've tested so far, but there is an issue when a network has <bandwidth> configured, and I don't want to post the patches for review until I figure that out. In the meantime anyone who wants to test with a non-<bandwidth> network, please do :-) https://gitlab.com/lainestump/libvirt/-/tree/network-freebsd-checksum-fix-tc...
On Fri, Nov 22, 2024 at 03:28:00AM -0500, Laine Stump wrote:
This branch uses tc to fixup the checksum. It's appearing to work with the few guests I've tested so far, but there is an issue when a network has <bandwidth> configured, and I don't want to post the patches for review until I figure that out. In the meantime anyone who wants to test with a non-<bandwidth> network, please do :-)
https://gitlab.com/lainestump/libvirt/-/tree/network-freebsd-checksum-fix-tc...
I've tested this with a pretty large variety of guest operating systems, including the ones that had proven to be incompatible with the previously attempted solution (GNU/Hurd, Haiku, NetBSD) and those that are not working correctly today (FreeBSD). Everything seems to be working fine. I've never used the <bandwidth> feature so I can't comment on that. Is it something that is already broken in master, or do these patches regress that functionality? -- Andrea Bolognani / Red Hat / Virtualization
On 11/22/24 11:00 AM, Andrea Bolognani wrote:
On Fri, Nov 22, 2024 at 03:28:00AM -0500, Laine Stump wrote:
This branch uses tc to fixup the checksum. It's appearing to work with the few guests I've tested so far, but there is an issue when a network has <bandwidth> configured, and I don't want to post the patches for review until I figure that out. In the meantime anyone who wants to test with a non-<bandwidth> network, please do :-)
https://gitlab.com/lainestump/libvirt/-/tree/network-freebsd-checksum-fix-tc...
I've tested this with a pretty large variety of guest operating systems, including the ones that had proven to be incompatible with the previously attempted solution (GNU/Hurd, Haiku, NetBSD) and those that are not working correctly today (FreeBSD). Everything seems to be working fine.
Thanks for the testing - that's very reassuring!
I've never used the <bandwidth> feature so I can't comment on that. Is it something that is already broken in master, or do these patches regress that functionality?
I actually didn't test it in master, but assume that it was working. Both <bandwidth> and the tc-based checksum fix use the same tc "qdisc" (queue discipline) object to attach their filters, so I've put the command that adds the qdisc into a separate function that 1) checks if there's already a qdisc on the interface, and 2) if not it adds one (using the same code as bandwidth previously used). Unfortunately the bandwidth code fails to add its filter if the qdisc had been added by the checksum fix code. I need to grab the exact commandlines out of the util.command debug log and try reproducing the issue by hand - hopefully it can be solved by some small change to the exact commands that are used. I do think that the structure of the code is sound though, and it can all be solved by just modifying a commandline argument or two, so it would be totally reasonable to go over the code to look for obvious ugliness at least :-)
On 11/22/24 12:01 PM, Laine Stump wrote:
On 11/22/24 11:00 AM, Andrea Bolognani wrote:
On Fri, Nov 22, 2024 at 03:28:00AM -0500, Laine Stump wrote:
This branch uses tc to fixup the checksum. It's appearing to work with the few guests I've tested so far, but there is an issue when a network has <bandwidth> configured, and I don't want to post the patches for review until I figure that out. In the meantime anyone who wants to test with a non-<bandwidth> network, please do :-)
https://gitlab.com/lainestump/libvirt/-/tree/network-freebsd- checksum-fix-tc2
I've tested this with a pretty large variety of guest operating systems, including the ones that had proven to be incompatible with the previously attempted solution (GNU/Hurd, Haiku, NetBSD) and those that are not working correctly today (FreeBSD). Everything seems to be working fine.
Thanks for the testing - that's very reassuring!
I've never used the <bandwidth> feature so I can't comment on that. Is it something that is already broken in master, or do these patches regress that functionality?
I actually didn't test it in master, but assume that it was working. Both <bandwidth> and the tc-based checksum fix use the same tc "qdisc" (queue discipline) object to attach their filters, so I've put the command that adds the qdisc into a separate function that 1) checks if there's already a qdisc on the interface, and 2) if not it adds one (using the same code as bandwidth previously used).
Unfortunately the bandwidth code fails to add its filter if the qdisc had been added by the checksum fix code. I need to grab the exact commandlines out of the util.command debug log and try reproducing the issue by hand - hopefully it can be solved by some small change to the exact commands that are used.
I've just learned that the bandwidth filter and the checksum-fix filter need to have different priority settings in order to not clash - once I make the checksum-fix "priority 2" it all starts working! I'm going to push that to my gitlab branch, do some testing, and then post the patches to the list later today (or possibly in the morning)
I do think that the structure of the code is sound though, and it can all be solved by just modifying a commandline argument or two, so it would be totally reasonable to go over the code to look for obvious ugliness at least :-)
On 11/22/24 1:01 PM, Laine Stump wrote:
On 11/22/24 12:01 PM, Laine Stump wrote:
On 11/22/24 11:00 AM, Andrea Bolognani wrote:
On Fri, Nov 22, 2024 at 03:28:00AM -0500, Laine Stump wrote:
This branch uses tc to fixup the checksum. It's appearing to work with the few guests I've tested so far, but there is an issue when a network has <bandwidth> configured, and I don't want to post the patches for review until I figure that out. In the meantime anyone who wants to test with a non-<bandwidth> network, please do :-)
https://gitlab.com/lainestump/libvirt/-/tree/network-freebsd- checksum-fix-tc2
I've tested this with a pretty large variety of guest operating systems, including the ones that had proven to be incompatible with the previously attempted solution (GNU/Hurd, Haiku, NetBSD) and those that are not working correctly today (FreeBSD). Everything seems to be working fine.
Thanks for the testing - that's very reassuring!
I've never used the <bandwidth> feature so I can't comment on that. Is it something that is already broken in master, or do these patches regress that functionality?
I actually didn't test it in master, but assume that it was working. Both <bandwidth> and the tc-based checksum fix use the same tc "qdisc" (queue discipline) object to attach their filters, so I've put the command that adds the qdisc into a separate function that 1) checks if there's already a qdisc on the interface, and 2) if not it adds one (using the same code as bandwidth previously used).
Unfortunately the bandwidth code fails to add its filter if the qdisc had been added by the checksum fix code. I need to grab the exact commandlines out of the util.command debug log and try reproducing the issue by hand - hopefully it can be solved by some small change to the exact commands that are used.
I've just learned that the bandwidth filter and the checksum-fix filter need to have different priority settings in order to not clash - once I make the checksum-fix "priority 2" it all starts working!
I'm going to push that to my gitlab branch, do some testing, and then post the patches to the list later today (or possibly in the morning)
I fixed the clash between the two filter rules, and posted the patches to the list for anyone who wants to review.
participants (3)
-
Andrea Bolognani -
Laine Stump -
Laine Stump