On 07/04/2016 12:05 PM, Johannes Thumshirn wrote:
> On Mon, Jul 04, 2016 at 10:29:22AM +0200, Hannes Reinecke wrote:
>> When running in VN2VN mode there is no central instance which
>> would send out any FIP VLAN discovery notifications. So this
>> patch adds a new sysfs attribute 'fip_vlan_responder' which
>> will activate a FIP VLAN discovery responder.
>>
>> Signed-off-by: Hannes Reinecke <hare@xxxxxxxx>
>> ---
>
> [...]
>
>> +/**
>> + * fcoe_ctlr_vlan_send() - Send a FIP VLAN Notification
>> + * @fip: The FCoE controller
>> + * @sub: sub-opcode for vlan notification or vn2vn vlan notification
>> + * @dest: The destination Ethernet MAC address
>> + * @min_len: minimum size of the Ethernet payload to be sent
>> + */
>> +static void fcoe_ctlr_vlan_send(struct fcoe_ctlr *fip,
>> + enum fip_vlan_subcode sub,
>> + const u8 *dest)
>> +{
>> + struct sk_buff *skb;
>> + struct fip_frame {
>> + struct ethhdr eth;
>> + struct fip_header fip;
>> + struct fip_mac_desc mac;
>> + struct fip_vlan_desc vlan;
>> + } __packed * frame;
>
> Hmmm this is the 2nd time fip_frame is defined in fcoe_ctlr.c. I'd prefere
> having the type definition somewhere else in this file and then use it in
> fcoe_ctlr_vlan_send() and fcoe_ctlr_vn_send().
>
This appears to be the convention in the file; every function which is
sending a frame on the wire defines a local variable 'struct fip_frame'
which contains the actual frame definition.
Might be that the style can be improved, but that should be a separate
patch.
>> + size_t len;
>> + size_t dlen;
>> +
>> + len = sizeof(*frame);
>> + dlen = sizeof(frame->mac) + sizeof(frame->vlan);
>> + len = max(len, sizeof(struct ethhdr));
>> +
>> + skb = dev_alloc_skb(len);
>> + if (!skb)
>> + return;
>
> dev_alloc_skb() uses GFP_ATOMIC so it's actually not unlikely to fail
> so please return -ENOMEM here, just so the caller knows what happened.
>
Again, the calling convention of fcoe_ctlr_send_vlan() insists on using
a void return.
Yes, this should be improved, but again with another patch.
So do I need to send two additional patches for updating the infrastructure?
Cheers,
Hannes
--
Dr. Hannes Reinecke Teamlead Storage & Networking
hare@xxxxxxx +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
