Skip to content

Add switches to the host init script to toggle legacy behaviour - #3139

Open
dxapd wants to merge 1 commit into
google:mainfrom
dxapd:nftables-initscript-switch
Open

Add switches to the host init script to toggle legacy behaviour#3139
dxapd wants to merge 1 commit into
google:mainfrom
dxapd:nftables-initscript-switch

Conversation

@dxapd

@dxapd dxapd commented Sep 3, 2026

Copy link
Copy Markdown
Collaborator

add two new flags for toggling static resource initialization as well as running cvdalloc's init (which is to be implemented in the final PR in this series).

due to the way nftables works, cvdalloc also needs to do a small amount of install-time init (namely, creating its tables and chains). that code is better off living inside the cvdalloc binary, so we have the init script, which is our one hook point into install and uninstall, invoke it at exactly those times.

this change, as is, should be a no-op. the code enclosed within the if [ "${allocate_static_resources}" = "1" ]; then branches is the code we hope to delete one day, but until that day it will remain.

@dxapd
dxapd marked this pull request as ready for review September 3, 2026 23:56
@dxapd
dxapd requested review from 3405691582 and Databean September 4, 2026 00:01
@3405691582

Copy link
Copy Markdown
Collaborator

due to the way nftables works, cvdalloc also needs to do a small amount of install-time init (namely, creating its tables and chains).

Can we do this without having to do this at init? Can we interrogate nftables and see if our tables and chains exist, and if not, install them then?

@dxapd

dxapd commented Sep 8, 2026

Copy link
Copy Markdown
Collaborator Author

due to the way nftables works, cvdalloc also needs to do a small amount of install-time init (namely, creating its tables and chains).

Can we do this without having to do this at init? Can we interrogate nftables and see if our tables and chains exist, and if not, install them then?

that can theoretically be done for setup, but for teardown, we'd need to do something similar to refcounting to reason out that it's safe to tear down all of the resources. i don't think that machinery is worth it in this case, given that the service stopping is a pretty natural hook to clean up the tables and chains. and then (IMO) once we have that teardown path, it's nice and simple to just expand it a bit to have a simple setup routine to go with it.

which i guess boils down to - it's possible, but imo this is simpler and we're handed the lifecycle for free by debian itself

@3405691582

Copy link
Copy Markdown
Collaborator

due to the way nftables works, cvdalloc also needs to do a small amount of install-time init (namely, creating its tables and chains).

Can we do this without having to do this at init? Can we interrogate nftables and see if our tables and chains exist, and if not, install them then?

that can theoretically be done for setup, but for teardown, we'd need to do something similar to refcounting to reason out that it's safe to tear down all of the resources. i don't think that machinery is worth it in this case, given that the service stopping is a pretty natural hook to clean up the tables and chains. and then (IMO) once we have that teardown path, it's nice and simple to just expand it a bit to have a simple setup routine to go with it.

which i guess boils down to - it's possible, but imo this is simpler and we're handed the lifecycle for free by debian itself

Ideally, we shouldn't need the init script at all. I don't know if we need to fully refcount; for example, if I remember correctly the existing implementation checks the bridge interface's associated addresses and only tears the bridge down if it's the last. I might not be fully having all the context at hand with nftables though.

If that's a bigger lift, nonetheless we should at least init dynamically and teardown statically and revisit stateless teardown later.

@dxapd

dxapd commented Sep 9, 2026

Copy link
Copy Markdown
Collaborator Author

i don't think i agree about this case; there is a handful of things that we will need to keep the init script around for (unless there is a plan to move off of the debian package that i didn't know about), so these hook points will always be there for the taking

and even beyond that, it's just very simple to reason about (imo) "run the setup routine when the program is installed, then run the teardown routine when it is removed" vs. depending on each instance at runtime to have some machinery to construct its own environment statelessly.

for the teardown case, there's two ways I'm familiar of:

  • nftables doesn't have any direct way to count rules, so the way around that would be to get it to output the chain in json and count the amount of rules in there. if only the current instance's rule, then clean up.
  • on the other hand, to do it in a refcounting manner, yeah you'd need to track that count state somewhere.

for the init case, sure, it's possible to make that idempotent. just make the expected environment exist at init time. but is that needed, given that we have the ideal hook points available already, and we're already using one of them? dunno, this one is less defensible to me than my reasoning for teardown above, but i do think it makes it very easy to understand how the assumptions are set up. either cvdalloc is installed or it isn't. unless of course there are pitfalls to the way debian handles it that i don't know about.

that aside, i do feel more strongly about teardown being run at uninstall time, just as a matter of hygiene. we're uninstalling the application, so we make sure we've cleaned house. i'd want that to be there regardless, i think.

so it's not really about how much of a lift it is; i just think it's cleaner, as well as easier to understand what assumptions the allocator is operating under like this. but perhaps there's an angle to this i haven't thought about yet.

@Databean

Databean commented Sep 9, 2026

Copy link
Copy Markdown
Member

Adding to @3405691582's points, part of the motivation of cvdalloc in the first place was to make the debian packages less intrusive to have installed, to make them more palatable for debian upstream. The average public user that installs the packages may be interested in launching a Cuttlefish device on occasion, but is less likely to be launching one every day. It's nicer to them to have fewer runtime system resources allocated while they're not actively engaging with our software.

@dxapd

dxapd commented Sep 9, 2026

Copy link
Copy Markdown
Collaborator Author

Adding to @3405691582's points, part of the motivation of cvdalloc in the first place was to make the debian packages less intrusive to have installed, to make them more palatable for debian upstream. The average public user that installs the packages may be interested in launching a Cuttlefish device on occasion, but is less likely to be launching one every day. It's nicer to them to have fewer runtime system resources allocated while they're not actively engaging with our software.

yes, i understand that, but it's not clear to me what the line between intrusive and standard is, especially since a single nftables table won't clutter any command outputs like static resources does.

that said! all of this discussion got me thinking, and I'm quite optimistic that making a fresh nftables table for every instance might actually be super clean and let us delete setup and teardown modes. i think that would resolve all of our concerns simultaneously since there would be no special running modes, no state to leak during uninstall, and the logic would also be dead simple... I'll look into whether or not it's possible

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants