In D6611#580317, @gallatin wrote:

In D6611#580267, @jhb wrote:

I think one question is how can you really identify these packets? You can send a packet into a VM (e.g. via an if_tap interface from a localhost connection) but that VM might itself forward the packet internally to another interface that then heads out onto the wire. You can't really know that before you send the packet into the if_tap, and the VM itself doesn't know which interfaces are internal and which are not.

I considered & rejected some magic solution where we constrain virtual interface mac addresses to a range that is unique and which can be easily checked. Then we'd know if a packet came in on a virtual interface & needs checksumming. But that doesn't work if the VM has pass-thru access to a physical NIC and can send a packet w/o going through the host.

In D6656#578863, @gallatin wrote:

In D6656#578326, @gallatin wrote:

I'm confused. t_maxsegsize is basically the connection's MSS, eg, what the NIC is supposed to break the TSO into. However if_hw_tsomaxsegsize is the max size of the TSO to be broken up. Eg, on a NIC without oddball limitations, if_hw_tsomaxsegcount might be 64, if_hw_tsomaxsegsize might be 65535. That means that TSO is supposed to send a max of 64K down to the NIC, in a max of 64 packets. If t_maxseg is less than 1000, then we'd have to limit the TSO to 64 packets.

It seems like by restricting if_hw_tsomaxsegsize to t_maxseg, you're essentially turning off TSO, since you're only allowing one MSS to be sent at a time.

Ugh, I'm wrong. I was confusing t_tsomax with t_tsomaxsegsize. I forgot that some NICs are apparently so broken that they limit TSO seg sizes.

In D6611#578858, @gallatin wrote:

This will break some NICs (see comments on D6656 -- basically, if you're setting CSUM_TCP then you need to replace the TCP checksum with the pseudohdr checksum, and store the offset of the TCP header's checksum in csum_data).

The important question is: If the packet has already had its checksum checked, then the checksum is correct. Why not update the checksum normally and send the packet without checksum offload enabled?

In D6611#578198, @rgrimes wrote:

Is there any concern or additional work needed for ipV6?

Ping? I would like to get this committed so xen-tools package builds again on HEAD. Thanks!

Also fix clang 10 build, by adding -Wno-misleading-indentation to
tools build

Thanks!

bump port revision

Thanks, I have a couple of comments about the need for GIANT on some of the sysctls. I have to admit I'm not very familiar with sysctls, so I might be missing something.

Moved iconv dependency from LIB_DEPENDS to USES.

LGTM, it's a shame we don't have a more declarative way to express such dependencies.

tyvm!

Hello, can I please get this approved?

Ping? This is an almost trivial change, since it's a straight backport from an upstream commit.

In D20770#450048, @mindal_semihalf.com wrote:

Interestingly enough on arm and powerpc that array is stored as a part of dma map structure, so we might run into that anyway.
Anyway I agree with you that we need to have the same behavior across architectures.

In D20770#450023, @mindal_semihalf.com wrote:

In D20770#449642, @royger wrote:

In D20770#449628, @mindal_semihalf.com wrote:

In D20770#449275, @royger wrote:

In D20770#449259, @mindal_semihalf.com wrote:

In D20770#449256, @royger wrote:

Can you be more specific about the flow that can lead to overwrite the segs array?

Basically if you have a lot of threads doing DMA loads on the same tag concurrently there is a small chance of the segs array being overwritten before it is read in the callback.
It is a rare race, but reproducible.

Hm, I'm not sure I see how that's supposed to happen. busdma_swi already takes a lock (dmat->common.lockfunc) to prevent issuing multiple loads on the same tag, thus there should be no overwriting while the caller callback is using tag->segs?

Well, I'm not sure how dusdma_swi can help us in this case.
If you go through the flow starting at bus_dmamap_load, then going into bounce_bus_dmamap_load_buffer no lock is acquired there.

Right, the lock is only used for the deferred callback.

I think this is fine, but should be done for x86 also at least (that's where arm64 busdma code is coming from). This expected behaviour should also be mentioned on the busdma man page.

I think the general expectation of busdma code is that you serialize loads when using the same tag, but I cannot find an explicit reference to it on the man page, and IMO the locking section is too vague and doesn't explicitly say that you are not supposed to issue concurrent load calls using the same tag.

Ok, so as you said we have two ways to proceed from here:

1. Modify the manual to explicitly require the user to serialize all loads on the same tag.

Do we know if all existing drivers follow that rule?

2. Move the segmets array on all architectures.

I could prepare the patch, but frankly I don't feel competent enough to make such design decision.

In D20770#449628, @mindal_semihalf.com wrote:

In D20770#449275, @royger wrote:

In D20770#449259, @mindal_semihalf.com wrote:

In D20770#449256, @royger wrote:

Can you be more specific about the flow that can lead to overwrite the segs array?

Basically if you have a lot of threads doing DMA loads on the same tag concurrently there is a small chance of the segs array being overwritten before it is read in the callback.
It is a rare race, but reproducible.

Hm, I'm not sure I see how that's supposed to happen. busdma_swi already takes a lock (dmat->common.lockfunc) to prevent issuing multiple loads on the same tag, thus there should be no overwriting while the caller callback is using tag->segs?

Well, I'm not sure how dusdma_swi can help us in this case.
If you go through the flow starting at bus_dmamap_load, then going into bounce_bus_dmamap_load_buffer no lock is acquired there.

In D20770#449259, @mindal_semihalf.com wrote:

In D20770#449256, @royger wrote:

Can you be more specific about the flow that can lead to overwrite the segs array?

Basically if you have a lot of threads doing DMA loads on the same tag concurrently there is a small chance of the segs array being overwritten before it is read in the callback.
It is a rare race, but reproducible.

I'm not sure I understand the motivation behind that change, is this fixing an existing issue with a specific driver?

Fixed in the new version, thanks!

s/FILESDIR/PATCHDIR/

In D20148#433866, @kevans wrote:

I suspect you'll need to bump some g_raid parts as well- it has modules declared at SI_SUB_DRIVERS/SI_ORDER_{FIRST,SECOND,THIRD} that I suspect could/should be bumped to SECOND/THIRD/FOURTH -- but I haven't had time to dig into it and examine the dependencies there.

In D19591#419483, @novel wrote:

Shouldn't this patch be added to EXTRA_PATCHES?

• Use a explicit target to only build man pages, instead of using the generic docs target and expecting only the man pages to be built.
• Remove the markdown dependency, the html files can be easily fetched from https://xenbits.xen.org/docs/.

In D19592#419471, @mat wrote:

What happened to all the html files in DOCSDIR?

In D19521#418756, @mat wrote:

The manpages should not be behind the DOCS option, they should either always be installed, or be behind a MANPAGES option (that defaults to on).

Forgot to ask but can you commit this?

Update to add patches for XSA-{284,287,290,292-294}

LGTM, thanks!

Ping?

Add a couple more of backported fixes that should improve the
user-experience, ie: prevent Xen from crashing or deadlocking.