[dpdk-dev] [PATCH 08/39] event/octeontx: add mailbox support

Eads, Gage gage.eads at intel.com
Thu Mar 23 17:46:07 CET 2017


Hi Jerin,

I identified a few issues below.

Thanks,
Gage

<snip>

>  +static inline void
>  +mbox_send_requeust(struct mbox *m, struct octeontx_mbox_hdr *hdr,
>  +			const void *txmsg, uint16_t txsize)

"requeust" -> "request"

<snip>

>  +
>  +static inline int
>  +mbox_wait_response(struct mbox *m, struct octeontx_mbox_hdr *hdr,
>  +			void *rxmsg, uint16_t rxsize)
>  +{
>  +	int res = 0, wait;
>  +	uint16_t len;
>  +	struct mbox_ram_hdr rx_hdr;
>  +	uint64_t *ram_mbox_hdr = (uint64_t *)m->ram_mbox_base;
>  +	uint8_t *ram_mbox_msg = m->ram_mbox_base + sizeof(struct
>  +mbox_ram_hdr);
>  +
>  +	/* Wait for response */
>  +	wait = MBOX_WAIT_TIME;
>  +	while (wait > 0) {
>  +		rte_delay_us(100);
>  +		rx_hdr.u64 = rte_read64(ram_mbox_hdr);
>  +		if (rx_hdr.chan_state == MBOX_CHAN_STATE_RES)
>  +			break;
>  +		wait -= 10;
>  +	}

'wait' is in units of milliseconds ("Mbox operation timeout in milliseconds"), so the function subtracts 10 ms after spinning for 100 us. Is that intentional?

>  +
>  +	hdr->res_code = rx_hdr.res_code;
>  +	m->tag_own++;
>  +
>  +	/* Tag mismatch */
>  +	if (m->tag_own != rx_hdr.tag) {
>  +		res = -EBADR;
>  +		goto error;
>  +	}
>  +
>  +	/* PF nacked the msg */
>  +	if (rx_hdr.res_code != MBOX_RET_SUCCESS) {
>  +		res = -EBADMSG;
>  +		goto error;
>  +	}
>  +
>  +	/* Timeout */
>  +	if (wait <= 0) {
>  +		res = -ETIMEDOUT;
>  +		goto error;
>  +	}

Will a timeout mean rx_hdr is invalid? If so, the timeout error should be checked before checking for a PF nack or tag mismatch.

<snip>

>  +static inline int
>  +mbox_send(struct mbox *m, struct octeontx_mbox_hdr *hdr, const void
>  *txmsg,
>  +		uint16_t txsize, void *rxmsg, uint16_t rxsize) {
>  +	int res = -EINVAL;
>  +
>  +	if (m->init_once == 0 || hdr == NULL ||
>  +		txsize > MAX_RAM_MBOX_LEN || rxsize >
>  MAX_RAM_MBOX_LEN) {
>  +		ssovf_log_err("Invalid init_once=%d hdr=%p txsz=%d rxsz=%d",
>  +				m->init_once, hdr, txsize, rxsize);
>  +		return res;
>  +	}
>  +
>  +	rte_spinlock_lock(&m->lock);
>  +
>  +	mbox_send_requeust(m, hdr, txmsg, txsize);

"requeust" -> "request"



More information about the dev mailing list