On 19 May 2026, at 15:02, Ilya Maximets wrote:
> On 5/18/26 4:00 PM, Eelco Chaudron wrote:
>>
>>
>> On 6 May 2026, at 0:52, Ilya Maximets wrote:
>>
>>> Windows support was deprecated in 3.7, it's time to remove it.
>>>
>>> We already removed the installer and the datapath implementation.
>>> Now removing all the code related to talking to the Windows datapath,
>>> building on Windows, using MSVC as a compiler, and the final bits of
>>> documentation related to Windows-specific behavior of certain features.
>>>
>>> It is possible to split this change into smaller parts, but it will
>>> be a non-trivial amount of work to keep the patches in a working state
>>> in the process. So, just removing everything at once. It would also
>>> be easier to re-apply in case someone wants to resurrect this code or
>>> maintain the Windows support in their own fork.
>>>
>>> A couple mentions of MSVC quirks are left in public headers as the
>>> code is good enough and there is no real need to change the way it
>>> is written at the moment.
>>>
>>> Signed-off-by: Ilya Maximets <[email protected]>
>>
>> This took some time to review, so it was probably also hard to make all
>> these changes ;)
>
> Yeah, it took some time to write for sure. :)
>
>>
>> See some comments inline, but also some general ones here:
>>
>> - faq/contributing.rst still has the below paragraph:
>>
>> The Windows datapath kernel module support, on the other hand, is
>> maintained within the OVS tree, so patches for that can go directly to
>> ovs-dev.
>
> I thought I removed this, but I guess it somehow came back.
> Will clean this up.
>
>>
>> - /faq/general.rst still has the following Windows reference:
>>
>> We've seen
>> Open vSwitch ported to a number of different platforms, including
>> FreeBSD,
>> Windows, and even non-POSIX embedded systems.
>
> I left this one intentionally, because we have definitely seen the Windows
> port.
> It doesn't mean it's supported in the mainline. The same as the mentioned
> non-POSIX embedded systems.
ACK
>>
>> - The below comments still reference to windows, but I guess it should be
>> fine:
>>
>> - vswitchd/bridge.c:2125; "only necessary on Windows"
>
> I left this one because it still seems a little weird to allow slashes in
> names, even if it's not necessary on other platforms. The comment says that
> "it's no great loss elsewhere."
>
>> - include/openvswitch/util.h:146,285; MSVC variadic/init macro comments
>
> Left these ones as it's a header and there is no much point in rewriting these
> macros. But it's good to keep the mentioning of why they were written this
> way in the first place, in case someone wants to re-do them in the future.
>
>> - include/openvswitch/ofp-errors.h:44; "and Windows"
>
> This talks about operating systems in general, so it seems fine to leave as
> is.
>
>> - lib/timeval.c:775; Visual Studio 2013 crash comment
>
> Similar to util.h, I didn't feel like it is necessary to change the code, the
> check seems reasonable, but the comment explains why things are the way they
> are.
Ack on all the above...
>>
>> //Eelco
>>
>> [...]
>>
>>> diff --git a/lib/dpif-netlink.c b/lib/dpif-netlink.c
>>> index f22a87934..65a9c3be5 100644
>>> --- a/lib/dpif-netlink.c
>>> +++ b/lib/dpif-netlink.c
>>
>> [...]
>>
>>> @@ -3555,8 +3249,6 @@ dpif_netlink_ct_timeout_policy_dump_done(struct dpif
>>> *dpif OVS_UNUSED,
>>> free(dump_state);
>>> return err;
>>> }
>>> -#endif
>>> -
>>
>> The diff does not show it, but we should keep one additional
>> newline after the FF.
>
> I guess, you meant "before"? The FF was after the endif and the empty line
> in the original code, AFAICT.
You are right... Guess I was tired from doing so many revies ;)
>>
>> [...]
>>
>>> diff --git a/lib/lockfile.c b/lib/lockfile.c
>>> index 42782d29e..6ecbb307e 100644
>>> --- a/lib/lockfile.c
>>> +++ b/lib/lockfile.c
>>
>> In the struct below, there is still 'HANDLE lock_handle', should this be
>> removed?
>
> Yep.
>
>>
>> struct lockfile {
>> struct hmap_node hmap_node;
>> char *name;
>> dev_t device;
>> ino_t inode;
>> int fd;
>> HANDLE lock_handle;
>> };
>>
>> [...]
>>
>>> diff --git a/lib/poll-loop.c b/lib/poll-loop.c
>>> index 70fabeb8a..cbeb6990b 100644
>>> --- a/lib/poll-loop.c
>>> +++ b/lib/poll-loop.c
>>
>> The below structure and some of the other code in poll-loop.c still have
>> 'wevent' references, I guess they all need to be removed.
>
> Yep. Missed those.
>
>>
>> struct poll_node {
>> struct hmap_node hmap_node;
>> struct pollfd pollfd; /* Events to pass to time_poll(). */
>> HANDLE wevent; /* Events for WaitForMultipleObjects(). */
>> const char *where; /* Where poll_node was created. */
>> };
>>
>> [...]
>>
>>
>>> diff --git a/ovsdb/log.c b/ovsdb/log.c
>>> index 86d6bdc24..6d75427cf 100644
>>> --- a/ovsdb/log.c
>>> +++ b/ovsdb/log.c
>>>
>>
>> [...]
>>
>>> @@ -882,20 +822,12 @@ void
>>> ovsdb_log_replace_abort(struct ovsdb_log *new)
>>> {
>>> if (new) {
>>> - /* Unlink the new file, but only after we close it (because Windows
>>> - * does not allow removing an open file). */
>>> - char *name = xstrdup(new->name);
>>> + /* Unlink and close the new file. */
>>> + unlink(new->name);
>>> ovsdb_log_close(new);
>>> - unlink(name);
>>> - free(name);
>>> }
>>> }
>>>
>>> -void
>>> -ovsdb_log_disable_renaming_open_files(void)
>>
>> This function's declaration still exists in ./ovsdb/log.h.
>
> Ack.
>
>>
>>> -{
>>> - rename_open_files = false;
>>> -}
>>>
>> [...]
>>
>>> diff --git a/utilities/checkpatch.py b/utilities/checkpatch.py
>>> index 9a10b78a9..2f7123586 100755
>>> --- a/utilities/checkpatch.py
>>> +++ b/utilities/checkpatch.py
>>> @@ -1059,7 +1059,7 @@ def ovs_checkpatch_parse(text, filename, author=None,
>>> committer=None):
>>> continue
>>>
>>> # Skip files which have /datapath in them, since they are
>>> - # linux or windows coding standards
>>> + # linux coding standards
>>
>> Should we add a dot (.) while we are at it?
>
> Sure.
>
> I suppose, reviewing the diff here is much easier than reviewing a v2.
> I can fold the following changes into appropriate patches while applying
> the set, if they look good to you:
Thanks for the diff, this definitely made thinks a lot easier ;)
With the diff applied,
Acked-by: Eelco Chaudron <[email protected]>
> diff --git a/Documentation/faq/contributing.rst
> b/Documentation/faq/contributing.rst
> index 9f7ea121d..1790a3c20 100644
> --- a/Documentation/faq/contributing.rst
> +++ b/Documentation/faq/contributing.rst
> @@ -65,9 +65,6 @@ Q: How do I add support for a new field or header?
> without it. If you implement kernel module support for Linux, then the
> Linux kernel "netdev" mailing list is the place to submit that support
> first; please read up on the Linux kernel development process separately.
> - The Windows datapath kernel module support, on the other hand, is
> - maintained within the OVS tree, so patches for that can go directly to
> - ovs-dev.
>
> Q: How do I add support for a new OpenFlow action?
>
> diff --git a/Documentation/faq/releases.rst b/Documentation/faq/releases.rst
> index 1f7313f13..b62bb4ed5 100644
> --- a/Documentation/faq/releases.rst
> +++ b/Documentation/faq/releases.rst
> @@ -50,7 +50,7 @@ Q: What Linux kernel versions does each Open vSwitch
> release work with?
> Q: Does Open vSwitch support running on Windows (Hyper-V)?
>
> A: Support for the Windows datapath, a.k.a. Hyper-V, was deprecated
> starting
> - with Open vSwitch 3.7 and the source code was completely removed from the
> + with Open vSwitch 3.7, and the source code was completely removed from the
> Open vSwitch source tree in the next release.
>
> Q: Are all features available with all datapaths?
> diff --git a/lib/dpif-netlink.c b/lib/dpif-netlink.c
> index 65a9c3be5..2bf7e7cf2 100644
> --- a/lib/dpif-netlink.c
> +++ b/lib/dpif-netlink.c
> @@ -3249,6 +3249,7 @@ dpif_netlink_ct_timeout_policy_dump_done(struct dpif
> *dpif OVS_UNUSED,
> free(dump_state);
> return err;
> }
> +
>
> /* Meters */
>
> diff --git a/lib/lockfile.c b/lib/lockfile.c
> index 6ecbb307e..3ad5f0a5d 100644
> --- a/lib/lockfile.c
> +++ b/lib/lockfile.c
> @@ -44,7 +44,6 @@ struct lockfile {
> dev_t device;
> ino_t inode;
> int fd;
> - HANDLE lock_handle;
> };
>
> /* Lock table.
> diff --git a/lib/poll-loop.c b/lib/poll-loop.c
> index cbeb6990b..a5fc4baf0 100644
> --- a/lib/poll-loop.c
> +++ b/lib/poll-loop.c
> @@ -41,7 +41,6 @@ COVERAGE_DEFINE(poll_zero_timeout);
> struct poll_node {
> struct hmap_node hmap_node;
> struct pollfd pollfd; /* Events to pass to time_poll(). */
> - HANDLE wevent; /* Events for WaitForMultipleObjects(). */
> const char *where; /* Where poll_node was created. */
> };
>
> @@ -57,20 +56,15 @@ struct poll_loop {
>
> static struct poll_loop *poll_loop(void);
>
> -/* Look up the node with same fd or wevent. */
> +/* Look up the node with same fd. */
> static struct poll_node *
> -find_poll_node(struct poll_loop *loop, int fd, HANDLE wevent)
> +find_poll_node(struct poll_loop *loop, int fd)
> {
> struct poll_node *node;
>
> - /* Both 'fd' and 'wevent' cannot be set. */
> - ovs_assert(!fd != !wevent);
> -
> - HMAP_FOR_EACH_WITH_HASH (node, hmap_node,
> - hash_2words(fd, (uint32_t)wevent),
> + HMAP_FOR_EACH_WITH_HASH (node, hmap_node, hash_int(fd, 0),
> &loop->poll_nodes) {
> - if ((fd && node->pollfd.fd == fd)
> - || (wevent && node->wevent == wevent)) {
> + if (node->pollfd.fd == fd) {
> return node;
> }
> }
> @@ -90,27 +84,22 @@ find_poll_node(struct poll_loop *loop, int fd, HANDLE
> wevent)
> * automatically provide the caller's source file and line number for
> * 'where'.) */
> static void
> -poll_create_node(int fd, HANDLE wevent, short int events, const char *where)
> +poll_create_node(int fd, short int events, const char *where)
> {
> struct poll_loop *loop = poll_loop();
> struct poll_node *node;
>
> COVERAGE_INC(poll_create_node);
>
> - /* Both 'fd' and 'wevent' cannot be set. */
> - ovs_assert(!fd != !wevent);
> -
> /* Check for duplicate. If found, "or" the events. */
> - node = find_poll_node(loop, fd, wevent);
> + node = find_poll_node(loop, fd);
> if (node) {
> node->pollfd.events |= events;
> } else {
> node = xzalloc(sizeof *node);
> - hmap_insert(&loop->poll_nodes, &node->hmap_node,
> - hash_2words(fd, (uint32_t)wevent));
> + hmap_insert(&loop->poll_nodes, &node->hmap_node, hash_int(fd, 0));
> node->pollfd.fd = fd;
> node->pollfd.events = events;
> - node->wevent = wevent;
> node->where = where;
> }
> }
> @@ -129,7 +118,7 @@ poll_create_node(int fd, HANDLE wevent, short int events,
> const char *where)
> void
> poll_fd_wait_at(int fd, short int events, const char *where)
> {
> - poll_create_node(fd, 0, events, where);
> + poll_create_node(fd, events, where);
> }
>
> /* Causes the following call to poll_block() to block for no more than 'msec'
> @@ -280,7 +269,6 @@ poll_block(void)
> struct poll_loop *loop = poll_loop();
> struct poll_node *node;
> struct pollfd *pollfds;
> - HANDLE *wevents = NULL;
> int elapsed;
> int retval;
> int i;
> @@ -302,7 +290,7 @@ poll_block(void)
> pollfds[i++] = node->pollfd;
> }
>
> - retval = time_poll(pollfds, hmap_count(&loop->poll_nodes), wevents,
> + retval = time_poll(pollfds, hmap_count(&loop->poll_nodes),
> loop->timeout_when, &elapsed);
> if (retval < 0) {
> static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
> @@ -323,7 +311,6 @@ poll_block(void)
> loop->timeout_when = LLONG_MAX;
> loop->timeout_where = NULL;
> free(pollfds);
> - free(wevents);
>
> /* Handle any pending signals before doing anything else. */
> fatal_signal_run();
> diff --git a/lib/timeval.c b/lib/timeval.c
> index e1fc35b27..5d7e30b2a 100644
> --- a/lib/timeval.c
> +++ b/lib/timeval.c
> @@ -282,7 +282,7 @@ time_alarm(unsigned int secs)
> *
> * Stores the number of milliseconds elapsed during poll in '*elapsed'. */
> int
> -time_poll(struct pollfd *pollfds, int n_pollfds, HANDLE *handles OVS_UNUSED,
> +time_poll(struct pollfd *pollfds, int n_pollfds,
> long long int timeout_when, int *elapsed)
> {
> long long int *last_wakeup = last_wakeup_get();
> diff --git a/lib/timeval.h b/lib/timeval.h
> index 422db8c63..9d1ebda29 100644
> --- a/lib/timeval.h
> +++ b/lib/timeval.h
> @@ -54,7 +54,7 @@ long long int time_wall_usec(void);
> void time_timespec(struct timespec *);
> void time_wall_timespec(struct timespec *);
> void time_alarm(unsigned int secs);
> -int time_poll(struct pollfd *, int n_pollfds, HANDLE *handles,
> +int time_poll(struct pollfd *, int n_pollfds,
> long long int timeout_when, int *elapsed);
>
> long long int timespec_to_msec(const struct timespec *);
> diff --git a/lib/util.h b/lib/util.h
> index e8884f4b2..03f0c151f 100644
> --- a/lib/util.h
> +++ b/lib/util.h
> @@ -121,8 +121,6 @@ ovs_prefetch_range(const void *start, size_t size)
> #define PRIxSIZE "zx"
> #define PRIXSIZE "zX"
>
> -typedef uint32_t HANDLE;
> -
> #ifdef __cplusplus
> extern "C" {
> #endif
> diff --git a/ovsdb/log.h b/ovsdb/log.h
> index 63e5681a0..735186101 100644
> --- a/ovsdb/log.h
> +++ b/ovsdb/log.h
> @@ -97,7 +97,4 @@ struct ovsdb_error *ovsdb_log_replace_commit(struct
> ovsdb_log *old,
> OVS_WARN_UNUSED_RESULT;
> void ovsdb_log_replace_abort(struct ovsdb_log *new);
>
> -/* For testing. */
> -void ovsdb_log_disable_renaming_open_files(void);
> -
> #endif /* ovsdb/log.h */
> diff --git a/utilities/checkpatch.py b/utilities/checkpatch.py
> index 2f7123586..e46c28967 100755
> --- a/utilities/checkpatch.py
> +++ b/utilities/checkpatch.py
> @@ -1059,7 +1059,7 @@ def ovs_checkpatch_parse(text, filename, author=None,
> committer=None):
> continue
>
> # Skip files which have /datapath in them, since they are
> - # linux coding standards
> + # linux coding standards.
> if current_file.startswith('datapath'):
> continue
> if current_file.startswith('include/linux'):
> ---
>
> Best regards, Ilya Maximets.
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
