I run elog -o dir=. -f tcp -c tcp in my home directory and a client, which uses the IPPROTO_TCP-level socket option TP_EVENTLOG to enable logging. The resulting file has the name:

tuexen@freebsd16:~ % file *.elog
\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245_42405_\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245\245_42405.elog: data

Is that intended?

This does not compile for me on a GENERIC kernel. I noted the issues inline.

In D57392#1315323, @timo.voelker_fh-muenster.de wrote:

In D57392#1315045, @adrian wrote:

Neat! So hm what devices are on the virtio pci bus? storage, net, anything else? I'm just trying to imagine the device testing map for this change.

Thanks for chasing this down!

I'm not a VirtIO expert. As far as I know, every VirtIO device can use the PCI bus to attach to the system.

In sys/dev/virtio, each subfolder stands for one implemented VirtIO driver implementation (except pci). QEMU can emulate the corresponding device. On my macOS, I get this list

qemu-system-aarch64 -device help | grep virt.\*pci\"
name "virtio-9p-pci", bus PCI, alias "virtio-9p"
name "virtio-blk-pci", bus PCI, alias "virtio-blk"
name "virtio-scsi-pci", bus PCI, alias "virtio-scsi"
name "virtio-net-pci", bus PCI, alias "virtio-net"
name "virtio-keyboard-pci", bus PCI, alias "virtio-keyboard"
name "virtio-mouse-pci", bus PCI, alias "virtio-mouse"
name "virtio-multitouch-pci", bus PCI
name "virtio-serial-pci", bus PCI, alias "virtio-serial"
name "virtio-tablet-pci", bus PCI, alias "virtio-tablet"
name "virtio-gpu-pci", bus PCI, alias "virtio-gpu"
name "virtio-sound-pci", bus PCI, alias "virtio-sound", desc "Virtio Sound"
name "virtio-balloon-pci", bus PCI, alias "virtio-balloon"
name "virtio-crypto-pci", bus PCI
name "virtio-iommu-pci", bus PCI, alias "virtio-iommu"
name "virtio-rng-pci", bus PCI, alias "virtio-rng"

I focused my test on virtio-net-pci but used some other VirtIO devices (mostly virtio-blk-pci) as well. If you have the chance to test other VirtIO devices, that would be great.

rrs is currently traveling and has no access to phabricator Therefore he asked me to forward some questions:

As Mark suggested.

