> +enum admin_port_state {
> + RSTP_ADMIN_BRIDGE_PORT_STATE_DISABLED = 0,
> + RSTP_ADMIN_BRIDGE_PORT_STATE_ENABLED = 1
> +};
> +
> +enum oper_p2p_mac_state {
> + RSTP_OPER_P2P_MAC_STATE_DISABLED = 0,
> + RSTP_OPER_P2P_MAC_STATE_ENABLED = 1
> +};
> +
> +/* State enumerations for state machines defined in rstp-state-machines.c */
> +enum port_receive_state_machine {
> + PORT_RECEIVE_SM_INIT,
> + PORT_RECEIVE_SM_DISCARD_EXEC,
> + PORT_RECEIVE_SM_DISCARD,
> + PORT_RECEIVE_SM_RECEIVE_EXEC,
> + PORT_RECEIVE_SM_RECEIVE
> +};
It seems that apart from the “_INIT” states, these are only used by the
rstp-state-machines.c.
Consider defining them with “_INIT = 0,” moving them to rstp-state-machines.c.
and maybe using a
memset(,0,) in rstp_initialize_port() to minimize the API header file(s).
Those states are also used outside rstp-state-machines.c .
> diff --git a/lib/rstp.h b/lib/rstp.h
> new file mode 100644
> index 0000000..d3eb6a6
> --- /dev/null
> +++ b/lib/rstp.h
> @@ -0,0 +1,184 @@
> +/*
> + * Copyright (c) 2011-2014 M3S, Srl - Italy
> + *
> + * Licensed under the Apache License, Version 2.0 (the "License");
> + * you may not use this file except in compliance with the License.
> + * You may obtain a copy of the License at:
> + *
> + * http://www.apache.org/licenses/LICENSE-2.0
> + *
> + * Unless required by applicable law or agreed to in writing, software
> + * distributed under the License is distributed on an "AS IS" BASIS,
> + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
> + * See the License for the specific language governing permissions and
> + * limitations under the License.
> + */
> +
> +/*
> + * Rapid Spanning Tree Protocol (IEEE 802.1D-2004) public interface (header
> + * file).
> + *
> + * Authors:
> + * Martino Fornasa <m...@fornasa.it>
> + * Daniele Venturino <daniele.ventur...@m3s.it>
> + *
> + * References to IEEE 802.1D-2004 standard are enclosed in square brackets.
> + * E.g. [17.3], [Table 17-1], etc.
> + *
> + */
> +
> +#ifndef RSTP_H
> +#define RSTP_H 1
> +
> +#include <stdint.h>
> +#include <stdbool.h>
> +#include "compiler.h"
> +#include “util.h”
> +
Use enums instead of defines for the constants here.
> +
> +#define RSTP_DEFAULT_BRIDGE_MAX_AGE 20
> +#define RSTP_MIN_BRIDGE_MAX_AGE 6
> +#define RSTP_MAX_BRIDGE_MAX_AGE 40
> +
> +#define RSTP_DEFAULT_BRIDGE_FORWARD_DELAY 15
> +#define RSTP_MIN_BRIDGE_FORWARD_DELAY 4
> +#define RSTP_MAX_BRIDGE_FORWARD_DELAY 3, but
These are not different values of the same variable, but bounds. It seems to me
more clear to leave that as defines.
> +static void rstp_send_bpdu(struct rstp_port *, const void *, size_t)
> + OVS_REQUIRES(mutex);
> +
Symbol ‘mutex’ is not known here. If you mean the ‘mutex’ defined in rstp.c,
please rename it to
‘rstp_mutex’ and declare it in rstp.h.
I'm not sure I understood the purpouse of OVS_REQUIRES(mutex).
I have some internal functions called only with a mutex already locked. Does
OVS_REQUIRES indicate the necessity of the mutex being locked or something else?
I’m sending an updated version of this patch (v4).
Daniele
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
