v2.1.0

Implement proper functionality for echoing transmits
Transmit errors are now reported properly
Transmit byte count is now reported properly
Use with icsscand >= v2.1.0 for full echo support

.gitignore

@@ -98,3 +98,7 @@ extra_certificates
 signing_key.priv
 signing_key.x509
 x509.genkey
+
+.vscode
+compile_commands.json
+intrepid.mod

CHANGELOG

@@ -1,3 +1,9 @@
+v2.1.0
+	Implement proper functionality for echoing transmits
+	Transmit errors are now reported properly
+	Transmit byte count is now reported properly
+	Use with icsscand >= v2.1.0 for full echo support
+
 v2.0.5
 	Add support for kernels 5.9+
 		Tested on 5.11.0-rc5

README.md

@@ -1,4 +1,4 @@
-Version 2.0.5
+Version 2.1.0
 
 This is the kernel object portion of the Intrepid Control Systems SocketCAN support. For SocketCAN to work with Intrepid devices you will need to have this kernel object loaded on your system. Once the module is built and loaded run [icsscand](https://github.com/intrepidcs/icsscand) to turn on SocketCAN support.
 

intrepid.c

@@ -1,7 +1,7 @@
 /*
  * intrepid.c - Netdevice driver for Intrepid CAN/Ethernet devices
  *
- * Copyright (c) 2016-2019 Intrepid Control Systems, Inc.
+ * Copyright (c) 2016-2022 Intrepid Control Systems, Inc.
  * All rights reserved.
  *
  * This program is free software; you can redistribute it and/or modify it
@@ -56,8 +56,8 @@
 
 #define KO_DESC "Netdevice driver for Intrepid CAN/Ethernet devices"
 #define KO_MAJOR 2
-#define KO_MINOR 0
-#define KO_PATCH 5
+#define KO_MINOR 1
+#define KO_PATCH 0
 #define KO_VERSION str(KO_MAJOR) "." str(KO_MINOR) "." str(KO_PATCH)
 #define KO_VERSION_INT (KO_MAJOR << 16) | (KO_MINOR << 8) | KO_PATCH
 
@@ -111,6 +111,7 @@ struct intrepid_netdevice {
 	spinlock_t              lock;
 	int                     is_stopped;
 	unsigned char           *from_user;
+	uint8_t                 tx_idx;
 };
 
 static int                      is_open;
@@ -134,6 +135,27 @@ static spinlock_t               tx_box_lock;
 	(shared_mem + (RX_BOX_SIZE * DEVICE_INDEX))
 #define GET_TX_BOX(BOX_INDEX) \
 	(shared_mem + (SHARED_MEM_SIZE / 2) + (BOX_INDEX * TX_BOX_SIZE))
+#define MAX_TX (0x100)
+#define DESC_OFFSET (2)
+
+static uint16_t intrepid_next_tx_description(
+	struct intrepid_netdevice* ics,
+	int* idx_out)
+{
+	/* we offset the description so that we know 0 is not us transmitting */
+	uint16_t description = ics->tx_idx + DESC_OFFSET;
+	*idx_out = ics->tx_idx;
+	ics->tx_idx++;
+	return description;
+}
+
+static int intrepid_description_to_idx(uint16_t description)
+{
+	if (description < DESC_OFFSET || description >= DESC_OFFSET + MAX_TX)
+		return -1;
+
+	return description - DESC_OFFSET;
+}
 
 /* Returns 1 when we would not have enough space to hold another message of `size` */
 static inline int intrepid_tx_box_no_space_for(size_t size)
