tcp_rst() complications

tcp_rst() complications

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 2031 bytes --]

Amongst other things, I spotted some additional complications using
tcp_rst() in the migration path (some of which might also have
implications in other contexts).  These might be things we can safely
ignore, at least for now, but I haven't thought through them enough to
be sure.

1) Sending RST to guest during migration

The first issue is that tcp_rst() will send an actual RST to the guest
on the tap interface.  During migration, that means we're sending to
the guest while it's suspended.  At the very least that means we
probably have a much higher that usual chance of getting a queue full
failure writing to the tap interface, which could hit problem (2).

But, beyond that, with vhost-user that means we're writing to guest
memory while the guest is suspended.  Kind of the whole point of the
suspended time is that the guest memory doesn't change during it, so
I'm not sure what the consequences will be.  Now, at the moment I
think all our tcp_rst() calls are either on the source during rollback
(i.e. we're committed to resuming only on the source) or on the target
past the point of no return (i.e. we're committed to resuming only on
the target).  I suspect that means we can get away with it, but I do
worry this could break something in qeme by violating that assumption.

2) tcp_rst() failures

tcp_rst() can fail if tcp_send_flag() fails.  In this case we *don't*
change the events to CLOSED.  I _think_ that's a bug: even if we
weren't able to send the RST to the guest, we've already closed the
socket so the flow is dead.  Moving to CLOSED state (and then removing
the flow entirely) should mean that we'll resend an RST if the guest
attempts to use the flow again later.

But.. I was worried there might be some subtle reason for not changing
the event state in that case.

-- 
David Gibson (he or they)	| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you, not the other way
				| around.
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: tcp_rst() complications

[Stefano Brivio]

On Wed, 26 Feb 2025 20:05:25 +1100
David Gibson <david@gibson.dropbear.id.au> wrote:

> Amongst other things, I spotted some additional complications using
> tcp_rst() in the migration path (some of which might also have
> implications in other contexts).  These might be things we can safely
> ignore, at least for now, but I haven't thought through them enough to
> be sure.
> 
> 1) Sending RST to guest during migration
> 
> The first issue is that tcp_rst() will send an actual RST to the guest
> on the tap interface.  During migration, that means we're sending to
> the guest while it's suspended.  At the very least that means we
> probably have a much higher that usual chance of getting a queue full
> failure writing to the tap interface, which could hit problem (2).
> 
> But, beyond that, with vhost-user that means we're writing to guest
> memory while the guest is suspended.  Kind of the whole point of the
> suspended time is that the guest memory doesn't change during it, so
> I'm not sure what the consequences will be.

If I recall correctly I checked this and something in the vhost-user
code will tell us that the queue is not ready yet, done.

Ideally we want to make sure we queue those, but queue sizes are finite
and I don't think we can guarantee we can pile up 128k RST segments.

Right now I would check that the functionality is not spectacularly
broken (I looked into that briefly, QEMU didn't crash, guest kept
running, but I didn't check further than that). If we miss a RST too
bad, things will time out eventually.

As a second step we could perhaps introduce a post-migration stage and
move calling tcp_rst() to there if the connection is in a given state?

> Now, at the moment I
> think all our tcp_rst() calls are either on the source during rollback
> (i.e. we're committed to resuming only on the source) or on the target
> past the point of no return (i.e. we're committed to resuming only on
> the target).  I suspect that means we can get away with it, but I do
> worry this could break something in qeme by violating that assumption.
> 
> 2) tcp_rst() failures
> 
> tcp_rst() can fail if tcp_send_flag() fails.  In this case we *don't*
> change the events to CLOSED.  I _think_ that's a bug: even if we
> weren't able to send the RST to the guest, we've already closed the
> socket so the flow is dead.  Moving to CLOSED state (and then removing
> the flow entirely) should mean that we'll resend an RST if the guest
> attempts to use the flow again later.
> 
> But.. I was worried there might be some subtle reason for not changing
> the event state in that case.

Not very subtle: my original idea was that if we fail to send the RST,
we should note that (by not moving to CLOSED) and try again from a
timer.

In practice I've never observed a tcp_send_flag() failure so I'm not
sure if that mechanism even works. Moving the socket to CLOSED sounds
totally okay to me, surely simpler, and probably more robust.

-- 
Stefano

Re: tcp_rst() complications

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 4376 bytes --]

