diff mbox

[10/25] iwlwifi: mvm: support packet injection

Message ID 20160919073028.22065-10-luca@coelho.fi (mailing list archive)
State Accepted
Delegated to: Kalle Valo
Headers show

Commit Message

Luca Coelho Sept. 19, 2016, 7:30 a.m. UTC
From: Sara Sharon <sara.sharon@intel.com>

For automatic testing packet injection can be useful.
Support injection through debugfs.

Signed-off-by: Sara Sharon <sara.sharon@intel.com>
Signed-off-by: Luca Coelho <luciano.coelho@intel.com>
---
 drivers/net/wireless/intel/iwlwifi/mvm/debugfs.c | 55 ++++++++++++++++++++++++
 1 file changed, 55 insertions(+)

Comments

Kalle Valo Sept. 24, 2016, 11:19 a.m. UTC | #1
Luca Coelho <luca@coelho.fi> writes:

> From: Sara Sharon <sara.sharon@intel.com>
>
> For automatic testing packet injection can be useful.
> Support injection through debugfs.
>
> Signed-off-by: Sara Sharon <sara.sharon@intel.com>
> Signed-off-by: Luca Coelho <luciano.coelho@intel.com>

The commit log doesn't tell a lot. I started to wonder why use debugfs
and not the proper interface through mac80211?