@@ -182,16 +204,14 @@ static void intrepid_pause_all_queues(void)
 static netdev_tx_t intrepid_netdevice_xmit(struct sk_buff *skb, struct net_device *dev)
 {
 	int ret = NETDEV_TX_OK;
-	struct net_device_stats *stats = &dev->stats;
 	struct intrepid_netdevice *ics = netdev_priv(dev);
 	struct canfd_frame *cf = (struct canfd_frame*)skb->data;
 	bool fd = can_is_canfd_skb(skb);
 	bool needs_unlock = false;
+	bool consumed = false;
+	int tx_idx;
 	neomessage_can_t msg = {0};
 
-	stats->tx_packets++;
-	stats->tx_bytes = cf->len;
-
 	if (can_dropped_invalid_skb(dev, skb)) {
 		pr_info("intrepid: dropping invalid frame on %s\n", dev->name);
 		goto exit;
@@ -237,6 +257,7 @@ static netdev_tx_t intrepid_netdevice_xmit(struct sk_buff *skb, struct net_devic
 
 	msg.length = cf->len;
 	msg.netid = dev->base_addr;
+	msg.type = ICSNEO_NETWORK_TYPE_CAN;
 
 	if (intrepid_tx_box_no_space_for(sizeof(neomessage_can_t) + msg.length)) {
 		/* This should never happen, the queue should be paused before this */
@@ -247,6 +268,10 @@ static netdev_tx_t intrepid_netdevice_xmit(struct sk_buff *skb, struct net_devic
 		goto exit;
 	}
 
+	msg.description = intrepid_next_tx_description(ics, &tx_idx);
+	can_put_echo_skb(skb, dev, tx_idx, msg.length);
+	consumed = true;
+
 	/* Copy the message into the usermode box */
 	memcpy(tx_boxes[current_tx_box] + tx_box_bytes[current_tx_box], &msg, sizeof(neomessage_can_t));
 	tx_box_bytes[current_tx_box] += sizeof(neomessage_can_t);
@@ -259,7 +284,7 @@ static netdev_tx_t intrepid_netdevice_xmit(struct sk_buff *skb, struct net_devic
 		intrepid_pause_all_queues();
 
 exit:
-	if(ret == NETDEV_TX_OK)
+	if(ret == NETDEV_TX_OK && !consumed)
 		consume_skb(skb);
 	wake_up_interruptible(&tx_wait);
 	if(needs_unlock)
@@ -271,6 +296,8 @@ static int intrepid_netdevice_stop(struct net_device *dev)
 {
 	struct intrepid_netdevice *ics = netdev_priv(dev);
 
+	close_candev(dev);
+
 	spin_lock_bh(&ics->lock);
 	netif_stop_queue(dev);
 	netif_carrier_off(dev);
@@ -350,7 +377,7 @@ static int intrepid_add_can_if(struct intrepid_netdevice **result, const char *r
 		goto exit;
 	}
 
-	dev = alloc_candev(sizeof(*ics), 1);
+	dev = alloc_candev(sizeof(*ics), MAX_TX);
 	if (!dev) {
 		pr_alert("intrepid: Could not allocate candev\n");
 		goto exit;
@@ -380,6 +407,7 @@ static int intrepid_add_can_if(struct intrepid_netdevice **result, const char *r
 	ics->dev             = dev;
 	ics->is_stopped      = 0;
 	ics->from_user       = GET_RX_BOX(i); /* incoming rx messages */
+	ics->tx_idx          = 0;
 
 	spin_lock_init(&ics->lock);
 
@@ -483,23 +511,74 @@ static int intrepid_fill_can_frame_from_neomessage(
 	return 0;
 }
 
+/* Returns true if this message was handled as a transmit receipt.
+ * If false is returned, this message should be handled as a receive
+ * message, regardless of the transmit flag.
+ */
+static bool handle_transmit_receipt(
+	struct net_device *device,
+	const neomessage_can_t *msg,
+	const uint8_t *data,
+	struct net_device_stats *stats)
+{
+	int length;
+	int tx_idx;
+
+	if (!msg->status.transmitMessage)
+		return false;
+
+	tx_idx = intrepid_description_to_idx(msg->description);
+
+	/* not transmitted by us, maybe by CoreMini */
+	/* just handle it as a receive */
+	if (tx_idx < 0)
+		return false;
+
+	/* unsuccessful transmits */
+	/* stats are handled in intrepid_fill_canerr_frame_from_neomessage */
+	if (msg->status.globalError) {
+		can_free_echo_skb(device, tx_idx, NULL);
+		return false;
+	}
+
+	length = can_get_echo_skb(device, tx_idx, NULL);
+	stats->tx_packets++;
+	stats->tx_bytes += length;
+	return true;
+}
+
 static struct sk_buff *intrepid_skb_from_neomessage(
 	struct net_device *device,
-	const neomessage_t *msg,
+	const neomessage_frame_t *msg_generic,
 	const uint8_t *data,
 	struct net_device_stats *stats)
 {
 	struct sk_buff *skb = NULL;
 	struct canfd_frame* cf = NULL;
+	const neomessage_can_t* msg = NULL;
 	int ret = 0;
 
 	/* input validation */
-	if (unlikely(device == NULL || msg == NULL || data == NULL || stats == NULL)) {
+	if (unlikely(device == NULL || msg_generic == NULL || data == NULL || stats == NULL)) {
 		stats->rx_dropped++;
 		pr_warn("intrepid: Dropping message on %s, skb from neomessage input validation failed", device->name);
-		goto fail;
+		goto out;
 	}
 
+	if (unlikely(msg_generic->type != ICSNEO_NETWORK_TYPE_CAN)) {
+		stats->rx_dropped++;
+		pr_warn("intrepid: Dropping message on %s, wrong type %d", device->name, (int)msg_generic->type);
+		goto out;
+	}
+
+	msg = (const neomessage_can_t*)msg_generic;
+
+	/* if this message is handled as a transmit receipt,
+	 * don't turn it into a receive skb here.
+	 */
+	if (handle_transmit_receipt(device, msg, data, stats))
+		goto out;
+
 	if (msg->status.globalError)
 		skb = alloc_can_err_skb(device, (struct can_frame**)&cf);
 	else if (msg->status.canfdFDF)
@@ -510,7 +589,7 @@ static struct sk_buff *intrepid_skb_from_neomessage(
 	if (unlikely(skb == NULL)) {
 		stats->rx_dropped++;
 		pr_warn("intrepid: Dropping message on %s, skb allocation failed", device->name);
-		goto fail;
+		goto out;
 	}
 
 	switch(msg->type) {
@@ -535,16 +614,16 @@ static struct sk_buff *intrepid_skb_from_neomessage(
 			break;
 		default:
 			pr_warn("intrepid: Dropping message on %s, invalid type %d", device->name, msg->type);
-			goto fail;
+			goto out;
 	}
 
 
 	if (unlikely(ret != 0)) {
 		pr_warn("intrepid: Dropping message on %s, frame fill failed", device->name);
-		goto fail;
+		goto out;
 	}
 
-fail:
+out:
 	return skb;
 }
 
@@ -569,19 +648,19 @@ static int intrepid_read_messages(int device_index, unsigned int count)
 	 * converting neomessage_t to a CAN sk_buff */
 
 	while (count--) {
-		const neomessage_t *msg;
+		const neomessage_frame_t *msg;
 		const uint8_t *data;
 		struct sk_buff *skb;
 		int ret = 0;
 
-		msg = (const neomessage_t*)currentPosition;
-		currentPosition += sizeof(neomessage_t);
+		msg = (const neomessage_frame_t*)currentPosition;
+		currentPosition += sizeof(neomessage_frame_t);
 		data = currentPosition;
 		currentPosition += msg->length;
 
 		/* pass along the converted message to the kernel for dispatch */
 		skb = intrepid_skb_from_neomessage(device, msg, data, stats);
-		if (likely(skb != NULL))
+		if (skb != NULL)
 			ret = netif_rx(skb);
 
 		if (ret == NET_RX_DROP)

neomessage.h

@@ -16,12 +16,12 @@ typedef union {
 		uint32_t extendedFrame : 1;
 		uint32_t remoteFrame : 1;
 		uint32_t crcError : 1;
-		uint32_t canErrorPassive : 1;
+		uint32_t canErrorPassive : 1; // Occupies the same space as headerCRCError
 		uint32_t incompleteFrame : 1;
 		uint32_t lostArbitration : 1;
 		uint32_t undefinedError : 1;
 		uint32_t canBusOff : 1;
-		uint32_t canErrorWarning : 1;
+		uint32_t canBusRecovered : 1;
 		uint32_t canBusShortedPlus : 1;
 		uint32_t canBusShortedGround : 1;
 		uint32_t checksumError : 1;
@@ -98,6 +98,10 @@ typedef union {
 #pragma warning(pop)
 #endif
 
+typedef uint16_t neonetid_t;
+typedef uint8_t neonettype_t;
+typedef uint16_t neomessagetype_t;
+
 #define ICSNEO_NETWORK_TYPE_INVALID ((uint8_t)0)
 #define ICSNEO_NETWORK_TYPE_INTERNAL ((uint8_t)1) // Used for statuses that don't actually need to be transferred to the client application
 #define ICSNEO_NETWORK_TYPE_CAN ((uint8_t)2)
@@ -109,42 +113,73 @@ typedef union {
 #define ICSNEO_NETWORK_TYPE_OTHER ((uint8_t)0xFF)
 
 typedef struct {
-	neomessage_statusbitfield_t status;
+	uint8_t _reserved1[16];
 	uint64_t timestamp;
-	uint64_t timestampReserved;
-	const uint8_t* data;
-	size_t length;
-	uint8_t header[4];
-	uint16_t netid;
-	uint8_t type;
-	uint8_t reserved[17];
+	uint64_t _reservedTimestamp;
+	uint8_t _reserved2[sizeof(size_t) * 2 + 7 + sizeof(neonetid_t) + sizeof(neonettype_t)];
+	neomessagetype_t messageType;
+	uint8_t _reserved3[12];
 } neomessage_t; // 72 bytes total
 // Any time you add another neomessage_*_t type, make sure to add it to the static_asserts below!
 
 typedef struct {
 	neomessage_statusbitfield_t status;
 	uint64_t timestamp;
-	uint64_t timestampReserved;
+	uint64_t _reservedTimestamp;
+	const uint8_t* data;
+	size_t length;
+	uint8_t header[4];
+	neonetid_t netid;
+	neonettype_t type;
+	uint8_t _reserved0;
+	uint16_t description;
+	neomessagetype_t messageType;
+	uint8_t _reserved1[12];
+} neomessage_frame_t;
+
+typedef struct {
+	neomessage_statusbitfield_t status;
+	uint64_t timestamp;
+	uint64_t _reservedTimestamp;
 	const uint8_t* data;
 	size_t length;
 	uint32_t arbid;
-	uint16_t netid;
-	uint8_t type;
+	neonetid_t netid;
+	neonettype_t type;
 	uint8_t dlcOnWire;
-	uint8_t reserved[16];
+	uint16_t description;
+	neomessagetype_t messageType;
+	uint8_t _reserved1[12];
 } neomessage_can_t;
 
 typedef struct {
 	neomessage_statusbitfield_t status;
 	uint64_t timestamp;
-	uint64_t timestampReserved;
+	uint64_t _reservedTimestamp;
+	size_t _reserved2[2];
+	uint8_t transmitErrorCount;
+	uint8_t receiveErrorCount;
+	uint8_t _reserved3[5];
+	neonetid_t netid;
+	neonettype_t type;
+	neomessagetype_t messageType;
+	uint8_t _reserved4[12];
+} neomessage_can_error_t;
+
+typedef struct {
+	neomessage_statusbitfield_t status;
+	uint64_t timestamp;
+	uint64_t _reservedTimestamp;
 	const uint8_t* data;
 	size_t length;
 	uint8_t preemptionFlags;
-	uint8_t reservedHeader[3];
-	uint16_t netid;
-	uint8_t type;
-	uint8_t reserved[17];
+	uint8_t _reservedHeader[3];
+	neonetid_t netid;
+	neonettype_t type;
+	uint8_t _reserved0;
+	uint16_t description;
+	neomessagetype_t messageType;
+	uint8_t _reserved1[12];
 } neomessage_eth_t;
 
 #pragma pack(pop)