On Wed, Feb 26, 2025 at 11:15:07AM +0100, Stefano Brivio wrote:
> On Wed, 26 Feb 2025 20:05:25 +1100
> David Gibson <david@gibson.dropbear.id.au> wrote:
> 
> > Amongst other things, I spotted some additional complications using
> > tcp_rst() in the migration path (some of which might also have
> > implications in other contexts).  These might be things we can safely
> > ignore, at least for now, but I haven't thought through them enough to
> > be sure.
> > 
> > 1) Sending RST to guest during migration
> > 
> > The first issue is that tcp_rst() will send an actual RST to the guest
> > on the tap interface.  During migration, that means we're sending to
> > the guest while it's suspended.  At the very least that means we
> > probably have a much higher that usual chance of getting a queue full
> > failure writing to the tap interface, which could hit problem (2).
> > 
> > But, beyond that, with vhost-user that means we're writing to guest
> > memory while the guest is suspended.  Kind of the whole point of the
> > suspended time is that the guest memory doesn't change during it, so
> > I'm not sure what the consequences will be.
> 
> If I recall correctly I checked this and something in the vhost-user
> code will tell us that the queue is not ready yet, done.

Ah, yes.  I'd seen those "Got packet, but RX virtqueue is not usable
yet" messages, but didn't make the connection.

I had been concerned that might only happen when we tried to kick the
queue, after we'd touched guest memory, but that doesn't appear to be
the case.  Ok, now I'm reasonably convinced this is ifne.

> Ideally we want to make sure we queue those, but queue sizes are finite
> and I don't think we can guarantee we can pile up 128k RST segments.

Well that, plus with vhost-user we don't really have anywhere we can
queue them other than in guest memory.

> Right now I would check that the functionality is not spectacularly
> broken (I looked into that briefly, QEMU didn't crash, guest kept
> running, but I didn't check further than that). If we miss a RST too
> bad, things will time out eventually.
> 
> As a second step we could perhaps introduce a post-migration stage and
> move calling tcp_rst() to there if the connection is in a given state?
> 
> > Now, at the moment I
> > think all our tcp_rst() calls are either on the source during rollback
> > (i.e. we're committed to resuming only on the source) or on the target
> > past the point of no return (i.e. we're committed to resuming only on
> > the target).  I suspect that means we can get away with it, but I do
> > worry this could break something in qeme by violating that assumption.
> > 
> > 2) tcp_rst() failures
> > 
> > tcp_rst() can fail if tcp_send_flag() fails.  In this case we *don't*
> > change the events to CLOSED.  I _think_ that's a bug: even if we
> > weren't able to send the RST to the guest, we've already closed the
> > socket so the flow is dead.  Moving to CLOSED state (and then removing
> > the flow entirely) should mean that we'll resend an RST if the guest
> > attempts to use the flow again later.
> > 
> > But.. I was worried there might be some subtle reason for not changing
> > the event state in that case.
> 
> Not very subtle: my original idea was that if we fail to send the RST,
> we should note that (by not moving to CLOSED) and try again from a
> timer.

Ok.  Makes sense, but I don't think it's necessary. We can treat a
failure here as if the RST was lost on the wire: we just forget the
connection, and if the guest doesn't drop it we'll send it another RST
if it tries to send a packet to the connection we no longer know
about.

> In practice I've never observed a tcp_send_flag() failure so I'm not
> sure if that mechanism even works.

I don't think the mechanism even exists.  I don't think there's any
timer that does this, and more to the point, if we don't change events
we basically don't have any record that we already reset the connection.

> Moving the socket to CLOSED sounds
> totally okay to me, surely simpler, and probably more robust.

Ok.  I'll go ahead with that.

-- 
David Gibson (he or they)	| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you, not the other way
				| around.
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: tcp_rst() complications

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 5748 bytes --]

