Jason Gunthorpe wrote:
> Check that the format of the multicast link address is correct before
> taking it from dev->mc_list to priv->multicast_list. This way we never
> try to send a bogus address to the SA, and prevents badness from
> erronous 'ip maddr addr add', broken bonding drivers, or whatever.
>
> Signed-off-by: Jason Gunthorpe <[email protected]>
> ---
> drivers/infiniband/ulp/ipoib/ipoib_multicast.c | 18 ++++++++++++++++++
> 1 files changed, 18 insertions(+), 0 deletions(-)
>
> Same problem Moni was working on, but lets just address it directly.
>
> There is work to try and fix the bonding driver but no fixed version
> is in mainline yet. This is a cheap and simple work around that is
> worth having even once the driver is fixed.
>
> Despite this, I think it is still necessary to do something like Moni
> was trying - to prevent the MCG join queue from head of line blocking
> on a single bad SA response. This can happen even if everything is
> correct.
>
> diff --git a/drivers/infiniband/ulp/ipoib/ipoib_multicast.c
> b/drivers/infiniband/ulp/ipoib/ipoib_multicast.c
> index 425e311..973a24b 100644
> --- a/drivers/infiniband/ulp/ipoib/ipoib_multicast.c
> +++ b/drivers/infiniband/ulp/ipoib/ipoib_multicast.c
> @@ -758,6 +758,20 @@ void ipoib_mcast_dev_flush(struct net_device *dev)
> }
> }
>
> +static int check_mcast(const u8 *addr,unsigned int addrlen,
> + const u8 *broadcast)
> +{
> + if (addrlen != 20)
> + return 0;
> + /* QPN, scope, reserved, sigature upper */
> + if (memcmp(addr,broadcast,6) != 0)
> + return 0;
> + /* signature lower, pkey */
> + if (memcmp(addr + 7,broadcast+7,3) != 0)
> + return 0;
> + return 1;
> +}
> +
> void ipoib_mcast_restart_task(struct work_struct *work)
> {
> struct ipoib_dev_priv *priv =
> @@ -791,6 +805,10 @@ void ipoib_mcast_restart_task(struct work_struct *work)
> for (mclist = dev->mc_list; mclist; mclist = mclist->next) {
> union ib_gid mgid;
>
> + if (!check_mcast(mclist->dmi_addr,mclist->dmi_addrlen,
> + dev->broadcast))
> + continue;
> +
> memcpy(mgid.raw, mclist->dmi_addr + 4, sizeof mgid);
>
> mcast = __ipoib_mcast_find(dev, &mgid);
Why not check validity with something that looks like the reverse operation
of the kernel function that maps ip -> link mcast addresses?
for example this is the function for IPv4
static inline void ip_ib_mc_map(__be32 naddr, const unsigned char *broadcast,
char *buf)
{
__u32 addr;
unsigned char scope = broadcast[5] & 0xF;
buf[0] = 0; /* Reserved */
buf[1] = 0xff; /* Multicast QPN */
buf[2] = 0xff;
buf[3] = 0xff;
addr = ntohl(naddr);
buf[4] = 0xff;
buf[5] = 0x10 | scope; /* scope from broadcast address */
buf[6] = 0x40; /* IPv4 signature */
buf[7] = 0x1b;
buf[8] = broadcast[8]; /* P_Key */
buf[9] = broadcast[9];
buf[10] = 0;
buf[11] = 0;
buf[12] = 0;
buf[13] = 0;
buf[14] = 0;
buf[15] = 0;
buf[19] = addr & 0xff;
addr >>= 8;
buf[18] = addr & 0xff;
addr >>= 8;
buf[17] = addr & 0xff;
addr >>= 8;
buf[16] = addr & 0x0f;
}
_______________________________________________
general mailing list
[email protected]
http://lists.openfabrics.org/cgi-bin/mailman/listinfo/general
To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
