Thanks Jon! This patch is good for me. Please add my "Acked-by: Ying Xue <[email protected]>" flag if you want.
Thanks, Ying On 07/07/2018 01:21 AM, Jon Maloy wrote: > In some virtual environments we observe a significant higher number of > packet reordering and delays than we have been used to traditionally. > > This makes it necessary with stricter checks on incoming link protocol > messages' session number, which until now only has been validated for > RESET messages. > > Since the other two message types, ACTIVATE and STATE messages also > carry this number, it is easy to extend the validation check to those > messages. > > We also introduce a flag indicating if a link has a valid peer session > number or not. This eliminates the mixing of 32- and 16-bit arithmethics > we are currently using to achieve this. > > --- > v3: Realized that protocol session and sequence number also must be > checked in function tipc_node_check_state(), so introduced new > validation function. > v4: Corrected check of STATE sequence number > > Signed-off-by: Jon Maloy <[email protected]> > --- > net/tipc/link.c | 68 > +++++++++++++++++++++++++++++++++++++++------------------ > net/tipc/link.h | 1 + > net/tipc/node.c | 5 ++++- > 3 files changed, 52 insertions(+), 22 deletions(-) > > diff --git a/net/tipc/link.c b/net/tipc/link.c > index e407385..6443153 100644 > --- a/net/tipc/link.c > +++ b/net/tipc/link.c > @@ -128,8 +128,8 @@ struct tipc_link { > struct net *net; > > /* Management and link supervision data */ > - u32 peer_session; > - u32 session; > + u16 peer_session; > + u16 session; > u16 snd_nxt_state; > u16 rcv_nxt_state; > u32 peer_bearer_id; > @@ -138,6 +138,7 @@ struct tipc_link { > u32 abort_limit; > u32 state; > u16 peer_caps; > + bool in_session; > bool active; > u32 silent_intv_cnt; > char if_name[TIPC_MAX_IF_NAME]; > @@ -216,11 +217,6 @@ enum { > */ > #define TIPC_NACK_INTV (TIPC_MIN_LINK_WIN * 2) > > -/* Wildcard value for link session numbers. When it is known that > - * peer endpoint is down, any session number must be accepted. > - */ > -#define ANY_SESSION 0x10000 > - > /* Link FSM states: > */ > enum { > @@ -473,7 +469,7 @@ bool tipc_link_create(struct net *net, char *if_name, int > bearer_id, > l->addr = peer; > l->peer_caps = peer_caps; > l->net = net; > - l->peer_session = ANY_SESSION; > + l->in_session = false; > l->bearer_id = bearer_id; > l->tolerance = tolerance; > l->net_plane = net_plane; > @@ -842,7 +838,7 @@ void link_prepare_wakeup(struct tipc_link *l) > > void tipc_link_reset(struct tipc_link *l) > { > - l->peer_session = ANY_SESSION; > + l->in_session = false; > l->session++; > l->mtu = l->advertised_mtu; > __skb_queue_purge(&l->transmq); > @@ -1450,6 +1446,44 @@ void tipc_link_tnl_prepare(struct tipc_link *l, struct > tipc_link *tnl, > } > } > > +/* tipc_link_validate_msg(): validate message against current link state > + * Returns true if message should be accepted, otherwise false > + */ > +bool tipc_link_validate_msg(struct tipc_link *l, struct tipc_msg *hdr) > +{ > + u16 curr_session = l->peer_session; > + u16 session = msg_session(hdr); > + int mtyp = msg_type(hdr); > + > + if (msg_user(hdr) != LINK_PROTOCOL) > + return true; > + > + switch (mtyp) { > + case RESET_MSG: > + if (!l->in_session) > + return true; > + /* Accept only RESET with new session number */ > + return more(session, curr_session); > + case ACTIVATE_MSG: > + if (!l->in_session) > + return true; > + /* Accept only ACTIVATE with new or current session number */ > + return !less(session, curr_session); > + case STATE_MSG: > + /* Accept only STATE with current session number */ > + if (!l->in_session) > + return false; > + if (session != curr_session) > + return false; > + if (!(l->peer_caps & TIPC_LINK_PROTO_SEQNO)) > + return true; > + /* Accept only STATE with new sequence number */ > + return !less(msg_seqno(hdr), l->rcv_nxt_state); > + default: > + return false; > + } > +} > + > /* tipc_link_proto_rcv(): receive link level protocol message : > * Note that network plane id propagates through the network, and may > * change at any time. The node with lowest numerical id determines > @@ -1483,17 +1517,12 @@ static int tipc_link_proto_rcv(struct tipc_link *l, > struct sk_buff *skb, > hdr = buf_msg(skb); > data = msg_data(hdr); > > + if (!tipc_link_validate_msg(l, hdr)) > + goto exit; > + > switch (mtyp) { > case RESET_MSG: > - > - /* Ignore duplicate RESET with old session number */ > - if ((less_eq(msg_session(hdr), l->peer_session)) && > - (l->peer_session != ANY_SESSION)) > - break; > - /* fall thru' */ > - > case ACTIVATE_MSG: > - > /* Complete own link name with peer's interface name */ > if_name = strrchr(l->name, ':') + 1; > if (sizeof(l->name) - (if_name - l->name) <= TIPC_MAX_IF_NAME) > @@ -1521,16 +1550,13 @@ static int tipc_link_proto_rcv(struct tipc_link *l, > struct sk_buff *skb, > rc = TIPC_LINK_UP_EVT; > > l->peer_session = msg_session(hdr); > + l->in_session = true; > l->peer_bearer_id = msg_bearer_id(hdr); > if (l->mtu > msg_max_pkt(hdr)) > l->mtu = msg_max_pkt(hdr); > break; > > case STATE_MSG: > - > - if (l->peer_caps & TIPC_LINK_PROTO_SEQNO && > - less(msg_seqno(hdr), l->rcv_nxt_state)) > - break; > l->rcv_nxt_state = msg_seqno(hdr) + 1; > > /* Update own tolerance if peer indicates a non-zero value */ > diff --git a/net/tipc/link.h b/net/tipc/link.h > index ec59348..7900dae 100644 > --- a/net/tipc/link.h > +++ b/net/tipc/link.h > @@ -110,6 +110,7 @@ char *tipc_link_name(struct tipc_link *l); > char tipc_link_plane(struct tipc_link *l); > int tipc_link_prio(struct tipc_link *l); > int tipc_link_window(struct tipc_link *l); > +bool tipc_link_validate_msg(struct tipc_link *l, struct tipc_msg *hdr); > unsigned long tipc_link_tolerance(struct tipc_link *l); > void tipc_link_set_tolerance(struct tipc_link *l, u32 tol, > struct sk_buff_head *xmitq); > diff --git a/net/tipc/node.c b/net/tipc/node.c > index cfdbaf4..4557a78 100644 > --- a/net/tipc/node.c > +++ b/net/tipc/node.c > @@ -1533,7 +1533,7 @@ static void tipc_node_bc_rcv(struct net *net, struct > sk_buff *skb, int bearer_id > * tipc_node_check_state - check and if necessary update node state > * @skb: TIPC packet > * @bearer_id: identity of bearer delivering the packet > - * Returns true if state is ok, otherwise consumes buffer and returns false > + * Returns true if state and msg are ok, otherwise false > */ > static bool tipc_node_check_state(struct tipc_node *n, struct sk_buff *skb, > int bearer_id, struct sk_buff_head *xmitq) > @@ -1567,6 +1567,9 @@ static bool tipc_node_check_state(struct tipc_node *n, > struct sk_buff *skb, > } > } > > + if (!tipc_link_validate_msg(l, hdr)) > + return false; > + > /* Check and update node accesibility if applicable */ > if (state == SELF_UP_PEER_COMING) { > if (!tipc_link_is_up(l)) > ------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot _______________________________________________ tipc-discussion mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/tipc-discussion
