[PATCH 0/6] llc2: Simplify llc_station

[PATCH 0/6] llc2: Simplify llc_station

[Ben Hutchings]

[-- Attachment #1: Type: text/plain, Size: 742 bytes --]

There seem to have been some grand plans for llc_station, but as they
haven't been fulfilled it's just unnecessarily complicated.

Ben.

Ben Hutchings (6):
  llc2: Remove pointless indirection through llc_stat_state_trans_end
  llc2: Remove dead code for state machine
  llc2: Collapse the station event receive path
  llc2: Remove the station send queue
  llc2: Remove explicit indexing of state action arrays
  llc2: Collapse remainder of state machine into simple if-else
    if-statement

 net/llc/llc_station.c |  600 +------------------------------------------------
 1 file changed, 9 insertions(+), 591 deletions(-)


-- 
Ben Hutchings
Experience is what causes a person to make new mistakes instead of old ones.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

[PATCH net-next 1/6] llc2: Remove pointless indirection through llc_stat_state_trans_end

[Ben Hutchings]

[-- Attachment #1: Type: text/plain, Size: 2131 bytes --]

Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
---
 net/llc/llc_station.c |   16 ++++------------
 1 file changed, 4 insertions(+), 12 deletions(-)

diff --git a/net/llc/llc_station.c b/net/llc/llc_station.c
index b2f2bac..e16c1b9 100644
--- a/net/llc/llc_station.c
+++ b/net/llc/llc_station.c
@@ -331,14 +331,6 @@ static int llc_station_ac_report_status(struct sk_buff *skb)
 	return 0;
 }
 
-/* COMMON STATION STATE transitions */
-
-/* dummy last-transition indicator; common to all state transition groups
- * last entry for this state
- * all members are zeros, .bss zeroes it
- */
-static struct llc_station_state_trans llc_stat_state_trans_end;
-
 /* DOWN STATE transitions */
 
 /* state transition for LLC_STATION_EV_ENABLE_WITH_DUP_ADDR_CHECK event */
@@ -372,7 +364,7 @@ static struct llc_station_state_trans llc_stat_down_state_trans_2 = {
 static struct llc_station_state_trans *llc_stat_dwn_state_trans[] = {
 	[0] = &llc_stat_down_state_trans_1,
 	[1] = &llc_stat_down_state_trans_2,
-	[2] = &llc_stat_state_trans_end,
+	[2] = NULL,
 };
 
 /* UP STATE transitions */
@@ -417,7 +409,7 @@ static struct llc_station_state_trans *llc_stat_up_state_trans [] = {
 	[0] = &llc_stat_up_state_trans_1,
 	[1] = &llc_stat_up_state_trans_2,
 	[2] = &llc_stat_up_state_trans_3,
-	[3] = &llc_stat_state_trans_end,
+	[3] = NULL,
 };
 
 /* DUP ADDR CHK STATE transitions */
@@ -512,7 +504,7 @@ static struct llc_station_state_trans *llc_stat_dupaddr_state_trans[] = {
 	[3] = &llc_stat_dupaddr_state_trans_1,	/* Receive frame */
 	[4] = &llc_stat_dupaddr_state_trans_2,
 	[5] = &llc_stat_dupaddr_state_trans_3,
-	[6] = &llc_stat_state_trans_end,
+	[6] = NULL,
 };
 
 static struct llc_station_state
@@ -568,7 +560,7 @@ static struct llc_station_state_trans *
 	struct llc_station_state *curr_state =
 				&llc_station_state_table[llc_main_station.state - 1];
 
-	for (next_trans = curr_state->transitions; next_trans[i]->ev; i++)
+	for (next_trans = curr_state->transitions; next_trans[i]; i++)
 		if (!next_trans[i]->ev(skb)) {
 			rc = next_trans[i];
 			break;



[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

[PATCH net-next 2/6] llc2: Remove dead code for state machine

[Ben Hutchings]

[-- Attachment #1: Type: text/plain, Size: 17967 bytes --]

The initial state is UP and there is no way to enter the other states
as the required event type is never generated.  Delete all states,
event types, and other dead code.  The only thing left is handling
of the XID and TEST commands.

Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
---
 net/llc/llc_station.c |  404 ++-----------------------------------------------
 1 file changed, 9 insertions(+), 395 deletions(-)

diff --git a/net/llc/llc_station.c b/net/llc/llc_station.c
index e16c1b9..917d700 100644
--- a/net/llc/llc_station.c
+++ b/net/llc/llc_station.c
@@ -30,19 +30,12 @@
  *
  * SAP and connection resource manager, one per adapter.
  *
- * @state: state of station
- * @xid_r_count: XID response PDU counter
  * @mac_sa: MAC source address
  * @sap_list: list of related SAPs
  * @ev_q: events entering state mach.
  * @mac_pdu_q: PDUs ready to send to MAC
  */
 struct llc_station {
-	u8			    state;
-	u8			    xid_r_count;
-	struct timer_list	    ack_timer;
-	u8			    retry_count;
-	u8			    maximum_retry;
 	struct {
 		struct sk_buff_head list;
 		spinlock_t	    lock;
@@ -50,162 +43,38 @@ struct llc_station {
 	struct sk_buff_head	    mac_pdu_q;
 };
 
-#define LLC_STATION_ACK_TIME (3 * HZ)
-
-int sysctl_llc_station_ack_timeout = LLC_STATION_ACK_TIME;
-
-/* Types of events (possible values in 'ev->type') */
-#define LLC_STATION_EV_TYPE_SIMPLE	1
-#define LLC_STATION_EV_TYPE_CONDITION	2
-#define LLC_STATION_EV_TYPE_PRIM	3
-#define LLC_STATION_EV_TYPE_PDU		4       /* command/response PDU */
-#define LLC_STATION_EV_TYPE_ACK_TMR	5
-#define LLC_STATION_EV_TYPE_RPT_STATUS	6
-
-/* Events */
-#define LLC_STATION_EV_ENABLE_WITH_DUP_ADDR_CHECK		1
-#define LLC_STATION_EV_ENABLE_WITHOUT_DUP_ADDR_CHECK		2
-#define LLC_STATION_EV_ACK_TMR_EXP_LT_RETRY_CNT_MAX_RETRY	3
-#define LLC_STATION_EV_ACK_TMR_EXP_EQ_RETRY_CNT_MAX_RETRY	4
-#define LLC_STATION_EV_RX_NULL_DSAP_XID_C			5
-#define LLC_STATION_EV_RX_NULL_DSAP_0_XID_R_XID_R_CNT_EQ	6
-#define LLC_STATION_EV_RX_NULL_DSAP_1_XID_R_XID_R_CNT_EQ	7
-#define LLC_STATION_EV_RX_NULL_DSAP_TEST_C			8
-#define LLC_STATION_EV_DISABLE_REQ				9
-
-struct llc_station_state_ev {
-	u8		 type;
-	u8		 prim;
-	u8		 prim_type;
-	u8		 reason;
-	struct list_head node; /* node in station->ev_q.list */
-};
-
-static __inline__ struct llc_station_state_ev *
-					llc_station_ev(struct sk_buff *skb)
-{
-	return (struct llc_station_state_ev *)skb->cb;
-}
-
 typedef int (*llc_station_ev_t)(struct sk_buff *skb);
 
-#define LLC_STATION_STATE_DOWN		1	/* initial state */
-#define LLC_STATION_STATE_DUP_ADDR_CHK	2
-#define LLC_STATION_STATE_UP		3
-
-#define LLC_NBR_STATION_STATES		3	/* size of state table */
-
 typedef int (*llc_station_action_t)(struct sk_buff *skb);
 
 /* Station component state table structure */
 struct llc_station_state_trans {
 	llc_station_ev_t ev;
-	u8 next_state;
 	llc_station_action_t *ev_actions;
 };
 
-struct llc_station_state {
-	u8 curr_state;
-	struct llc_station_state_trans **transitions;
-};
-
 static struct llc_station llc_main_station;
 
-static int llc_stat_ev_enable_with_dup_addr_check(struct sk_buff *skb)
-{
-	struct llc_station_state_ev *ev = llc_station_ev(skb);
-
-	return ev->type == LLC_STATION_EV_TYPE_SIMPLE &&
-	       ev->prim_type ==
-			      LLC_STATION_EV_ENABLE_WITH_DUP_ADDR_CHECK ? 0 : 1;
-}
-
-static int llc_stat_ev_enable_without_dup_addr_check(struct sk_buff *skb)
-{
-	struct llc_station_state_ev *ev = llc_station_ev(skb);
-
-	return ev->type == LLC_STATION_EV_TYPE_SIMPLE &&
-	       ev->prim_type ==
-			LLC_STATION_EV_ENABLE_WITHOUT_DUP_ADDR_CHECK ? 0 : 1;
-}
-
-static int llc_stat_ev_ack_tmr_exp_lt_retry_cnt_max_retry(struct sk_buff *skb)
-{
-	struct llc_station_state_ev *ev = llc_station_ev(skb);
-
-	return ev->type == LLC_STATION_EV_TYPE_ACK_TMR &&
-		llc_main_station.retry_count <
-		llc_main_station.maximum_retry ? 0 : 1;
-}
-
-static int llc_stat_ev_ack_tmr_exp_eq_retry_cnt_max_retry(struct sk_buff *skb)
-{
-	struct llc_station_state_ev *ev = llc_station_ev(skb);
-
-	return ev->type == LLC_STATION_EV_TYPE_ACK_TMR &&
-		llc_main_station.retry_count ==
-		llc_main_station.maximum_retry ? 0 : 1;
-}
-
 static int llc_stat_ev_rx_null_dsap_xid_c(struct sk_buff *skb)
 {
-	struct llc_station_state_ev *ev = llc_station_ev(skb);
 	struct llc_pdu_un *pdu = llc_pdu_un_hdr(skb);
 
-	return ev->type == LLC_STATION_EV_TYPE_PDU &&
-	       LLC_PDU_IS_CMD(pdu) &&			/* command PDU */
+	return LLC_PDU_IS_CMD(pdu) &&			/* command PDU */
 	       LLC_PDU_TYPE_IS_U(pdu) &&		/* U type PDU */
 	       LLC_U_PDU_CMD(pdu) == LLC_1_PDU_CMD_XID &&
 	       !pdu->dsap ? 0 : 1;			/* NULL DSAP value */
 }
 
-static int llc_stat_ev_rx_null_dsap_0_xid_r_xid_r_cnt_eq(struct sk_buff *skb)
-{
-	struct llc_station_state_ev *ev = llc_station_ev(skb);
-	struct llc_pdu_un *pdu = llc_pdu_un_hdr(skb);
-
-	return ev->type == LLC_STATION_EV_TYPE_PDU &&
-	       LLC_PDU_IS_RSP(pdu) &&			/* response PDU */
-	       LLC_PDU_TYPE_IS_U(pdu) &&		/* U type PDU */
-	       LLC_U_PDU_RSP(pdu) == LLC_1_PDU_CMD_XID &&
-	       !pdu->dsap &&				/* NULL DSAP value */
-	       !llc_main_station.xid_r_count ? 0 : 1;
-}
-
-static int llc_stat_ev_rx_null_dsap_1_xid_r_xid_r_cnt_eq(struct sk_buff *skb)
-{
-	struct llc_station_state_ev *ev = llc_station_ev(skb);
-	struct llc_pdu_un *pdu = llc_pdu_un_hdr(skb);
-
-	return ev->type == LLC_STATION_EV_TYPE_PDU &&
-	       LLC_PDU_IS_RSP(pdu) &&			/* response PDU */
-	       LLC_PDU_TYPE_IS_U(pdu) &&		/* U type PDU */
-	       LLC_U_PDU_RSP(pdu) == LLC_1_PDU_CMD_XID &&
-	       !pdu->dsap &&				/* NULL DSAP value */
-	       llc_main_station.xid_r_count == 1 ? 0 : 1;
-}
-
 static int llc_stat_ev_rx_null_dsap_test_c(struct sk_buff *skb)
 {
-	struct llc_station_state_ev *ev = llc_station_ev(skb);
 	struct llc_pdu_un *pdu = llc_pdu_un_hdr(skb);
 
-	return ev->type == LLC_STATION_EV_TYPE_PDU &&
-	       LLC_PDU_IS_CMD(pdu) &&			/* command PDU */
+	return LLC_PDU_IS_CMD(pdu) &&			/* command PDU */
 	       LLC_PDU_TYPE_IS_U(pdu) &&		/* U type PDU */
 	       LLC_U_PDU_CMD(pdu) == LLC_1_PDU_CMD_TEST &&
 	       !pdu->dsap ? 0 : 1;			/* NULL DSAP */
 }
 
-static int llc_stat_ev_disable_req(struct sk_buff *skb)
-{
-	struct llc_station_state_ev *ev = llc_station_ev(skb);
-
-	return ev->type == LLC_STATION_EV_TYPE_PRIM &&
-	       ev->prim == LLC_DISABLE_PRIM &&
-	       ev->prim_type == LLC_PRIM_TYPE_REQ ? 0 : 1;
-}
-
 /**
  *	llc_station_send_pdu - queues PDU to send
  *	@skb: Address of the PDU
@@ -220,58 +89,6 @@ static void llc_station_send_pdu(struct sk_buff *skb)
 			break;
 }
 
-static int llc_station_ac_start_ack_timer(struct sk_buff *skb)
-{
-	mod_timer(&llc_main_station.ack_timer,
-		  jiffies + sysctl_llc_station_ack_timeout);
-	return 0;
-}
-
-static int llc_station_ac_set_retry_cnt_0(struct sk_buff *skb)
-{
-	llc_main_station.retry_count = 0;
-	return 0;
-}
-
-static int llc_station_ac_inc_retry_cnt_by_1(struct sk_buff *skb)
-{
-	llc_main_station.retry_count++;
-	return 0;
-}
-
-static int llc_station_ac_set_xid_r_cnt_0(struct sk_buff *skb)
-{
-	llc_main_station.xid_r_count = 0;
-	return 0;
-}
-
-static int llc_station_ac_inc_xid_r_cnt_by_1(struct sk_buff *skb)
-{
-	llc_main_station.xid_r_count++;
-	return 0;
-}
-
-static int llc_station_ac_send_null_dsap_xid_c(struct sk_buff *skb)
-{
-	int rc = 1;
-	struct sk_buff *nskb = llc_alloc_frame(NULL, skb->dev, LLC_PDU_TYPE_U,
-					       sizeof(struct llc_xid_info));
-
-	if (!nskb)
-		goto out;
-	llc_pdu_header_init(nskb, LLC_PDU_TYPE_U, 0, 0, LLC_PDU_CMD);
-	llc_pdu_init_as_xid_cmd(nskb, LLC_XID_NULL_CLASS_2, 127);
-	rc = llc_mac_hdr_init(nskb, skb->dev->dev_addr, skb->dev->dev_addr);
-	if (unlikely(rc))
-		goto free;
-	llc_station_send_pdu(nskb);
-out:
-	return rc;
-free:
-	kfree_skb(nskb);
-	goto out;
-}
-
 static int llc_station_ac_send_xid_r(struct sk_buff *skb)
 {
 	u8 mac_da[ETH_ALEN], dsap;
@@ -326,60 +143,6 @@ free:
 	goto out;
 }
 
-static int llc_station_ac_report_status(struct sk_buff *skb)
-{
-	return 0;
-}
-
-/* DOWN STATE transitions */
-
-/* state transition for LLC_STATION_EV_ENABLE_WITH_DUP_ADDR_CHECK event */
-static llc_station_action_t llc_stat_down_state_actions_1[] = {
-	[0] = llc_station_ac_start_ack_timer,
-	[1] = llc_station_ac_set_retry_cnt_0,
-	[2] = llc_station_ac_set_xid_r_cnt_0,
-	[3] = llc_station_ac_send_null_dsap_xid_c,
-	[4] = NULL,
-};
-
-static struct llc_station_state_trans llc_stat_down_state_trans_1 = {
-	.ev	    = llc_stat_ev_enable_with_dup_addr_check,
-	.next_state = LLC_STATION_STATE_DUP_ADDR_CHK,
-	.ev_actions = llc_stat_down_state_actions_1,
-};
-
-/* state transition for LLC_STATION_EV_ENABLE_WITHOUT_DUP_ADDR_CHECK event */
-static llc_station_action_t llc_stat_down_state_actions_2[] = {
-	[0] = llc_station_ac_report_status,	/* STATION UP */
-	[1] = NULL,
-};
-
-static struct llc_station_state_trans llc_stat_down_state_trans_2 = {
-	.ev	    = llc_stat_ev_enable_without_dup_addr_check,
-	.next_state = LLC_STATION_STATE_UP,
-	.ev_actions = llc_stat_down_state_actions_2,
-};
-
-/* array of pointers; one to each transition */
-static struct llc_station_state_trans *llc_stat_dwn_state_trans[] = {
-	[0] = &llc_stat_down_state_trans_1,
-	[1] = &llc_stat_down_state_trans_2,
-	[2] = NULL,
-};
-
-/* UP STATE transitions */
-/* state transition for LLC_STATION_EV_DISABLE_REQ event */
-static llc_station_action_t llc_stat_up_state_actions_1[] = {
-	[0] = llc_station_ac_report_status,	/* STATION DOWN */
-	[1] = NULL,
-};
-
-static struct llc_station_state_trans llc_stat_up_state_trans_1 = {
-	.ev	    = llc_stat_ev_disable_req,
-	.next_state = LLC_STATION_STATE_DOWN,
-	.ev_actions = llc_stat_up_state_actions_1,
-};
-
 /* state transition for LLC_STATION_EV_RX_NULL_DSAP_XID_C event */
 static llc_station_action_t llc_stat_up_state_actions_2[] = {
 	[0] = llc_station_ac_send_xid_r,
@@ -388,7 +151,6 @@ static llc_station_action_t llc_stat_up_state_actions_2[] = {
 
 static struct llc_station_state_trans llc_stat_up_state_trans_2 = {
 	.ev	    = llc_stat_ev_rx_null_dsap_xid_c,
-	.next_state = LLC_STATION_STATE_UP,
 	.ev_actions = llc_stat_up_state_actions_2,
 };
 
@@ -400,127 +162,14 @@ static llc_station_action_t llc_stat_up_state_actions_3[] = {
 
 static struct llc_station_state_trans llc_stat_up_state_trans_3 = {
 	.ev	    = llc_stat_ev_rx_null_dsap_test_c,
-	.next_state = LLC_STATION_STATE_UP,
 	.ev_actions = llc_stat_up_state_actions_3,
 };
 
 /* array of pointers; one to each transition */
 static struct llc_station_state_trans *llc_stat_up_state_trans [] = {
-	[0] = &llc_stat_up_state_trans_1,
-	[1] = &llc_stat_up_state_trans_2,
-	[2] = &llc_stat_up_state_trans_3,
-	[3] = NULL,
-};
-
-/* DUP ADDR CHK STATE transitions */
-/* state transition for LLC_STATION_EV_RX_NULL_DSAP_0_XID_R_XID_R_CNT_EQ
- * event
- */
-static llc_station_action_t llc_stat_dupaddr_state_actions_1[] = {
-	[0] = llc_station_ac_inc_xid_r_cnt_by_1,
-	[1] = NULL,
-};
-
-static struct llc_station_state_trans llc_stat_dupaddr_state_trans_1 = {
-	.ev	    = llc_stat_ev_rx_null_dsap_0_xid_r_xid_r_cnt_eq,
-	.next_state = LLC_STATION_STATE_DUP_ADDR_CHK,
-	.ev_actions = llc_stat_dupaddr_state_actions_1,
-};
-
-/* state transition for LLC_STATION_EV_RX_NULL_DSAP_1_XID_R_XID_R_CNT_EQ
- * event
- */
-static llc_station_action_t llc_stat_dupaddr_state_actions_2[] = {
-	[0] = llc_station_ac_report_status,	/* DUPLICATE ADDRESS FOUND */
-	[1] = NULL,
-};
-
-static struct llc_station_state_trans llc_stat_dupaddr_state_trans_2 = {
-	.ev	    = llc_stat_ev_rx_null_dsap_1_xid_r_xid_r_cnt_eq,
-	.next_state = LLC_STATION_STATE_DOWN,
-	.ev_actions = llc_stat_dupaddr_state_actions_2,
-};
-
-/* state transition for LLC_STATION_EV_RX_NULL_DSAP_XID_C event */
-static llc_station_action_t llc_stat_dupaddr_state_actions_3[] = {
-	[0] = llc_station_ac_send_xid_r,
-	[1] = NULL,
-};
-
-static struct llc_station_state_trans llc_stat_dupaddr_state_trans_3 = {
-	.ev	    = llc_stat_ev_rx_null_dsap_xid_c,
-	.next_state = LLC_STATION_STATE_DUP_ADDR_CHK,
-	.ev_actions = llc_stat_dupaddr_state_actions_3,
-};
-
-/* state transition for LLC_STATION_EV_ACK_TMR_EXP_LT_RETRY_CNT_MAX_RETRY
- * event
- */
-static llc_station_action_t llc_stat_dupaddr_state_actions_4[] = {
-	[0] = llc_station_ac_start_ack_timer,
-	[1] = llc_station_ac_inc_retry_cnt_by_1,
-	[2] = llc_station_ac_set_xid_r_cnt_0,
-	[3] = llc_station_ac_send_null_dsap_xid_c,
-	[4] = NULL,
-};
-
-static struct llc_station_state_trans llc_stat_dupaddr_state_trans_4 = {
-	.ev	    = llc_stat_ev_ack_tmr_exp_lt_retry_cnt_max_retry,
-	.next_state = LLC_STATION_STATE_DUP_ADDR_CHK,
-	.ev_actions = llc_stat_dupaddr_state_actions_4,
-};
-
-/* state transition for LLC_STATION_EV_ACK_TMR_EXP_EQ_RETRY_CNT_MAX_RETRY
- * event
- */
-static llc_station_action_t llc_stat_dupaddr_state_actions_5[] = {
-	[0] = llc_station_ac_report_status,	/* STATION UP */
-	[1] = NULL,
-};
-
-static struct llc_station_state_trans llc_stat_dupaddr_state_trans_5 = {
-	.ev	    = llc_stat_ev_ack_tmr_exp_eq_retry_cnt_max_retry,
-	.next_state = LLC_STATION_STATE_UP,
-	.ev_actions = llc_stat_dupaddr_state_actions_5,
-};
-
-/* state transition for LLC_STATION_EV_DISABLE_REQ event */
-static llc_station_action_t llc_stat_dupaddr_state_actions_6[] = {
-	[0] = llc_station_ac_report_status,	/* STATION DOWN */
-	[1] = NULL,
-};
-
-static struct llc_station_state_trans llc_stat_dupaddr_state_trans_6 = {
-	.ev	    = llc_stat_ev_disable_req,
-	.next_state = LLC_STATION_STATE_DOWN,
-	.ev_actions = llc_stat_dupaddr_state_actions_6,
-};
-
-/* array of pointers; one to each transition */
-static struct llc_station_state_trans *llc_stat_dupaddr_state_trans[] = {
-	[0] = &llc_stat_dupaddr_state_trans_6,	/* Request */
-	[1] = &llc_stat_dupaddr_state_trans_4,	/* Timer */
-	[2] = &llc_stat_dupaddr_state_trans_5,
-	[3] = &llc_stat_dupaddr_state_trans_1,	/* Receive frame */
-	[4] = &llc_stat_dupaddr_state_trans_2,
-	[5] = &llc_stat_dupaddr_state_trans_3,
-	[6] = NULL,
-};
-
-static struct llc_station_state
-			llc_station_state_table[LLC_NBR_STATION_STATES] = {
-	[LLC_STATION_STATE_DOWN - 1] = {
-		.curr_state  = LLC_STATION_STATE_DOWN,
-		.transitions = llc_stat_dwn_state_trans,
-	},
-	[LLC_STATION_STATE_DUP_ADDR_CHK - 1] = {
-		.curr_state  = LLC_STATION_STATE_DUP_ADDR_CHK,
-		.transitions = llc_stat_dupaddr_state_trans,
-	},
-	[LLC_STATION_STATE_UP - 1] = {
-		.curr_state  = LLC_STATION_STATE_UP,
-		.transitions = llc_stat_up_state_trans,
-	},
+	&llc_stat_up_state_trans_2,
+	&llc_stat_up_state_trans_3,
+	NULL,
 };
 
 /**
@@ -557,10 +206,8 @@ static struct llc_station_state_trans *
 	int i = 0;
 	struct llc_station_state_trans *rc = NULL;
 	struct llc_station_state_trans **next_trans;
-	struct llc_station_state *curr_state =
-				&llc_station_state_table[llc_main_station.state - 1];
 
-	for (next_trans = curr_state->transitions; next_trans[i]; i++)
+	for (next_trans = llc_stat_up_state_trans; next_trans[i]; i++)
 		if (!next_trans[i]->ev(skb)) {
 			rc = next_trans[i];
 			break;
@@ -576,10 +223,7 @@ static struct llc_station_state_trans *
  */
 static void llc_station_free_ev(struct sk_buff *skb)
 {
-	struct llc_station_state_ev *ev = llc_station_ev(skb);
-
-	if (ev->type == LLC_STATION_EV_TYPE_PDU)
-		kfree_skb(skb);
+	kfree_skb(skb);
 }
 
 /**
@@ -594,26 +238,18 @@ static u16 llc_station_next_state(struct sk_buff *skb)
 	u16 rc = 1;
 	struct llc_station_state_trans *trans;
 
-	if (llc_main_station.state > LLC_NBR_STATION_STATES)
-		goto out;
 	trans = llc_find_station_trans(skb);
-	if (trans) {
+	if (trans)
 		/* got the state to which we next transition; perform the
 		 * actions associated with this transition before actually
 		 * transitioning to the next state
 		 */
 		rc = llc_exec_station_trans_actions(trans, skb);
-		if (!rc)
-			/* transition station to next state if all actions
-			 * execute successfully; done; wait for next event
-			 */
-			llc_main_station.state = trans->next_state;
-	} else
+	else
 		/* event not recognized in current state; re-queue it for
 		 * processing again at a later time; return failure
 		 */
 		rc = 0;
-out:
 	llc_station_free_ev(skb);
 	return rc;
 }
@@ -652,18 +288,6 @@ static void llc_station_state_process(struct sk_buff *skb)
 	spin_unlock_bh(&llc_main_station.ev_q.lock);
 }
 
-static void llc_station_ack_tmr_cb(unsigned long timeout_data)
-{
-	struct sk_buff *skb = alloc_skb(0, GFP_ATOMIC);
-
-	if (skb) {
-		struct llc_station_state_ev *ev = llc_station_ev(skb);
-
-		ev->type = LLC_STATION_EV_TYPE_ACK_TMR;
-		llc_station_state_process(skb);
-	}
-}
-
 /**
  *	llc_station_rcv - send received pdu to the station state machine
  *	@skb: received frame.
@@ -672,10 +296,6 @@ static void llc_station_ack_tmr_cb(unsigned long timeout_data)
  */
 static void llc_station_rcv(struct sk_buff *skb)
 {
-	struct llc_station_state_ev *ev = llc_station_ev(skb);
-
-	ev->type   = LLC_STATION_EV_TYPE_PDU;
-	ev->reason = 0;
 	llc_station_state_process(skb);
 }
 
@@ -684,12 +304,6 @@ void __init llc_station_init(void)
 	skb_queue_head_init(&llc_main_station.mac_pdu_q);
 	skb_queue_head_init(&llc_main_station.ev_q.list);
 	spin_lock_init(&llc_main_station.ev_q.lock);
-	setup_timer(&llc_main_station.ack_timer, llc_station_ack_tmr_cb,
-			(unsigned long)&llc_main_station);
-	llc_main_station.ack_timer.expires  = jiffies +
-						sysctl_llc_station_ack_timeout;
-	llc_main_station.maximum_retry	= 1;
-	llc_main_station.state		= LLC_STATION_STATE_UP;
 	llc_set_station_handler(llc_station_rcv);
 }
 



[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

[PATCH net-next 3/6] llc2: Collapse the station event receive path

[Ben Hutchings]

[-- Attachment #1: Type: text/plain, Size: 3942 bytes --]

We only ever put one skb on the event queue, and then immediately
process it.  Remove the queue and fold together the related functions,
removing several blatantly false comments.

Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
---
 net/llc/llc_station.c |   87 ++++---------------------------------------------
 1 file changed, 6 insertions(+), 81 deletions(-)

diff --git a/net/llc/llc_station.c b/net/llc/llc_station.c
index 917d700..3bdb888 100644
--- a/net/llc/llc_station.c
+++ b/net/llc/llc_station.c
@@ -32,14 +32,9 @@
  *
  * @mac_sa: MAC source address
  * @sap_list: list of related SAPs
- * @ev_q: events entering state mach.
  * @mac_pdu_q: PDUs ready to send to MAC
  */
 struct llc_station {
-	struct {
-		struct sk_buff_head list;
-		spinlock_t	    lock;
-	} ev_q;
 	struct sk_buff_head	    mac_pdu_q;
 };
 
@@ -216,79 +211,6 @@ static struct llc_station_state_trans *
 }
 
 /**
- *	llc_station_free_ev - frees an event
- *	@skb: Address of the event
- *
- *	Frees an event.
- */
-static void llc_station_free_ev(struct sk_buff *skb)
-{
-	kfree_skb(skb);
-}
-
-/**
- *	llc_station_next_state - processes event and goes to the next state
- *	@skb: Address of the event
- *
- *	Processes an event, executes any transitions related to that event and
- *	updates the state of the station.
- */
-static u16 llc_station_next_state(struct sk_buff *skb)
-{
-	u16 rc = 1;
-	struct llc_station_state_trans *trans;
-
-	trans = llc_find_station_trans(skb);
-	if (trans)
-		/* got the state to which we next transition; perform the
-		 * actions associated with this transition before actually
-		 * transitioning to the next state
-		 */
-		rc = llc_exec_station_trans_actions(trans, skb);
-	else
-		/* event not recognized in current state; re-queue it for
-		 * processing again at a later time; return failure
-		 */
-		rc = 0;
-	llc_station_free_ev(skb);
-	return rc;
-}
-
-/**
- *	llc_station_service_events - service events in the queue
- *
- *	Get an event from the station event queue (if any); attempt to service
- *	the event; if event serviced, get the next event (if any) on the event
- *	queue; if event not service, re-queue the event on the event queue and
- *	attempt to service the next event; when serviced all events in queue,
- *	finished; if don't transition to different state, just service all
- *	events once; if transition to new state, service all events again.
- *	Caller must hold llc_main_station.ev_q.lock.
- */
-static void llc_station_service_events(void)
-{
-	struct sk_buff *skb;
-
-	while ((skb = skb_dequeue(&llc_main_station.ev_q.list)) != NULL)
-		llc_station_next_state(skb);
-}
-
-/**
- *	llc_station_state_process - queue event and try to process queue.
- *	@skb: Address of the event
- *
- *	Queues an event (on the station event queue) for handling by the
- *	station state machine and attempts to process any queued-up events.
- */
-static void llc_station_state_process(struct sk_buff *skb)
-{
-	spin_lock_bh(&llc_main_station.ev_q.lock);
-	skb_queue_tail(&llc_main_station.ev_q.list, skb);
-	llc_station_service_events();
-	spin_unlock_bh(&llc_main_station.ev_q.lock);
-}
-
-/**
  *	llc_station_rcv - send received pdu to the station state machine
  *	@skb: received frame.
  *
@@ -296,14 +218,17 @@ static void llc_station_state_process(struct sk_buff *skb)
  */
 static void llc_station_rcv(struct sk_buff *skb)
 {
-	llc_station_state_process(skb);
+	struct llc_station_state_trans *trans;
+
+	trans = llc_find_station_trans(skb);
+	if (trans)
+		llc_exec_station_trans_actions(trans, skb);
+	kfree_skb(skb);
 }
 
 void __init llc_station_init(void)
 {
 	skb_queue_head_init(&llc_main_station.mac_pdu_q);
-	skb_queue_head_init(&llc_main_station.ev_q.list);
-	spin_lock_init(&llc_main_station.ev_q.lock);
 	llc_set_station_handler(llc_station_rcv);
 }
 



[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

[PATCH net-next 4/6] llc2: Remove the station send queue

[Ben Hutchings]

[-- Attachment #1: Type: text/plain, Size: 2637 bytes --]

We only ever put one skb on the send queue, and then immediately
send it.  Remove the queue and call dev_queue_xmit() directly.

This leaves struct llc_station empty, so remove that as well.

Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
---
 net/llc/llc_station.c |   34 ++--------------------------------
 1 file changed, 2 insertions(+), 32 deletions(-)

diff --git a/net/llc/llc_station.c b/net/llc/llc_station.c
index 3bdb888..48c2118 100644
--- a/net/llc/llc_station.c
+++ b/net/llc/llc_station.c
@@ -25,19 +25,6 @@
 #include <net/llc_s_st.h>
 #include <net/llc_pdu.h>
 
-/**
- * struct llc_station - LLC station component
- *
- * SAP and connection resource manager, one per adapter.
- *
- * @mac_sa: MAC source address
- * @sap_list: list of related SAPs
- * @mac_pdu_q: PDUs ready to send to MAC
- */
-struct llc_station {
-	struct sk_buff_head	    mac_pdu_q;
-};
-
 typedef int (*llc_station_ev_t)(struct sk_buff *skb);
 
 typedef int (*llc_station_action_t)(struct sk_buff *skb);
@@ -48,8 +35,6 @@ struct llc_station_state_trans {
 	llc_station_action_t *ev_actions;
 };
 
-static struct llc_station llc_main_station;
-
 static int llc_stat_ev_rx_null_dsap_xid_c(struct sk_buff *skb)
 {
 	struct llc_pdu_un *pdu = llc_pdu_un_hdr(skb);
@@ -70,20 +55,6 @@ static int llc_stat_ev_rx_null_dsap_test_c(struct sk_buff *skb)
 	       !pdu->dsap ? 0 : 1;			/* NULL DSAP */
 }
 
-/**
- *	llc_station_send_pdu - queues PDU to send
- *	@skb: Address of the PDU
- *
- *	Queues a PDU to send to the MAC layer.
- */
-static void llc_station_send_pdu(struct sk_buff *skb)
-{
-	skb_queue_tail(&llc_main_station.mac_pdu_q, skb);
-	while ((skb = skb_dequeue(&llc_main_station.mac_pdu_q)) != NULL)
-		if (dev_queue_xmit(skb))
-			break;
-}
-
 static int llc_station_ac_send_xid_r(struct sk_buff *skb)
 {
 	u8 mac_da[ETH_ALEN], dsap;
@@ -101,7 +72,7 @@ static int llc_station_ac_send_xid_r(struct sk_buff *skb)
 	rc = llc_mac_hdr_init(nskb, skb->dev->dev_addr, mac_da);
 	if (unlikely(rc))
 		goto free;
-	llc_station_send_pdu(nskb);
+	dev_queue_xmit(nskb);
 out:
 	return rc;
 free:
@@ -130,7 +101,7 @@ static int llc_station_ac_send_test_r(struct sk_buff *skb)
 	rc = llc_mac_hdr_init(nskb, skb->dev->dev_addr, mac_da);
 	if (unlikely(rc))
 		goto free;
-	llc_station_send_pdu(nskb);
+	dev_queue_xmit(nskb);
 out:
 	return rc;
 free:
@@ -228,7 +199,6 @@ static void llc_station_rcv(struct sk_buff *skb)
 
 void __init llc_station_init(void)
 {
-	skb_queue_head_init(&llc_main_station.mac_pdu_q);
 	llc_set_station_handler(llc_station_rcv);
 }
 



[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

[PATCH net-next 5/6] llc2: Remove explicit indexing of state action arrays

[Ben Hutchings]

[-- Attachment #1: Type: text/plain, Size: 1198 bytes --]

These arrays are accessed by iteration in
llc_exec_station_trans_actions().  There must not be any zero-filled
gaps in them, so the explicit indices are pointless.

Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
---
 net/llc/llc_station.c |    8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/net/llc/llc_station.c b/net/llc/llc_station.c
index 48c2118..fe43158 100644
--- a/net/llc/llc_station.c
+++ b/net/llc/llc_station.c
@@ -111,8 +111,8 @@ free:
 
 /* state transition for LLC_STATION_EV_RX_NULL_DSAP_XID_C event */
 static llc_station_action_t llc_stat_up_state_actions_2[] = {
-	[0] = llc_station_ac_send_xid_r,
-	[1] = NULL,
+	llc_station_ac_send_xid_r,
+	NULL,
 };
 
 static struct llc_station_state_trans llc_stat_up_state_trans_2 = {
@@ -122,8 +122,8 @@ static struct llc_station_state_trans llc_stat_up_state_trans_2 = {
 
 /* state transition for LLC_STATION_EV_RX_NULL_DSAP_TEST_C event */
 static llc_station_action_t llc_stat_up_state_actions_3[] = {
-	[0] = llc_station_ac_send_test_r,
-	[1] = NULL,
+	llc_station_ac_send_test_r,
+	NULL,
 };
 
 static struct llc_station_state_trans llc_stat_up_state_trans_3 = {



[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

[PATCH net-next 6/6] llc2: Collapse remainder of state machine into simple if-else if-statement

[Ben Hutchings]

[-- Attachment #1: Type: text/plain, Size: 3669 bytes --]

Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
---
 net/llc/llc_station.c |   91 +++----------------------------------------------
 1 file changed, 4 insertions(+), 87 deletions(-)

diff --git a/net/llc/llc_station.c b/net/llc/llc_station.c
index fe43158..204a835 100644
--- a/net/llc/llc_station.c
+++ b/net/llc/llc_station.c
@@ -25,16 +25,6 @@
 #include <net/llc_s_st.h>
 #include <net/llc_pdu.h>
 
-typedef int (*llc_station_ev_t)(struct sk_buff *skb);
-
-typedef int (*llc_station_action_t)(struct sk_buff *skb);
-
-/* Station component state table structure */
-struct llc_station_state_trans {
-	llc_station_ev_t ev;
-	llc_station_action_t *ev_actions;
-};
-
 static int llc_stat_ev_rx_null_dsap_xid_c(struct sk_buff *skb)
 {
 	struct llc_pdu_un *pdu = llc_pdu_un_hdr(skb);
@@ -109,78 +99,6 @@ free:
 	goto out;
 }
 
-/* state transition for LLC_STATION_EV_RX_NULL_DSAP_XID_C event */
-static llc_station_action_t llc_stat_up_state_actions_2[] = {
-	llc_station_ac_send_xid_r,
-	NULL,
-};
-
-static struct llc_station_state_trans llc_stat_up_state_trans_2 = {
-	.ev	    = llc_stat_ev_rx_null_dsap_xid_c,
-	.ev_actions = llc_stat_up_state_actions_2,
-};
-
-/* state transition for LLC_STATION_EV_RX_NULL_DSAP_TEST_C event */
-static llc_station_action_t llc_stat_up_state_actions_3[] = {
-	llc_station_ac_send_test_r,
-	NULL,
-};
-
-static struct llc_station_state_trans llc_stat_up_state_trans_3 = {
-	.ev	    = llc_stat_ev_rx_null_dsap_test_c,
-	.ev_actions = llc_stat_up_state_actions_3,
-};
-
-/* array of pointers; one to each transition */
-static struct llc_station_state_trans *llc_stat_up_state_trans [] = {
-	&llc_stat_up_state_trans_2,
-	&llc_stat_up_state_trans_3,
-	NULL,
-};
-
-/**
- *	llc_exec_station_trans_actions - executes actions for transition
- *	@trans: Address of the transition
- *	@skb: Address of the event that caused the transition
- *
- *	Executes actions of a transition of the station state machine. Returns
- *	0 if all actions complete successfully, nonzero otherwise.
- */
-static u16 llc_exec_station_trans_actions(struct llc_station_state_trans *trans,
-					  struct sk_buff *skb)
-{
-	u16 rc = 0;
-	llc_station_action_t *next_action = trans->ev_actions;
-
-	for (; next_action && *next_action; next_action++)
-		if ((*next_action)(skb))
-			rc = 1;
-	return rc;
-}
-
-/**
- *	llc_find_station_trans - finds transition for this event
- *	@skb: Address of the event
- *
- *	Search thru events of the current state of the station until list
- *	exhausted or it's obvious that the event is not valid for the current
- *	state. Returns the address of the transition if cound, %NULL otherwise.
- */
-static struct llc_station_state_trans *
-				llc_find_station_trans(struct sk_buff *skb)
-{
-	int i = 0;
-	struct llc_station_state_trans *rc = NULL;
-	struct llc_station_state_trans **next_trans;
-
-	for (next_trans = llc_stat_up_state_trans; next_trans[i]; i++)
-		if (!next_trans[i]->ev(skb)) {
-			rc = next_trans[i];
-			break;
-		}
-	return rc;
-}
-
 /**
  *	llc_station_rcv - send received pdu to the station state machine
  *	@skb: received frame.
@@ -189,11 +107,10 @@ static struct llc_station_state_trans *
  */
 static void llc_station_rcv(struct sk_buff *skb)
 {
-	struct llc_station_state_trans *trans;
-
-	trans = llc_find_station_trans(skb);
-	if (trans)
-		llc_exec_station_trans_actions(trans, skb);
+	if (llc_stat_ev_rx_null_dsap_xid_c(skb))
+		llc_station_ac_send_xid_r(skb);
+	else if (llc_stat_ev_rx_null_dsap_test_c(skb))
+		llc_station_ac_send_test_r(skb);
 	kfree_skb(skb);
 }
 


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

Re: [PATCH 0/6] llc2: Simplify llc_station

[Ben Hutchings]

[-- Attachment #1: Type: text/plain, Size: 397 bytes --]

On Sun, 2012-09-16 at 04:09 +0100, Ben Hutchings wrote:
> There seem to have been some grand plans for llc_station, but as they
> haven't been fulfilled it's just unnecessarily complicated.

Oh, a warning: this is compile-tested only.  I don't even know what I
would test this with.

Ben.

-- 
Ben Hutchings
Experience is what causes a person to make new mistakes instead of old ones.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

Re: [PATCH 0/6] llc2: Simplify llc_station

[David Miller]

From: Ben Hutchings <ben@decadent.org.uk>
Date: Sun, 16 Sep 2012 04:09:42 +0100

> There seem to have been some grand plans for llc_station, but as they
> haven't been fulfilled it's just unnecessarily complicated.

I went over these a few times, they look correct, so I am going
to apply them to net-next.

If there are any problems let's hope that exposure in the tree
helps shake those out.

Re: [PATCH 0/6] llc2: Simplify llc_station

[David Miller]

From: David Miller <davem@davemloft.net>
Date: Mon, 17 Sep 2012 13:05:31 -0400 (EDT)

> From: Ben Hutchings <ben@decadent.org.uk>
> Date: Sun, 16 Sep 2012 04:09:42 +0100
> 
>> There seem to have been some grand plans for llc_station, but as they
>> haven't been fulfilled it's just unnecessarily complicated.
> 
> I went over these a few times, they look correct, so I am going
> to apply them to net-next.
> 
> If there are any problems let's hope that exposure in the tree
> helps shake those out.

It doesn't even build properly, please fix this and resubmit:

ERROR: "sysctl_llc_station_ack_timeout" [net/llc/llc2.ko] undefined!

Re: [PATCH 0/6] llc2: Simplify llc_station

[David Miller]

From: David Miller <davem@davemloft.net>
Date: Mon, 17 Sep 2012 13:10:17 -0400 (EDT)

> From: David Miller <davem@davemloft.net>
> Date: Mon, 17 Sep 2012 13:05:31 -0400 (EDT)
> 
>> From: Ben Hutchings <ben@decadent.org.uk>
>> Date: Sun, 16 Sep 2012 04:09:42 +0100
>> 
>>> There seem to have been some grand plans for llc_station, but as they
>>> haven't been fulfilled it's just unnecessarily complicated.
>> 
>> I went over these a few times, they look correct, so I am going
>> to apply them to net-next.
>> 
>> If there are any problems let's hope that exposure in the tree
>> helps shake those out.
> 
> It doesn't even build properly, please fix this and resubmit:
> 
> ERROR: "sysctl_llc_station_ack_timeout" [net/llc/llc2.ko] undefined!

Actually, since I trusted you when you said you build tested this,
I pushed it out to net-next pre-maturely.

I'm going to fix this meanwhile by simply removing the sysctl that
references this symbol.

But you need to check for me whether that's ok or not.

Re: [PATCH 0/6] llc2: Simplify llc_station

[Ben Hutchings]

On Mon, Sep 17, 2012 at 01:12:37PM -0400, David Miller wrote:
> From: David Miller <davem@davemloft.net>
> Date: Mon, 17 Sep 2012 13:10:17 -0400 (EDT)
> 
> > From: David Miller <davem@davemloft.net>
> > Date: Mon, 17 Sep 2012 13:05:31 -0400 (EDT)
> > 
> >> From: Ben Hutchings <ben@decadent.org.uk>
> >> Date: Sun, 16 Sep 2012 04:09:42 +0100
> >> 
> >>> There seem to have been some grand plans for llc_station, but as they
> >>> haven't been fulfilled it's just unnecessarily complicated.
> >> 
> >> I went over these a few times, they look correct, so I am going
> >> to apply them to net-next.
> >> 
> >> If there are any problems let's hope that exposure in the tree
> >> helps shake those out.
> > 
> > It doesn't even build properly, please fix this and resubmit:
> > 
> > ERROR: "sysctl_llc_station_ack_timeout" [net/llc/llc2.ko] undefined!
> 
> Actually, since I trusted you when you said you build tested this,
> I pushed it out to net-next pre-maturely.
> 
> I'm going to fix this meanwhile by simply removing the sysctl that
> references this symbol.
> 
> But you need to check for me whether that's ok or not.
 
The sysctl had no effect so I think it's fairly safe to assume nothing
depends on it.  Thanks.

Ben.

-- 
Ben Hutchings
We get into the habit of living before acquiring the habit of thinking.
                                                              - Albert Camus

Re: [PATCH 0/6] llc2: Simplify llc_station

[Ben Hutchings]

On Mon, Sep 17, 2012 at 01:10:17PM -0400, David Miller wrote:
> From: David Miller <davem@davemloft.net>
> Date: Mon, 17 Sep 2012 13:05:31 -0400 (EDT)
> 
> > From: Ben Hutchings <ben@decadent.org.uk>
> > Date: Sun, 16 Sep 2012 04:09:42 +0100
> > 
> >> There seem to have been some grand plans for llc_station, but as they
> >> haven't been fulfilled it's just unnecessarily complicated.
> > 
> > I went over these a few times, they look correct, so I am going
> > to apply them to net-next.
> > 
> > If there are any problems let's hope that exposure in the tree
> > helps shake those out.
> 
> It doesn't even build properly, please fix this and resubmit:
> 
> ERROR: "sysctl_llc_station_ack_timeout" [net/llc/llc2.ko] undefined!

Sorry, I was able to build net/llc/ successfully but didn't do a full
build which would have caught this.

Ben.

-- 
Ben Hutchings
We get into the habit of living before acquiring the habit of thinking.
                                                              - Albert Camus