> --- a/drivers/net/wireless/intel/iwlwifi/mvm/debugfs.c
> +++ b/drivers/net/wireless/intel/iwlwifi/mvm/debugfs.c
> @@ -917,6 +917,59 @@ static ssize_t iwl_dbgfs_indirection_tbl_write(struct iwl_mvm *mvm,
>  	return ret ?: count;
>  }
>  
> +static ssize_t iwl_dbgfs_inject_packet_write(struct iwl_mvm *mvm,
> +					     char *buf, size_t count,
> +					     loff_t *ppos)
> +{
> +	struct iwl_rx_cmd_buffer rxb = {
> +		._rx_page_order = 0,
> +		.truesize = 0, /* not used */
> +		._offset = 0,
> +	};
> +	struct iwl_rx_packet *pkt;
> +	struct iwl_rx_mpdu_desc *desc;
> +	int bin_len = count / 2;
> +	int ret = -EINVAL;
> +
> +	/* supporting only 9000 descriptor */
> +	if (!mvm->trans->cfg->mq_rx_supported)
> +		return -ENOTSUPP;
> +
> +	rxb._page = alloc_pages(GFP_ATOMIC, 0);
> +	if (!rxb._page)
> +		return -ENOMEM;
> +	pkt = rxb_addr(&rxb);
> +
> +	ret = hex2bin(page_address(rxb._page), buf, bin_len);
> +	if (ret)
> +		goto out;
> +
> +	/* avoid invalid memory access */
> +	if (bin_len < sizeof(*pkt) + sizeof(*desc))
> +		goto out;
> +
> +	/* check this is RX packet */
> +	if (WIDE_ID(pkt->hdr.group_id, pkt->hdr.cmd) !=
> +	    WIDE_ID(LEGACY_GROUP, REPLY_RX_MPDU_CMD))
> +		goto out;
> +
> +	/* check the length in metadata matches actual received length */
> +	desc = (void *)pkt->data;
> +	if (le16_to_cpu(desc->mpdu_len) !=
> +	    (bin_len - sizeof(*desc) - sizeof(*pkt)))
> +		goto out;
> +
> +	local_bh_disable();
> +	iwl_mvm_rx_mpdu_mq(mvm, NULL, &rxb, 0);
> +	local_bh_enable();
> +	ret = 0;

But reading from the code makes me suspect that this isn't really normal
packet injection, more like passing full descriptors to the hw. Did I
understand correctly?
Sharon, Sara Sept. 26, 2016, 8:10 a.m. UTC | #2
> The commit log doesn't tell a lot. I started to wonder why use debugfs and not the proper interface through mac80211?

This is RX packet injection, not TX. So the injection goes up - from the driver to mac80211.
Since we use it to test some mvm data path stuff, we don't want to inject at mac80211 level.

> But reading from the code makes me suspect that this isn't really normal packet injection, more like passing full descriptors to the hw. Did I understand correctly?

For injecting at driver level we need the injected packet to contain a fake of the HW descriptor for the driver processing.
This descriptor isn't being passed to the hw, but to the driver since it is RX.
The code isn't creating the descriptor, but it is validating length and other checks to make sure we won't access invalid memory or so.

Sara
---------------------------------------------------------------------
A member of the Intel Corporation group of companies

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.
Kalle Valo Sept. 26, 2016, 8:52 a.m. UTC | #3
"Sharon, Sara" <sara.sharon@intel.com> writes:

>> The commit log doesn't tell a lot. I started to wonder why use
>> debugfs and not the proper interface through mac80211?
>
> This is RX packet injection, not TX. So the injection goes up - from
> the driver to mac80211. Since we use it to test some mvm data path
> stuff, we don't want to inject at mac80211 level.
>
>> But reading from the code makes me suspect that this isn't really
>> normal packet injection, more like passing full descriptors to the
>> hw. Did I understand correctly?
>
> For injecting at driver level we need the injected packet to contain a
> fake of the HW descriptor for the driver processing. This descriptor
> isn't being passed to the hw, but to the driver since it is RX. The
> code isn't creating the descriptor, but it is validating length and
> other checks to make sure we won't access invalid memory or so.

Ah, makes sense. No need to resend this, but in the future please try to
explain higher level design like this in the commit log. That avoids a
lot of guessing (wrong) for me and others.
diff mbox

Patch

diff --git a/drivers/net/wireless/intel/iwlwifi/mvm/debugfs.c b/drivers/net/wireless/intel/iwlwifi/mvm/debugfs.c
index b344898..540b7c9 100644
--- a/drivers/net/wireless/intel/iwlwifi/mvm/debugfs.c
+++ b/drivers/net/wireless/intel/iwlwifi/mvm/debugfs.c
@@ -917,6 +917,59 @@  static ssize_t iwl_dbgfs_indirection_tbl_write(struct iwl_mvm *mvm,
 	return ret ?: count;
 }
 
+static ssize_t iwl_dbgfs_inject_packet_write(struct iwl_mvm *mvm,
+					     char *buf, size_t count,
+					     loff_t *ppos)
+{
+	struct iwl_rx_cmd_buffer rxb = {
+		._rx_page_order = 0,
+		.truesize = 0, /* not used */
+		._offset = 0,
+	};
+	struct iwl_rx_packet *pkt;
+	struct iwl_rx_mpdu_desc *desc;
+	int bin_len = count / 2;
+	int ret = -EINVAL;
+
+	/* supporting only 9000 descriptor */
+	if (!mvm->trans->cfg->mq_rx_supported)
+		return -ENOTSUPP;
+
+	rxb._page = alloc_pages(GFP_ATOMIC, 0);
+	if (!rxb._page)
+		return -ENOMEM;
+	pkt = rxb_addr(&rxb);
+
+	ret = hex2bin(page_address(rxb._page), buf, bin_len);
+	if (ret)
+		goto out;
+
+	/* avoid invalid memory access */
+	if (bin_len < sizeof(*pkt) + sizeof(*desc))
+		goto out;
+
+	/* check this is RX packet */
+	if (WIDE_ID(pkt->hdr.group_id, pkt->hdr.cmd) !=
+	    WIDE_ID(LEGACY_GROUP, REPLY_RX_MPDU_CMD))
+		goto out;
+
+	/* check the length in metadata matches actual received length */
+	desc = (void *)pkt->data;
+	if (le16_to_cpu(desc->mpdu_len) !=
+	    (bin_len - sizeof(*desc) - sizeof(*pkt)))
+		goto out;
+
+	local_bh_disable();
+	iwl_mvm_rx_mpdu_mq(mvm, NULL, &rxb, 0);
+	local_bh_enable();
+	ret = 0;
+
+out:
+	iwl_free_rxb(&rxb);
+
+	return ret ?: count;
+}
+
 static ssize_t iwl_dbgfs_fw_dbg_conf_read(struct file *file,
 					  char __user *user_buf,
 					  size_t count, loff_t *ppos)
@@ -1454,6 +1507,7 @@  MVM_DEBUGFS_WRITE_FILE_OPS(cont_recording, 8);
 MVM_DEBUGFS_WRITE_FILE_OPS(max_amsdu_len, 8);
 MVM_DEBUGFS_WRITE_FILE_OPS(indirection_tbl,
 			   (IWL_RSS_INDIRECTION_TABLE_SIZE * 2));
+MVM_DEBUGFS_WRITE_FILE_OPS(inject_packet, 512);
 
 #ifdef CONFIG_IWLWIFI_BCAST_FILTERING
 MVM_DEBUGFS_READ_WRITE_FILE_OPS(bcast_filters, 256);
@@ -1502,6 +1556,7 @@  int iwl_mvm_dbgfs_register(struct iwl_mvm *mvm, struct dentry *dbgfs_dir)
 	MVM_DEBUGFS_ADD_FILE(send_echo_cmd, mvm->debugfs_dir, S_IWUSR);
 	MVM_DEBUGFS_ADD_FILE(cont_recording, mvm->debugfs_dir, S_IWUSR);
 	MVM_DEBUGFS_ADD_FILE(indirection_tbl, mvm->debugfs_dir, S_IWUSR);
+	MVM_DEBUGFS_ADD_FILE(inject_packet, mvm->debugfs_dir, S_IWUSR);
 	if (!debugfs_create_bool("enable_scan_iteration_notif",
 				 S_IRUSR | S_IWUSR,
 				 mvm->debugfs_dir,