Re: [PATCH v01] net/af_packet: don't specify protocol on socket create

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

On 10/13/2024 2:59 PM, Gur Stavi wrote:
> When creating AF_PACKET socket with specified protocol it is
> immediately implicitly bound to any existing interface and
> becomes RUNNING. Calling bind on such socket is affectively unbind
> from "any interface", then bind to the specific interface.
> 

Doesn't mean even if we don't bind the socket, will it receive packets?
And what is "any interface" here, does it mean all active interfaces or
one active interface, like first one?

> When creating socket with 0 as protocol, it is created in non-RUNNING
> state, then it can be bound to interface and protocol in a single bind
> call and switch to RUNNING state.
> 
> Especially with ETH_P_ALL, binding to any interface is not a good idea.
> It is safer and faster to use the 2nd approach.
>

I can see unbinding and binding may add to initialization time but that
is slow path anyway.
Why it is not good idea, or less safe?

> 
> This patch replaces protocol in socket creation from ETH_P_ALL to 0.
> 
> Signed-off-by: Gur Stavi <gur.stavi@huawei.com>
> ---
>  drivers/net/af_packet/rte_eth_af_packet.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/af_packet/rte_eth_af_packet.c b/drivers/net/af_packet/rte_eth_af_packet.c
> index 68870dd3e2..b30f88d618 100644
> --- a/drivers/net/af_packet/rte_eth_af_packet.c
> +++ b/drivers/net/af_packet/rte_eth_af_packet.c
> @@ -650,7 +650,7 @@ open_packet_iface(const char *key __rte_unused,
>  	int *sockfd = extra_args;
>  
>  	/* Open an AF_PACKET socket... */
> -	*sockfd = socket(AF_PACKET, SOCK_RAW, htons(ETH_P_ALL));
> +	*sockfd = socket(AF_PACKET, SOCK_RAW, 0);
>  	if (*sockfd == -1) {
>  		PMD_LOG(ERR, "Could not open AF_PACKET socket");
>  		return -1;
> @@ -778,7 +778,7 @@ rte_pmd_init_internals(struct rte_vdev_device *dev,
>  
>  	for (q = 0; q < nb_queues; q++) {
>  		/* Open an AF_PACKET socket for this queue... */
> -		qsockfd = socket(AF_PACKET, SOCK_RAW, htons(ETH_P_ALL));
> +		qsockfd = socket(AF_PACKET, SOCK_RAW, 0);
>  		if (qsockfd == -1) {
>  			PMD_LOG_ERRNO(ERR,
>  				"%s: could not open AF_PACKET socket",
> 
> base-commit: 98613d32e3dac58d685f4f236cf8cc9733abaaf3