[PATCH 13/19] P2PS: add p2ps_supported_cpt configuration property

[Stepanov, Max]

>The above snippet mostly applies to the value that should be sent in PD
>request. The value in PD request indicates all the modes supported by an ASP
>in general. But PD response is supposed to have a single transport bit set and
>that mode should be used for that service, which means the responder
>should/can select the preferred transport mode for that service.

Agree. Please notice that this flow supposes to work on a seeker side handling a follow on PD request and not having per service CPT preferences. 


>Now coming to your patch, it talks about setting supported features as well as
>the priority between them. The overall supported features can be global
>while the priority between them can be specific to each service. As the spec is
>providing flexibility to choose different transports let us not limit this in
>supplicant. I suggest giving the configuration flexibility as much as possible to
>the ASP, which is supposed to be implemented on top of supplicant.

Assume we have per service CPT preference on an advertiser side. How should a seeker (making a final decision) select CPT bit for a follow on PD response if a follow on PD request contains a number of CPT bit options in a feature capability field? Definitely the seeker selection may not match an advertiser per service preference. My point is that perhaps we may need both per service CPT and a global CPT preference lists to cover your requirement. Per service CPT preference list is supposed to be used by an advertiser handling a direct PD request. (To be precise a per service CPT preference usage is restricted only for service advertisements having auto_accept == true and config_methods != KEYPAD.) A global CPT preference list supposed to be used in other cases, e.g. by advertisers when per service preference list is not defined or by seekers on follow-on PD request.

What do you think?


>Another thing, looking at the way p2p_match_feat_cap() is implemented, I
>would like to bring to your notice the point that "feature capability" is not only
>meant for transport modes but for any kind of ASP features. Writing the
>below logic so generic may not good hold for future. In general its good
>practice/mandate to set all the reserved fields to zero.
>
>+	for (i = 0; p2p->cfg->p2ps_asp_cpt_priority[i]; i++) {
>+		if (p2p->cfg->p2ps_asp_cpt_priority[i] & req_cap[0]) {
>+			resp_cap[0] = p2p->cfg->p2ps_asp_cpt_priority[i];
>+			break;
>+		}
>+	}

The first byte is defined as Coordination Protocol Transport Bitmask field, so, PD responder sets only one single bit selecting the transport.
The rest of the feature capability fields are explicitly set to zero a line before this loop:

	os_memset(resp_cap, 0, P2PS_FEATURE_CAPAB_LEN);

I think this function and P2PS_FEATURE_CAPAB_LEN has to be updated when new fields in feature capability to be added.

Thanks,
Max