Re: [PATCH net-next 1/2] ipvs: add assured state for conn templates

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



	Hello,

On Mon, 4 Jun 2018, Simon Horman wrote:

> > +/* States for conn templates: NONE or letters separated with "," */
> > +static const char *ip_vs_ctpl_state_name_table[IP_VS_CTPL_S_LAST] = {
> > +	[IP_VS_CTPL_S_NONE]			= "NONE",
> > +	[IP_VS_CTPL_S_ASSURED]			= "A",
> 
> "A" seems a bit cryptic, why not "ASSURED" ?

	I posted v2 which includes this change.

> I'd slightly prefer if refactoring ip_vs_state_name to not take a state
> parameter was a separate patch. Its a nice cleanup. But it doesn't seem
> related to the rest of what is going on here.

	Done

> > +		if (next_state == IP_VS_SCTP_S_ESTABLISHED) {
> > +			struct ip_vs_conn *ct = cp->control;
> > +
> > +			if (ct && (ct->flags & IP_VS_CONN_F_TEMPLATE) &&
> > +			    !(ct->state & IP_VS_CTPL_S_ASSURED))
> > +				ct->state |= IP_VS_CTPL_S_ASSURED;
> > +		}
> 
> The logic above seems to be replicated several times below.
> Could we have a helper function?

	Done

> >  static int __udp_init(struct netns_ipvs *ipvs, struct ip_vs_proto_data *pd)
> > diff --git a/net/netfilter/ipvs/ip_vs_sync.c b/net/netfilter/ipvs/ip_vs_sync.c
> > index 001501e..24891bd 100644
> > --- a/net/netfilter/ipvs/ip_vs_sync.c
> > +++ b/net/netfilter/ipvs/ip_vs_sync.c
> > @@ -1003,11 +1003,10 @@ static void ip_vs_process_message_v0(struct netns_ipvs *ipvs, const char *buffer
> >  				continue;
> >  			}
> >  		} else {
> > -			/* protocol in templates is not used for state/timeout */
> > -			if (state > 0) {
> > -				IP_VS_DBG(2, "BACKUP v0, Invalid template state %u\n",
> > -					state);
> > -				state = 0;
> > +			if (state >= IP_VS_CTPL_S_LAST) {
> > +				IP_VS_DBG(7, "BACKUP v0, Invalid tpl state %u\n",
> > +					  state);
> 
> Not strictly related to this patch, but should these debug messages
> be rate limited in some way?

	I didn't included any changes to rate limit the
debug messages. It can be done with additional patch, for
all such messages. Not sure, may be we can add some macro
to both rate limit and check debugging level, even a
per-backup macro would be useful, it can also include the
IP address of sender.

Regards

--
Julian Anastasov <ja@xxxxxx>
--
To unsubscribe from this list: send the line "unsubscribe lvs-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Filesystem Devel]     [Linux NFS]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux SCSI]     [X.Org]

  Powered by Linux