Support IP Type of Service / Traffic Class: --ip-tos

[orgads]

Add --ip-tos option to the command line tool for setting TOS for IPv4 or Traffic Class for IPv6.

[orgads]

I don't know what to do with the remaining tests that fail. Can you please advise?

[bagder]

Why is a new libcurl option needed? Can't CURLOPT_SOCKOPTFUNCTION already do this?

[bagder]

This is a IPv4-only feature. What happens for IPv6 sockets and how is a user supposed to know when this works or not?

Can you please explain your use case for this?

[orgads]

Why is a new libcurl option needed? Can't CURLOPT_SOCKOPTFUNCTION already do this?

We use it with the command-line interface. And if I understand correctly, CURLOPT_SOCKOPTFUNCTION is for user applications, not for internal use by curl, right?

[orgads]

This is a IPv4-only feature. What happens for IPv6 sockets and how is a user supposed to know when this works or not?

Can you please explain your use case for this?

We need it for applying QoS for IP packets. All the other parts of our application support this, and we've been patching curl for long time to add support. Now we'd like to upstream our patches (2 more will follow :)).

I noticed that another used proposed a similar change in #7762, but it was abandoned. I took the user-friendly names from that PR.

[bagder]

Why is a new libcurl option needed? Can't CURLOPT_SOCKOPTFUNCTION already do this?

We use it with the command-line interface. And if I understand correctly, CURLOPT_SOCKOPTFUNCTION is for user applications, not for internal use by curl, right?

You add CURLOPT_TYPE_OF_SERVICE in this PR. It is not for the command line tool, that's a new option for libcurl and I wonder why you need it.

[orgads]

Why is a new libcurl option needed? Can't CURLOPT_SOCKOPTFUNCTION already do this?

We use it with the command-line interface. And if I understand correctly, CURLOPT_SOCKOPTFUNCTION is for user applications, not for internal use by curl, right?

You add CURLOPT_TYPE_OF_SERVICE in this PR. It is not for the command line tool, that's a new option for libcurl and I wonder why you need it.

It is used in tool_operate.c. If I understand the relations correctly, this sets the library options according to the command-line arguments. So do you suggest using CURLOPT_SOCKOPTFUNCTION there? I don't see any other option that uses it there, why should this one be different?

[bagder]

So do you suggest using CURLOPT_SOCKOPTFUNCTION there?

I do. I don't see why libcurl needs another option when you can already accomplish the same thing using an existing option.

[orgads]

So do you suggest using CURLOPT_SOCKOPTFUNCTION there?

I do. I don't see why libcurl needs another option when you can already accomplish the same thing using an existing option.

The next feature I plan to add is setting VLAN priority, which is another setsockopt (setsockopt(sockfd, SOL_SOCKET, SO_PRIORITY, (char*)&priority, sizeof(priority))). How can I support both options? There is only one SOCKOPTFUNCTION entry.

[bagder]

There is only one SOCKOPTFUNCTION entry.

So do both in the same callback?

[orgads]

There is only one SOCKOPTFUNCTION entry.

So do both in the same callback?

Done. Is this what you meant?

[orgads]

@bagder Is this good to go, now that 8.8 was released?

[bagder]

We cannot merge this anyway until the feature window opens on June 1st if everything goes well.

[orgads]

Is the msys2 with debug flaky? I couldn't find anything in the log that looks related to my changes.

[vszakats]

@orgads: Yes, the MSYS2 mingw-w64 ones sometimes hang in tests. I'm tracking the problem here: #13599 (comment). Could not yet figure out which test is prone to hanging though.

[orgads]

Is there a way to retrigger only the failing job?

[vszakats]

@orgads: I don't think that is made possible by GitHub. Tried to fix the obscure issue that prevented caching the failing download, here: 1d63e33.

Restarted that job manually — if it fails again, please rebase on master.

[orgads]

Passed, thanks!

[orgads]

All passed. I have one last PR, but it depends on this one, so I'll push it after this is merged.

Thanks you all for your constructive review!