On Thu, Feb 27, 2025 at 12:57:15PM +1100, David Gibson wrote:
> On Wed, Feb 26, 2025 at 11:15:07AM +0100, Stefano Brivio wrote:
> > On Wed, 26 Feb 2025 20:05:25 +1100
> > David Gibson <david@gibson.dropbear.id.au> wrote:
> > 
> > > Amongst other things, I spotted some additional complications using
> > > tcp_rst() in the migration path (some of which might also have
> > > implications in other contexts).  These might be things we can safely
> > > ignore, at least for now, but I haven't thought through them enough to
> > > be sure.
> > > 
> > > 1) Sending RST to guest during migration
> > > 
> > > The first issue is that tcp_rst() will send an actual RST to the guest
> > > on the tap interface.  During migration, that means we're sending to
> > > the guest while it's suspended.  At the very least that means we
> > > probably have a much higher that usual chance of getting a queue full
> > > failure writing to the tap interface, which could hit problem (2).
> > > 
> > > But, beyond that, with vhost-user that means we're writing to guest
> > > memory while the guest is suspended.  Kind of the whole point of the
> > > suspended time is that the guest memory doesn't change during it, so
> > > I'm not sure what the consequences will be.
> > 
> > If I recall correctly I checked this and something in the vhost-user
> > code will tell us that the queue is not ready yet, done.
> 
> Ah, yes.  I'd seen those "Got packet, but RX virtqueue is not usable
> yet" messages, but didn't make the connection.

Significantly, of course, that case also still returns 0 from
tcp_send_flag() so we carry on with changing the state to CLOSED.

> I had been concerned that might only happen when we tried to kick the
> queue, after we'd touched guest memory, but that doesn't appear to be
> the case.  Ok, now I'm reasonably convinced this is ifne.
> 
> > Ideally we want to make sure we queue those, but queue sizes are finite
> > and I don't think we can guarantee we can pile up 128k RST segments.
> 
> Well that, plus with vhost-user we don't really have anywhere we can
> queue them other than in guest memory.
> 
> > Right now I would check that the functionality is not spectacularly
> > broken (I looked into that briefly, QEMU didn't crash, guest kept
> > running, but I didn't check further than that). If we miss a RST too
> > bad, things will time out eventually.
> > 
> > As a second step we could perhaps introduce a post-migration stage and
> > move calling tcp_rst() to there if the connection is in a given state?
> > 
> > > Now, at the moment I
> > > think all our tcp_rst() calls are either on the source during rollback
> > > (i.e. we're committed to resuming only on the source) or on the target
> > > past the point of no return (i.e. we're committed to resuming only on
> > > the target).  I suspect that means we can get away with it, but I do
> > > worry this could break something in qeme by violating that assumption.
> > > 
> > > 2) tcp_rst() failures
> > > 
> > > tcp_rst() can fail if tcp_send_flag() fails.  In this case we *don't*
> > > change the events to CLOSED.  I _think_ that's a bug: even if we
> > > weren't able to send the RST to the guest, we've already closed the
> > > socket so the flow is dead.  Moving to CLOSED state (and then removing
> > > the flow entirely) should mean that we'll resend an RST if the guest
> > > attempts to use the flow again later.
> > > 
> > > But.. I was worried there might be some subtle reason for not changing
> > > the event state in that case.
> > 
> > Not very subtle: my original idea was that if we fail to send the RST,
> > we should note that (by not moving to CLOSED) and try again from a
> > timer.
> 
> Ok.  Makes sense, but I don't think it's necessary. We can treat a
> failure here as if the RST was lost on the wire: we just forget the
> connection, and if the guest doesn't drop it we'll send it another RST
> if it tries to send a packet to the connection we no longer know
> about.

I was wrong about this, and we hit this case in practice for a
migration where the target has a different address (which I've now
managed to test properly).

With my draft fix, after the bind() failure we correctly We tcp_rst()
the imported flow, but because the queue is disabled the RST isn't
actually delivered to the guest.  So, the (target) guest will now send
mid-stream packets which no longer match any flow.  Turns out
flow-less packets which aren't SYN we simply ignore.

That seems wrong, I think we should RST and/or send an ICMP before.
I'm slightly worried it might be one of those cases where that was
how it was originally supposed to work, but the realities of a hostile
internet mean it turns out to be unwise.  Then again, this is guest
facing, so it won't usually be exposed to the whole internet.

> > In practice I've never observed a tcp_send_flag() failure so I'm not
> > sure if that mechanism even works.
> 
> I don't think the mechanism even exists.  I don't think there's any
> timer that does this, and more to the point, if we don't change events
> we basically don't have any record that we already reset the connection.
> 
> > Moving the socket to CLOSED sounds
> > totally okay to me, surely simpler, and probably more robust.
> 
> Ok.  I'll go ahead with that.

I still think this change is correct, and I'm going ahead with it, but
I think we probably want another change on that to RST (or something)
on flowless !SYN packets.

-- 
David Gibson (he or they)	| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you, not the other way
				| around.
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: tcp_rst() complications

[Stefano Brivio]

On Thu, 27 Feb 2025 15:10:32 +1100
David Gibson <david@gibson.dropbear.id.au> wrote:

> On Thu, Feb 27, 2025 at 12:57:15PM +1100, David Gibson wrote:
> > On Wed, Feb 26, 2025 at 11:15:07AM +0100, Stefano Brivio wrote:  
> > > On Wed, 26 Feb 2025 20:05:25 +1100
> > > David Gibson <david@gibson.dropbear.id.au> wrote:
> > >   
> > > > Amongst other things, I spotted some additional complications using
> > > > tcp_rst() in the migration path (some of which might also have
> > > > implications in other contexts).  These might be things we can safely
> > > > ignore, at least for now, but I haven't thought through them enough to
> > > > be sure.
> > > > 
> > > > 1) Sending RST to guest during migration
> > > > 
> > > > The first issue is that tcp_rst() will send an actual RST to the guest
> > > > on the tap interface.  During migration, that means we're sending to
> > > > the guest while it's suspended.  At the very least that means we
> > > > probably have a much higher that usual chance of getting a queue full
> > > > failure writing to the tap interface, which could hit problem (2).
> > > > 
> > > > But, beyond that, with vhost-user that means we're writing to guest
> > > > memory while the guest is suspended.  Kind of the whole point of the
> > > > suspended time is that the guest memory doesn't change during it, so
> > > > I'm not sure what the consequences will be.  
> > > 
> > > If I recall correctly I checked this and something in the vhost-user
> > > code will tell us that the queue is not ready yet, done.  
> > 
> > Ah, yes.  I'd seen those "Got packet, but RX virtqueue is not usable
> > yet" messages, but didn't make the connection.  
> 
> Significantly, of course, that case also still returns 0 from
> tcp_send_flag() so we carry on with changing the state to CLOSED.

Yes, I think we should eventually re-evaluate that 'return 0;' at some
point. On the other hand we're (generally) dealing with an unexpected
situation.

> > I had been concerned that might only happen when we tried to kick the
> > queue, after we'd touched guest memory, but that doesn't appear to be
> > the case.  Ok, now I'm reasonably convinced this is ifne.
> >   
> > > Ideally we want to make sure we queue those, but queue sizes are finite
> > > and I don't think we can guarantee we can pile up 128k RST segments.  
> > 
> > Well that, plus with vhost-user we don't really have anywhere we can
> > queue them other than in guest memory.
> >   
> > > Right now I would check that the functionality is not spectacularly
> > > broken (I looked into that briefly, QEMU didn't crash, guest kept
> > > running, but I didn't check further than that). If we miss a RST too
> > > bad, things will time out eventually.
> > > 
> > > As a second step we could perhaps introduce a post-migration stage and
> > > move calling tcp_rst() to there if the connection is in a given state?
> > >   
> > > > Now, at the moment I
> > > > think all our tcp_rst() calls are either on the source during rollback
> > > > (i.e. we're committed to resuming only on the source) or on the target
> > > > past the point of no return (i.e. we're committed to resuming only on
> > > > the target).  I suspect that means we can get away with it, but I do
> > > > worry this could break something in qeme by violating that assumption.
> > > > 
> > > > 2) tcp_rst() failures
> > > > 
> > > > tcp_rst() can fail if tcp_send_flag() fails.  In this case we *don't*
> > > > change the events to CLOSED.  I _think_ that's a bug: even if we
> > > > weren't able to send the RST to the guest, we've already closed the
> > > > socket so the flow is dead.  Moving to CLOSED state (and then removing
> > > > the flow entirely) should mean that we'll resend an RST if the guest
> > > > attempts to use the flow again later.
> > > > 
> > > > But.. I was worried there might be some subtle reason for not changing
> > > > the event state in that case.  
> > > 
> > > Not very subtle: my original idea was that if we fail to send the RST,
> > > we should note that (by not moving to CLOSED) and try again from a
> > > timer.  
> > 
> > Ok.  Makes sense, but I don't think it's necessary. We can treat a
> > failure here as if the RST was lost on the wire: we just forget the
> > connection, and if the guest doesn't drop it we'll send it another RST
> > if it tries to send a packet to the connection we no longer know
> > about.  
> 
> I was wrong about this, and we hit this case in practice for a
> migration where the target has a different address (which I've now
> managed to test properly).
> 
> With my draft fix, after the bind() failure we correctly We tcp_rst()
> the imported flow, but because the queue is disabled the RST isn't
> actually delivered to the guest.  So, the (target) guest will now send
> mid-stream packets which no longer match any flow.  Turns out
> flow-less packets which aren't SYN we simply ignore.
> 
> That seems wrong, I think we should RST and/or send an ICMP before.
> I'm slightly worried it might be one of those cases where that was
> how it was originally supposed to work, but the realities of a hostile
> internet mean it turns out to be unwise.  Then again, this is guest
> facing, so it won't usually be exposed to the whole internet.

Correct, we should RST on !RST:

  https://datatracker.ietf.org/doc/html/rfc9293#section-3.10.7.1

the consequences of not doing so shouldn't be relevant because
data-less segments can't be expected to be delivered reliably, plus
"stray" RSTs are very commonly dropped by firewalls.

Still yes, we should fix it, it was pretty much an oversight on my side.

> > > In practice I've never observed a tcp_send_flag() failure so I'm not
> > > sure if that mechanism even works.  
> > 
> > I don't think the mechanism even exists.  I don't think there's any
> > timer that does this, and more to the point, if we don't change events
> > we basically don't have any record that we already reset the connection.
> >   
> > > Moving the socket to CLOSED sounds
> > > totally okay to me, surely simpler, and probably more robust.  
> > 
> > Ok.  I'll go ahead with that.  
> 
> I still think this change is correct, and I'm going ahead with it, but
> I think we probably want another change on that to RST (or something)
> on flowless !SYN packets.

Yes, it's RST with sequence number 0 if !ACK, matching sequence number
(needing a separate version of tcp_rst() perhaps?) on ACK.

-- 
Stefano

Re: tcp_rst() complications

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 7330 bytes --]

