Re: [PATCH v4 1/1] ethdev: parsing multiple representor devargs string

[Ferruh Yigit <ferruh.yigit@amd.com>]

On 1/21/2024 7:19 PM, Harman Kalra wrote:
> Adding support for parsing multiple representor devargs strings
> passed to a PCI BDF. There may be scenario where port representors
> for various PFs or VFs under PFs are required and all these are
> representor ports shall be backed by single pci device. In such
> case port representors can be created using devargs string:
> <PCI BDF>,representor=[pf[0-1],pf2vf[1,2-3],[4-5]]
> 

This patch is to be able to parse multiple representor device argument,
but I am concerned how it is used.

When there are multiple representor ports backed up by same device,
can't it cause syncronization issues, like if two representor interfaces
used for conflicting configurations. Or if datapath will be supported,
what if two representator used simultaneously.



Meanwhile please check some comments below related to the parser code.

<...>

> @@ -459,9 +460,26 @@ eth_dev_devargs_tokenise(struct rte_kvargs *arglist, const char *str_in)
>  			break;
>  
>  		case 3: /* Parsing list */
> -			if (*letter == ']')
> -				state = 2;
> -			else if (*letter == '\0')
> +			if (*letter == ']') {
> +				/* For devargs having singles lists move to state 2 once letter
> +				 * becomes ']' so each can be considered as different pair key
> +				 * value. But in nested lists case e.g. multiple representors
> +				 * case i.e. [pf[0-3],pfvf[3,4-6]], complete nested list should
> +				 * be considered as one pair value, hence checking if end of outer
> +				 * list ']' is reached else stay on state 3.
> +				 */
> +				if ((strcmp("representor", pair->key) == 0)	    &&
> +				    (*(letter + 1) != '\0' && *(letter + 2) != '\0' &&
> +				     *(letter + 3) != '\0')			    &&
> +				    ((*(letter + 2) == 'p' && *(letter + 3) == 'f') ||
> +				     (*(letter + 2) == 'v' && *(letter + 3) == 'f') ||
> +				     (*(letter + 2) == 's' && *(letter + 3) == 'f') ||
> +				     (*(letter + 2) == 'c' && isdigit(*(letter + 3))) ||
> +				     (*(letter + 2) == '[' && isdigit(*(letter + 3)))))
>

Above is hard to understand but there are some assumptions in the input,
can we list supported syntax in the comment to make it more clear.

For example following seems not support, can you please check if
intentional:
[vf0,vf1] // I am aware this can be put as vf[0,1] too
[vf[0,1],3]
[vf[0],vf[1]]

I am not saying above should be supported, but syntax should be clear
what is supported what is not.


Also I can't check but is the redundant input verified, like:
[vf[0-3],vf[3,4]]