This was committed (by accident) in[[ https://cgit.FreeBSD.org/src/commit/?id=1bfd392b9e4dcddef3d80efaa517fafa648cd0b1 | 1bfd392b9e4d ]].

Address Marks comment.

I actually used and extended it. For example, syzkaller is dumping the information for all locked tcpcbs when panicing. This gives at least some information in a situation where you cannot access a core. This was good enough for me to fix some bugs.

In D56623#1297750, @ivy wrote:

i don't feel competent to review this (i don't own the hardware and know nothing about it) but @thj has committed to if_awg recently and might be able to help.

A second proposed solution integrating Patrick's proposal. Now ipopts is handled like cred.

In D57104#1309231, @glebius wrote:

I like Patrick's change more, but maybe with more verbose comment.

In D57104#1309249, @glebius wrote:

Of course this gets things a bit more sophisticated. I don't like that syncache_socket() will again pull the ipopts from the mbuf as the syncache_add() already just did. Maybe after the fix try to make this less sophisticated?

In D57104#1308954, @bms wrote:

s/net.inet.ip.accept_sourcerouting/net.inet.ip.accept_sourceroute/

In D57045#1307623, @markj wrote:

In D57045#1307615, @tuexen wrote:

In D57045#1307590, @des wrote:

I would definitely argue that not implementing a 15-year-old IETF BCP is a bug.

OK. So I learn that implementing a BCP is considered a bugfix and can be brought in via an EN.

There are no hard rules for determining whether a bugfix should be published as an EN. It's a judgement call:

• Does the change fix a widely reported problem? If only a few users seem to need the change (e.g., the patched feature is not widely used), we are less likely to issue an EN.
• Does the problem have a workaround? If so, then the pressure to publish an EN is lower.
• Is the patch complex and likely to introduce regressions or change behaviour in a surprising way? If so, we will be less comfortable publishing it.

In D57045#1307593, @markj wrote:

In D57045#1307588, @tuexen wrote:

In D57045#1307570, @markj wrote:

In D57045#1307566, @tuexen wrote:

OK. So you want to have an EN to add a feature to 15.0? Why do you need to complexity of the sysctl? Why can't you just take what is in the main, stable/15, and stable/14 branch?

Just to be clear: I like the support of RFC 6191. I just want to understand why you don't want to have it and therefore need a sysctl variable?

Because ENs should be conservative: users applying changes with freebsd-update cannot pick and choose the patches they apply. So changes shipped that way should aim to do as little as possible.

Can't the people provide a patch for their system or do they need an EN? If they need an EN, which do you need something you can disable? Wouldn't it be acceptable to just enable support for RFC 6191 unconditionally?

I am not convinced that we need or want this sysctl in main, stable/* etc.. But if we're going to ship the change in an EN, I think we should avoid changing the behaviour by default in releng/*; those that explicitly want RFC 6191 support can opt in. That's why I (on behalf of secteam) asked for the sysctl.

OK. So we want to deliver a new feature, off by default, in a EN to released versions and allow people to enable it. I just wasn't aware that we do this. But shouldn't we then also disable this feature in stable branches? In particular, should we disable this for 15.1? I can revert my unconditional enabling of RFC 6191 support and ask re@ to merge that into releng/15.1.

I don't really see why, unless the change is particularly risky. Users upgrading to 15.1 or tracking stable have more options if the upgrade causes problems, they can roll back to 15.0 or revert a particular patch etc.. Users applying SA/EN patches have fewer options, so we should avoid publishing ENs unless we're very confident in the change. In this case, the change seems delicate enough that we shouldn't automatically enable it.

In D57045#1307590, @des wrote:

I would definitely argue that not implementing a 15-year-old IETF BCP is a bug.

In D57045#1307570, @markj wrote:

In D57045#1307566, @tuexen wrote:

OK. So you want to have an EN to add a feature to 15.0? Why do you need to complexity of the sysctl? Why can't you just take what is in the main, stable/15, and stable/14 branch?

Just to be clear: I like the support of RFC 6191. I just want to understand why you don't want to have it and therefore need a sysctl variable?

Because ENs should be conservative: users applying changes with freebsd-update cannot pick and choose the patches they apply. So changes shipped that way should aim to do as little as possible.

Can't the people provide a patch for their system or do they need an EN? If they need an EN, which do you need something you can disable? Wouldn't it be acceptable to just enable support for RFC 6191 unconditionally?

I am not convinced that we need or want this sysctl in main, stable/* etc.. But if we're going to ship the change in an EN, I think we should avoid changing the behaviour by default in releng/*; those that explicitly want RFC 6191 support can opt in. That's why I (on behalf of secteam) asked for the sysctl.

In D57045#1307543, @kevans wrote:

In D57045#1307542, @tuexen wrote:

In D57045#1307541, @kevans wrote:

In D57045#1307188, @tuexen wrote:

In D57045#1307179, @des wrote:

In D57045#1307167, @tuexen wrote:

Please note that the support of RFC 6191 has been MFC'ed to stable/14 and stable/15. I did not use a sysctl variable, since I don't see a need for adding the configuration complexity for this feature. Why would you want to not use it?

Our intention is to merge this to 15 and 14 as well, then do an EN with the combined change.

What is the errata about? Did my change introduce a bug? Adding the sysctl-variable doubles the testing effort. Not sure why we need it. Could you elaborate?

Ideally we deprecate the sysctl in the medium-term, but the desire is that we have the RFC6191 behavior in release branches to be able to solve the bug originally reported. There's some desire to be more conservative since there have been few known reports of bugs with the previous behavior, so the knob allows it to be retrofitted without breaking anyone.

Are there any reports that the new behavior is problematic? The current behavior is not a bug. Adding support RFC 6191 is adding a feature which will be in upcoming 15 and 14 releases. Are we using ENs to add features?

The pre-6191 behavior is a bug for the folks that originally requested it, because it results in significant drops that should be avoided. The new feature that avoids those drops is incredibly light and easy to keep disabled for the release branches only, thus the knob that will get its default flipped for existing releases (but maintain current state in stable and main).

In D57045#1307541, @kevans wrote:

In D57045#1307188, @tuexen wrote:

In D57045#1307179, @des wrote:

In D57045#1307167, @tuexen wrote:

Please note that the support of RFC 6191 has been MFC'ed to stable/14 and stable/15. I did not use a sysctl variable, since I don't see a need for adding the configuration complexity for this feature. Why would you want to not use it?

Our intention is to merge this to 15 and 14 as well, then do an EN with the combined change.

What is the errata about? Did my change introduce a bug? Adding the sysctl-variable doubles the testing effort. Not sure why we need it. Could you elaborate?

Ideally we deprecate the sysctl in the medium-term, but the desire is that we have the RFC6191 behavior in release branches to be able to solve the bug originally reported. There's some desire to be more conservative since there have been few known reports of bugs with the previous behavior, so the knob allows it to be retrofitted without breaking anyone.

In D57045#1307179, @des wrote:

In D57045#1307167, @tuexen wrote:

Please note that the support of RFC 6191 has been MFC'ed to stable/14 and stable/15. I did not use a sysctl variable, since I don't see a need for adding the configuration complexity for this feature. Why would you want to not use it?

Our intention is to merge this to 15 and 14 as well, then do an EN with the combined change.

Please note that the support of RFC 6191 has been MFC'ed to stable/14 and stable/15. I did not use a sysctl variable, since I don't see a need for adding the configuration complexity for this feature. Why would you want to not use it?

Be more explicit in the comment as suggested my markj@ and rscheff@.

Use bpf instead of bpf* as suggested by markj@.

In D56742#1299817, @glebius wrote:

Sorry for slightly switching context. This actually links to the recent "regression" that I planted. With a new API to get exact list of possible bpf taps, instead of guessing via getifaddrs(3) + list USB buses, the building of the list requires privileges. Previously you could run tcpdump -D without privileges on FreeBSD, now it can't. The wireshark/tcpdump guys think that it is a regression and should be fixed.

I can understand that.

There are two ways to solve:

1. Change permissions on /dev/bpf and do privileges checking inside the device code. Allow BIOCGETIFLIST for unprivileged user.
2. In libpcap fallback to getifaddrs(3)+/dev/usb guessing if opening /dev/bpf failed.

Your opinions?

I would tend to 2. I guess most people run at least wireshark with 0660 on bpf devices, since this is what it recommended when the port is installed, they would get the new code with higher quality output. The rest gets what it was getting earlier.

In D56742#1299590, @markj wrote:

In particular, this allows members of the network group to monitor traffic without running with root privileges.

It also lets those members inject packets... maybe the suggested permissions should be 0640 at least?

Use 0640 as markj@ suggested.