On Thu, Feb 27, 2025 at 05:26:44AM +0100, Stefano Brivio wrote:
> On Thu, 27 Feb 2025 15:10:32 +1100
> David Gibson <david@gibson.dropbear.id.au> wrote:
> 
> > On Thu, Feb 27, 2025 at 12:57:15PM +1100, David Gibson wrote:
> > > On Wed, Feb 26, 2025 at 11:15:07AM +0100, Stefano Brivio wrote:  
> > > > On Wed, 26 Feb 2025 20:05:25 +1100
> > > > David Gibson <david@gibson.dropbear.id.au> wrote:
> > > >   
> > > > > Amongst other things, I spotted some additional complications using
> > > > > tcp_rst() in the migration path (some of which might also have
> > > > > implications in other contexts).  These might be things we can safely
> > > > > ignore, at least for now, but I haven't thought through them enough to
> > > > > be sure.
> > > > > 
> > > > > 1) Sending RST to guest during migration
> > > > > 
> > > > > The first issue is that tcp_rst() will send an actual RST to the guest
> > > > > on the tap interface.  During migration, that means we're sending to
> > > > > the guest while it's suspended.  At the very least that means we
> > > > > probably have a much higher that usual chance of getting a queue full
> > > > > failure writing to the tap interface, which could hit problem (2).
> > > > > 
> > > > > But, beyond that, with vhost-user that means we're writing to guest
> > > > > memory while the guest is suspended.  Kind of the whole point of the
> > > > > suspended time is that the guest memory doesn't change during it, so
> > > > > I'm not sure what the consequences will be.  
> > > > 
> > > > If I recall correctly I checked this and something in the vhost-user
> > > > code will tell us that the queue is not ready yet, done.  
> > > 
> > > Ah, yes.  I'd seen those "Got packet, but RX virtqueue is not usable
> > > yet" messages, but didn't make the connection.  
> > 
> > Significantly, of course, that case also still returns 0 from
> > tcp_send_flag() so we carry on with changing the state to CLOSED.
> 
> Yes, I think we should eventually re-evaluate that 'return 0;' at some
> point. On the other hand we're (generally) dealing with an unexpected
> situation.

Right.

