On 11/6/2021 11:08 AM, Sergey Ryazanov wrote:

On Mon, Nov 1, 2021 at 6:57 AM Ricardo Martinez
[off-list ref] wrote:

quoted

Creates the Cross Core Modem Network Interface (CCMNI) which implements
the wwan_ops for registration with the WWAN framework, CCMNI also
implements the net_device_ops functions used by the network device.
Network device operations include open, close, start transmission, TX
timeout, change MTU, and select queue.

[skipped]

quoted

+static enum txq_type get_txq_type(struct sk_buff *skb)
+{
+       u32 total_len, payload_len, l4_off;
+       bool tcp_syn_fin_rst, is_tcp;
+       struct ipv6hdr *ip6h;
+       struct tcphdr *tcph;
+       struct iphdr *ip4h;
+       u32 packet_type;
+       __be16 frag_off;
+
+       packet_type = skb->data[0] & SBD_PACKET_TYPE_MASK;
+       if (packet_type == IPV6_VERSION) {
+               ip6h = (struct ipv6hdr *)skb->data;
+               total_len = sizeof(struct ipv6hdr) + ntohs(ip6h->payload_len);
+               l4_off = ipv6_skip_exthdr(skb, sizeof(struct ipv6hdr), &ip6h->nexthdr, &frag_off);
+               tcph = (struct tcphdr *)(skb->data + l4_off);
+               is_tcp = ip6h->nexthdr == IPPROTO_TCP;
+               payload_len = total_len - l4_off - (tcph->doff << 2);
+       } else if (packet_type == IPV4_VERSION) {
+               ip4h = (struct iphdr *)skb->data;
+               tcph = (struct tcphdr *)(skb->data + (ip4h->ihl << 2));
+               is_tcp = ip4h->protocol == IPPROTO_TCP;
+               payload_len = ntohs(ip4h->tot_len) - (ip4h->ihl << 2) - (tcph->doff << 2);
+       } else {
+               return TXQ_NORMAL;
+       }
+
+       tcp_syn_fin_rst = tcph->syn || tcph->fin || tcph->rst;
+       if (is_tcp && !payload_len && !tcp_syn_fin_rst)
+               return TXQ_FAST;
+
+       return TXQ_NORMAL;
+}

I am wondering how much modem performance has improved with this
optimization compared to the performance loss on each packet due to
the cache miss? Do you have any measurement results?

No performance gains observed in the latest tests, this is going to be 
removed for the

next iteration.

quoted

+static u16 ccmni_select_queue(struct net_device *dev, struct sk_buff *skb,
+                             struct net_device *sb_dev)
+{
+       struct ccmni_instance *ccmni;
+
+       ccmni = netdev_priv(dev);
+
+       if (ccmni->ctlb->capability & NIC_CAP_DATA_ACK_DVD)
+               return get_txq_type(skb);
+
+       return TXQ_NORMAL;
+}
+
+static int ccmni_open(struct net_device *dev)
+{
+       struct ccmni_instance *ccmni;
+
+       ccmni = wwan_netdev_drvpriv(dev);

[skipped]

quoted

+       skb_set_mac_header(skb, -ETH_HLEN);
+       skb_reset_network_header(skb);
+       skb->dev = dev;
+       if (pkt_type == IPV6_VERSION)
+               skb->protocol = htons(ETH_P_IPV6);
+       else
+               skb->protocol = htons(ETH_P_IP);
+
+       skb_len = skb->len;
+
+       netif_rx_any_context(skb);

Did you consider using NAPI for the packet Rx path? This should
improve Rx performance.

Yes, NAPI implementation is in the plan.

quoted

+       dev->stats.rx_packets++;
+       dev->stats.rx_bytes += skb_len;
+}

[skipped]

quoted

diff --git a/drivers/net/wwan/t7xx/t7xx_netdev.h b/drivers/net/wwan/t7xx/t7xx_netdev.h

...
+#define CCMNI_TX_QUEUE         1000

Is this a really carefully selected queue depth limit, or just an
arbitrary value? If the last one, then feel free to use  the
DEFAULT_TX_QUEUE_LEN macro.

Changing this to DEFAULT_TX_QUEUE_LEN for the next iteration

quoted

..
+#define IPV4_VERSION           0x40
+#define IPV6_VERSION           0x60

Just curious why the _VERSION suffix? Why not, for example, PKT_TYPE_ prefix?

Nothing special about _VERSION, but it does look a bit weird, will use 
PKT_TYPE_  as suggested

--
Sergey

Ricardo