Fix inadvertent txqueuelen being set to zero#1100
Fix inadvertent txqueuelen being set to zero#1100squeed merged 1 commit intocontainernetworking:mainfrom
Conversation
d376c8d to
4ffccfd
Compare
|
Been trying to sort out what's up with the failing |
|
Thanks for the PR. Have you seen #1097? Are these two PRs duplicate or complementary? |
|
@squeed The second issue #1097 describes is somewhat related, but it misses the mark on setting it to zero meaning that the kernel will pick a default. This can be illustrated as such: $ ip link add dev vm1 type veth
$ ip link show vm1
... snip snip snip ...
20: vm1@vm2: <BROADCAST,MULTICAST,M-DOWN> mtu 1500 qdisc noop state DOWN mode DEFAULT group default qlen 1000
link/ether 76:8b:12:5d:e9:c0 brd ff:ff:ff:ff:ff:ffNote the $ ip link add dev vm1 txqueuelen 0 type veth
$ ip link show vm1
24: vm1@veth0: <BROADCAST,MULTICAST,M-DOWN> mtu 1500 qdisc noop state DOWN mode DEFAULT group default
link/ether 6a:19:4e:0b:3a:5e brd ff:ff:ff:ff:ff:ffNote the missing What I did notice on our end when we worked around this issue, was that we saw less tx packet drops. Now I can't remember the specifics on whether that was on |
|
(@LionelJouin found the cause of the flake, he should have a fix soon) |
squeed
left a comment
There was a problem hiding this comment.
Looks good to me. Mind if I squash this in to a single commit?
3da6c95 to
a73af81
Compare
|
Rebased to pick up SBR fixes. |
TxQLen was unintentionally set to 0 due to a struct literal. Signed-off-by: Gudmundur Bjarni Olafsson <gudmundur.bjarni@gmail.com>
a73af81 to
8831990
Compare
|
@squeed Took care of the squashing for you. Reworded the commit message for a single commit as well. |
When diagnosing networking issues, I noticed that
txqueuelenonvethpairs andtapdevices was set to zero. In investigating the issue, I found that most usages ofnetlink.LinkAttrsin this repository uses the struct literal, which in thenetlinkdocs is advised against unlessTxQLenis explicitly set.When using the
ipcommand directly without specifyingtxqueuelen, Linux will automatically to 1000:This change goes through every single usage of
netlink.LinkAttrs{and replaces it with thenetlink.NewLinkAttrsconstructor, and then overrides the individual fields in the struct.I even noticed that in the
bridgeplugin, this is somewhat called out when using the literal. I've replaced this explicitly in d376c8d though.