> > > I had been concerned that might only happen when we tried to kick the
> > > queue, after we'd touched guest memory, but that doesn't appear to be
> > > the case.  Ok, now I'm reasonably convinced this is ifne.
> > >   
> > > > Ideally we want to make sure we queue those, but queue sizes are finite
> > > > and I don't think we can guarantee we can pile up 128k RST segments.  
> > > 
> > > Well that, plus with vhost-user we don't really have anywhere we can
> > > queue them other than in guest memory.
> > >   
> > > > Right now I would check that the functionality is not spectacularly
> > > > broken (I looked into that briefly, QEMU didn't crash, guest kept
> > > > running, but I didn't check further than that). If we miss a RST too
> > > > bad, things will time out eventually.
> > > > 
> > > > As a second step we could perhaps introduce a post-migration stage and
> > > > move calling tcp_rst() to there if the connection is in a given state?
> > > >   
> > > > > Now, at the moment I
> > > > > think all our tcp_rst() calls are either on the source during rollback
> > > > > (i.e. we're committed to resuming only on the source) or on the target
> > > > > past the point of no return (i.e. we're committed to resuming only on
> > > > > the target).  I suspect that means we can get away with it, but I do
> > > > > worry this could break something in qeme by violating that assumption.
> > > > > 
> > > > > 2) tcp_rst() failures
> > > > > 
> > > > > tcp_rst() can fail if tcp_send_flag() fails.  In this case we *don't*
> > > > > change the events to CLOSED.  I _think_ that's a bug: even if we
> > > > > weren't able to send the RST to the guest, we've already closed the
> > > > > socket so the flow is dead.  Moving to CLOSED state (and then removing
> > > > > the flow entirely) should mean that we'll resend an RST if the guest
> > > > > attempts to use the flow again later.
> > > > > 
> > > > > But.. I was worried there might be some subtle reason for not changing
> > > > > the event state in that case.  
> > > > 
> > > > Not very subtle: my original idea was that if we fail to send the RST,
> > > > we should note that (by not moving to CLOSED) and try again from a
> > > > timer.  
> > > 
> > > Ok.  Makes sense, but I don't think it's necessary. We can treat a
> > > failure here as if the RST was lost on the wire: we just forget the
> > > connection, and if the guest doesn't drop it we'll send it another RST
> > > if it tries to send a packet to the connection we no longer know
> > > about.  
> > 
> > I was wrong about this, and we hit this case in practice for a
> > migration where the target has a different address (which I've now
> > managed to test properly).
> > 
> > With my draft fix, after the bind() failure we correctly We tcp_rst()
> > the imported flow, but because the queue is disabled the RST isn't
> > actually delivered to the guest.  So, the (target) guest will now send
> > mid-stream packets which no longer match any flow.  Turns out
> > flow-less packets which aren't SYN we simply ignore.
> > 
> > That seems wrong, I think we should RST and/or send an ICMP before.
> > I'm slightly worried it might be one of those cases where that was
> > how it was originally supposed to work, but the realities of a hostile
> > internet mean it turns out to be unwise.  Then again, this is guest
> > facing, so it won't usually be exposed to the whole internet.
> 
> Correct, we should RST on !RST:
> 
>   https://datatracker.ietf.org/doc/html/rfc9293#section-3.10.7.1
> 
> the consequences of not doing so shouldn't be relevant because
> data-less segments can't be expected to be delivered reliably, plus
> "stray" RSTs are very commonly dropped by firewalls.
> 
> Still yes, we should fix it, it was pretty much an oversight on my side.

Ok, I might try to tackle that tomorrow.

> > > > In practice I've never observed a tcp_send_flag() failure so I'm not
> > > > sure if that mechanism even works.  
> > > 
> > > I don't think the mechanism even exists.  I don't think there's any
> > > timer that does this, and more to the point, if we don't change events
> > > we basically don't have any record that we already reset the connection.
> > >   
> > > > Moving the socket to CLOSED sounds
> > > > totally okay to me, surely simpler, and probably more robust.  
> > > 
> > > Ok.  I'll go ahead with that.  
> > 
> > I still think this change is correct, and I'm going ahead with it, but
> > I think we probably want another change on that to RST (or something)
> > on flowless !SYN packets.
> 
> Yes, it's RST with sequence number 0 if !ACK, matching sequence number
> (needing a separate version of tcp_rst() perhaps?) on ACK.

We'll certainly need a separate version of tcp_rst().  tcp_rst() takes
the connection as a parameter.  Here we're dealing with cases where
there is no existing connection, and that's kind of the point.

-- 
David Gibson (he or they)	| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you, not the other way
				| around.
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]