Re: test branch of patches that use a "tc" filter to fixup UDP checksums

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)